Re: [PATCH V8 2/2] printk: hash addresses printed with %p

From: Tobin C. Harding
Date: Mon Oct 30 2017 - 18:44:27 EST


On Mon, Oct 30, 2017 at 05:33:22PM -0400, Steven Rostedt wrote:
> On Thu, 26 Oct 2017 13:58:38 +1100
> "Tobin C. Harding" <me@xxxxxxxx> wrote:
>
> > > +static bool have_filled_random_ptr_key;
> > > +static siphash_key_t ptr_key __read_mostly;
> > > +
> > > +static void fill_random_ptr_key(struct random_ready_callback *unused)
> > > +{
> > > + get_random_bytes(&ptr_key, sizeof(ptr_key));
> > > + WRITE_ONCE(have_filled_random_ptr_key, true);
> >
> > This usage of WRITE_ONCE was suggested by Jason A. Donenfeld. I read
> > include/linux/compiler.h but was not able to grok it. Is this enough to
> > stop the compiler re-ordering these two statements?
> >
> > Or do I need to read Documentation/memory-barriers.txt [again]?
>
> No, the WRITE_ONCE does not stop the compiler from reordering those
> statements. If you need that, then you need to do:
>
> get_random_bytes(&ptr_key, sizeof(ptr_key));
> barrier();
> WRITE_ONCE(have_filled_random_ptr_key, true);
>
> and that only works against interrupts. If you need synchronization
> across CPUs, then you need smp_mb().

Cool. So I think we need

get_random_bytes(&ptr_key, sizeof(ptr_key));
smp_mb();
WRITE_ONCE(have_filled_random_ptr_key, true);

V10 to include this unless I have it wrong.

thanks,
Tobin.