From: Eric Biggers <ebiggers@kernel.org>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: tytso@mit.edu, linux-kernel@vger.kernel.org,
linux-crypto@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] random: use correct memory barriers for crng_node_pool
Date: Thu, 17 Sep 2020 09:58:02 -0700 [thread overview]
Message-ID: <20200917165802.GC855@sol.localdomain> (raw)
In-Reply-To: <20200917072644.GA5311@gondor.apana.org.au>
On Thu, Sep 17, 2020 at 05:26:44PM +1000, Herbert Xu wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > When a CPU selects which CRNG to use, it accesses crng_node_pool without
> > a memory barrier. That's wrong, because crng_node_pool can be set by
> > another CPU concurrently. Without a memory barrier, the crng_state that
> > is used might not appear to be fully initialized.
>
> The only architecture that requires a barrier for data dependency
> is Alpha. The correct primitive to ensure that barrier is present
> is smp_barrier_depends, or you could just use READ_ONCE.
>
smp_load_acquire() is obviously correct, whereas READ_ONCE() is an optimization
that is difficult to tell whether it's correct or not. For trivial data
structures it's "easy" to tell. But whenever there is a->b where b is an
internal implementation detail of another kernel subsystem, the use of which
could involve accesses to global or static data (for example, spin_lock()
accessing lockdep stuff), a control dependency can slip in.
The last time I tried to use READ_ONCE(), it started a big controversy
(https://lkml.kernel.org/linux-fsdevel/20200713033330.205104-1-ebiggers@kernel.org/T/#u,
https://lkml.kernel.org/linux-fsdevel/20200717044427.68747-1-ebiggers@kernel.org/T/#u,
https://lwn.net/Articles/827180/). In the end, people refused to even allow the
READ_ONCE() optimization to be documented, because they felt that
smp_load_acquire() should just be used instead.
So I think we should just go with smp_load_acquire()...
- Eric
next prev parent reply other threads:[~2020-09-17 16:58 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-16 23:30 [PATCH] random: use correct memory barriers for crng_node_pool Eric Biggers
2020-09-17 7:26 ` Herbert Xu
2020-09-17 16:58 ` Eric Biggers [this message]
2020-09-21 8:19 ` Herbert Xu
2020-09-21 15:27 ` Paul E. McKenney
2020-09-21 22:11 ` Herbert Xu
2020-09-21 23:26 ` Paul E. McKenney
2020-09-21 23:51 ` Herbert Xu
2020-09-22 18:42 ` Paul E. McKenney
2020-09-22 18:59 ` Eric Biggers
2020-09-22 20:31 ` Paul E. McKenney
2020-09-21 23:52 ` Eric Biggers
2020-09-22 18:31 ` Paul E. McKenney
2020-09-22 19:09 ` Eric Biggers
2020-09-22 20:56 ` Paul E. McKenney
2020-09-22 21:55 ` Eric Biggers
2020-09-25 0:59 ` Paul E. McKenney
2020-09-25 2:09 ` Eric Biggers
2020-09-25 3:31 ` Paul E. McKenney
2020-10-02 3:07 ` Eric Biggers
2020-10-08 18:31 ` Paul E. McKenney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200917165802.GC855@sol.localdomain \
--to=ebiggers@kernel.org \
--cc=herbert@gondor.apana.org.au \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.