Message ID | 20220423212623.1957011-7-Jason@zx2c4.com |
---|---|
State | Superseded |
Headers | show |
Series | archs/random: fallback to best raw ktime when no cycle counter | expand |
On Sat, Apr 23 2022 at 23:26, Jason A. Donenfeld wrote: > The addition of random_get_entropy_fallback() provides access to > whichever time source has the highest frequency, which is useful for > gathering entropy on platforms without available cycle counters. It's > not necessarily as good as being able to quickly access a cycle counter > that the CPU has, but it's still something, even when it falls back to > being jiffies-based. > > In the event that a given arch does not define get_cycles(), falling > back to the get_cycles() default implementation that returns 0 is really > not the best we can do. Instead, at least calling > random_get_entropy_fallback() would be preferable, because that always > needs to return _something_, even falling back to jiffies eventually. > It's not as though random_get_entropy_fallback() is super high precision > or guaranteed to be entropic, but basically anything that's not zero all > the time is better than returning zero all the time. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Theodore Ts'o <tytso@mit.edu> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Not that I care much, but in general taking over authorship w/o attribution via Suggested-by or such is frowned upon. Thanks, tglx
Hi Thomas, On Mon, Apr 25, 2022 at 02:37:11PM +0200, Thomas Gleixner wrote: > On Sat, Apr 23 2022 at 23:26, Jason A. Donenfeld wrote: > > The addition of random_get_entropy_fallback() provides access to > > whichever time source has the highest frequency, which is useful for > > gathering entropy on platforms without available cycle counters. It's > > not necessarily as good as being able to quickly access a cycle counter > > that the CPU has, but it's still something, even when it falls back to > > being jiffies-based. > > > > In the event that a given arch does not define get_cycles(), falling > > back to the get_cycles() default implementation that returns 0 is really > > not the best we can do. Instead, at least calling > > random_get_entropy_fallback() would be preferable, because that always > > needs to return _something_, even falling back to jiffies eventually. > > It's not as though random_get_entropy_fallback() is super high precision > > or guaranteed to be entropic, but basically anything that's not zero all > > the time is better than returning zero all the time. > > > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Theodore Ts'o <tytso@mit.edu> > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > Not that I care much, but in general taking over authorship w/o > attribution via Suggested-by or such is frowned upon. Sorry about that. Usually I'm pretty good about adding those. I guess something must have gotten lost this time through, as the v1 of this started out using sched_clock() (Arnd's suggestion) and then moved to using the raw ktime clock after your suggestion, and I missed the Suggested-by. I'll add that. Meanwhile, do you want to Ack this patch? Do the technical aspects look okay to you? Jason
On Mon, Apr 25 2022 at 15:22, Jason A. Donenfeld wrote: > On Mon, Apr 25, 2022 at 02:37:11PM +0200, Thomas Gleixner wrote: >> On Sat, Apr 23 2022 at 23:26, Jason A. Donenfeld wrote: >> > >> > Cc: Thomas Gleixner <tglx@linutronix.de> >> > Cc: Arnd Bergmann <arnd@arndb.de> >> > Cc: Theodore Ts'o <tytso@mit.edu> >> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> >> >> Not that I care much, but in general taking over authorship w/o >> attribution via Suggested-by or such is frowned upon. > > Sorry about that. Usually I'm pretty good about adding those. I guess > something must have gotten lost this time through, as the v1 of this > started out using sched_clock() (Arnd's suggestion) and then moved to > using the raw ktime clock after your suggestion, and I missed the > Suggested-by. I'll add that. Meanwhile, do you want to Ack this patch? > Do the technical aspects look okay to you? Yes. Please fix the subject line: timekeeping: Add raw clock fallback for random_get_entropy() and stick the Cc's below the SOB, so it conforms with the TIP tree rules. Other than that: Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
diff --git a/include/linux/timex.h b/include/linux/timex.h index 5745c90c8800..3871b06bd302 100644 --- a/include/linux/timex.h +++ b/include/linux/timex.h @@ -62,6 +62,8 @@ #include <linux/types.h> #include <linux/param.h> +unsigned long random_get_entropy_fallback(void); + #include <asm/timex.h> #ifndef random_get_entropy @@ -74,8 +76,14 @@ * * By default we use get_cycles() for this purpose, but individual * architectures may override this in their asm/timex.h header file. + * If a given arch does not have get_cycles(), then we fallback to + * using random_get_entropy_fallback(). */ +#ifdef get_cycles #define random_get_entropy() ((unsigned long)get_cycles()) +#else +#define random_get_entropy() random_get_entropy_fallback() +#endif #endif /* diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index dcdcb85121e4..7cd2ec239cae 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -17,6 +17,7 @@ #include <linux/clocksource.h> #include <linux/jiffies.h> #include <linux/time.h> +#include <linux/timex.h> #include <linux/tick.h> #include <linux/stop_machine.h> #include <linux/pvclock_gtod.h> @@ -2380,6 +2381,15 @@ static int timekeeping_validate_timex(const struct __kernel_timex *txc) return 0; } +/** + * random_get_entropy_fallback - Returns the raw clock source value, + * used by random.c for platforms with no valid random_get_entropy(). + */ +unsigned long random_get_entropy_fallback(void) +{ + return tk_clock_read(&tk_core.timekeeper.tkr_mono); +} +EXPORT_SYMBOL_GPL(random_get_entropy_fallback); /** * do_adjtimex() - Accessor function to NTP __do_adjtimex function
The addition of random_get_entropy_fallback() provides access to whichever time source has the highest frequency, which is useful for gathering entropy on platforms without available cycle counters. It's not necessarily as good as being able to quickly access a cycle counter that the CPU has, but it's still something, even when it falls back to being jiffies-based. In the event that a given arch does not define get_cycles(), falling back to the get_cycles() default implementation that returns 0 is really not the best we can do. Instead, at least calling random_get_entropy_fallback() would be preferable, because that always needs to return _something_, even falling back to jiffies eventually. It's not as though random_get_entropy_fallback() is super high precision or guaranteed to be entropic, but basically anything that's not zero all the time is better than returning zero all the time. Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Theodore Ts'o <tytso@mit.edu> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- include/linux/timex.h | 8 ++++++++ kernel/time/timekeeping.c | 10 ++++++++++ 2 files changed, 18 insertions(+)