[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 19 Jun 2017 22:55:37 +0200
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: "Theodore Ts'o" <tytso@....edu>, Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>, kernel-hardening@...ts.openwall.com,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Eric Biggers <ebiggers3@...il.com>,
Linus Torvalds <torvalds@...ux-foundation.org>, David Miller <davem@...emloft.net>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] random: silence compiler warnings and fix race
On Mon, Jun 19, 2017 at 9:45 AM, Sebastian Andrzej Siewior
<bigeasy@...utronix.de> wrote:
> ehm. You sure? I simply delayed the lock-dropping _after_ the state
> variable was been modified. So it was basically what your patch did
> except it was unlocked later…
Yes, I'm sure. You moved the call to invalidate_batched_entropy() to
be after the assignment of crng_init. However, the call to
invalidate_batched_entropy() must be made _before_ the assignment of
crng_init.
>> > Are use about that? I am not sure that the gcc will inline "crng_init"
>> > read twice. It is not a local variable. READ_ONCE() is usually used
>> > where gcc could cache a memory access but you do not want this. But hey!
>> > If someone knows better I am here to learn.
>>
>> The whole purpose is that I _want_ it to cache the memory access so
>> that it is _not_ inlined. So, based on your understanding, it does
>> exactly what I intended it to do. The reason is that I'd like to avoid
>> a lock imbalance, which could happen if the read is inlined.
>
> So it was good as it was which means you can drop that READ_ONCE().
Except READ_ONCE ensures that the compiler will never inline it, so it
actually needs to stay.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
