| Message ID | 20221116161642.1670235-3-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Use EFI variables for random seed | expand |
On Wed 2022-11-16 17:16:38, Jason A. Donenfeld wrote: > Rather than polling every second, use the new notifier to do this at > exactly the right moment. Great news! > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -752,26 +753,21 @@ early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); > > static bool filled_random_ptr_key __read_mostly; > static siphash_key_t ptr_key __read_mostly; > -static void fill_ptr_key_workfn(struct work_struct *work); > -static DECLARE_DELAYED_WORK(fill_ptr_key_work, fill_ptr_key_workfn); > > -static void fill_ptr_key_workfn(struct work_struct *work) > +static int fill_ptr_key(struct notifier_block *nb, unsigned long action, void *data) > { > - if (!rng_is_initialized()) { > - queue_delayed_work(system_unbound_wq, &fill_ptr_key_work, HZ * 2); > - return; > - } > - > get_random_bytes(&ptr_key, sizeof(ptr_key)); > > /* Pairs with smp_rmb() before reading ptr_key. */ > smp_wmb(); > WRITE_ONCE(filled_random_ptr_key, true); > + return 0; I believe that we should rather return NOTIFY_DONE here. It is rather a formal change. The value is 0 as well. That said, I have never really understood the difference between NOTIFY_OK and NOTIFY_DONE. > } > > static int __init vsprintf_init_hashval(void) > { > - fill_ptr_key_workfn(NULL); > + static struct notifier_block fill_ptr_key_nb = { .notifier_call = fill_ptr_key }; > + notify_on_rng_initialized(&fill_ptr_key_nb); > return 0; > } > subsys_initcall(vsprintf_init_hashval) Anyway, the code looks good to me: Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
On Fri, Nov 18, 2022 at 3:16 PM Petr Mladek <pmladek@suse.com> wrote: > > + return 0; > > I believe that we should rather return NOTIFY_DONE here. > It is rather a formal change. The value is 0 as well. > > That said, I have never really understood the difference between > NOTIFY_OK and NOTIFY_DONE. Ah yes, the varying degrees of apathy: #define NOTIFY_DONE 0x0000 /* Don't care */ #define NOTIFY_OK 0x0001 /* Suits me */ In a sense, the fact that there's this return value at all indicates a notifier block isn't *quite* the API we want, since this happens only once and it really should never stop. But it's so convenient and small to use that I think it's fine. Anyway, I'll use the right constant here as you suggested. Jason
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 24f37bab8bc1..70aa5de3c330 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -41,6 +41,7 @@ #include <linux/siphash.h> #include <linux/compiler.h> #include <linux/property.h> +#include <linux/notifier.h> #ifdef CONFIG_BLOCK #include <linux/blkdev.h> #endif @@ -752,26 +753,21 @@ early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); static bool filled_random_ptr_key __read_mostly; static siphash_key_t ptr_key __read_mostly; -static void fill_ptr_key_workfn(struct work_struct *work); -static DECLARE_DELAYED_WORK(fill_ptr_key_work, fill_ptr_key_workfn); -static void fill_ptr_key_workfn(struct work_struct *work) +static int fill_ptr_key(struct notifier_block *nb, unsigned long action, void *data) { - if (!rng_is_initialized()) { - queue_delayed_work(system_unbound_wq, &fill_ptr_key_work, HZ * 2); - return; - } - get_random_bytes(&ptr_key, sizeof(ptr_key)); /* Pairs with smp_rmb() before reading ptr_key. */ smp_wmb(); WRITE_ONCE(filled_random_ptr_key, true); + return 0; } static int __init vsprintf_init_hashval(void) { - fill_ptr_key_workfn(NULL); + static struct notifier_block fill_ptr_key_nb = { .notifier_call = fill_ptr_key }; + notify_on_rng_initialized(&fill_ptr_key_nb); return 0; } subsys_initcall(vsprintf_init_hashval)
Rather than polling every second, use the new notifier to do this at exactly the right moment. Cc: Mike Galbraith <efault@gmx.de> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Cc: Petr Mladek <pmladek@suse.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- lib/vsprintf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)