All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	tytso@mit.edu, linux-kernel@vger.kernel.org,
	linux-crypto@vger.kernel.org, stable@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] random: use correct memory barriers for crng_node_pool
Date: Tue, 22 Sep 2020 11:59:31 -0700	[thread overview]
Message-ID: <20200922185931.GA1616407@gmail.com> (raw)
In-Reply-To: <20200922184243.GA29330@paulmck-ThinkPad-P72>

On Tue, Sep 22, 2020 at 11:42:43AM -0700, Paul E. McKenney wrote:
> On Tue, Sep 22, 2020 at 09:51:36AM +1000, Herbert Xu wrote:
> > On Mon, Sep 21, 2020 at 04:26:39PM -0700, Paul E. McKenney wrote:
> > >
> > > > But this reasoning could apply to any data structure that contains
> > > > a spin lock, in particular ones that are dereferenced through RCU.
> > > 
> > > I lost you on this one.  What is special about a spin lock?
> > 
> > I don't know, that was Eric's concern.  He is inferring that
> > spin locks through lockdep debugging may trigger dependencies
> > that require smp_load_acquire.
> > 
> > Anyway, my point is if it applies to crng_node_pool then it
> > would equally apply to RCU in general.
> 
> Referring to the patch you call out below...
> 
> Huh.  The old cmpxchg() primitive is fully ordered, so the old mb()
> preceding it must have been for correctly interacting with hardware on
> !SMP systems.  If that is the case, then the use of cmpxchg_release()
> is incorrect.  This is not the purview of the memory model, but rather
> of device-driver semantics.  Or does crng not (or no longer, as the case
> might be) interact with hardware RNGs?

No hardware involved here.  The mb() is just unnecessary, as I noted in my patch
https://lore.kernel.org/lkml/20200916233042.51634-1-ebiggers@kernel.org/.

> What prevents either the old or the new code from kfree()ing the old
> state out from under another CPU that just now picked up a pointer to the
> old state?  The combination of cmpxchg_release() and smp_load_acquire()
> won't do anything to prevent this from happening.  This is after all not
> a memory-ordering issue, but instead an object-lifetime issue.  But maybe
> you have a lock or something that provides the needed protection.  I don't
> see how this can be the case and still require the cmpxchg_release()
> and smp_load_acquire(), but perhaps this is a failure of imagination on
> my part.

crng_node_pool is initialized only once, and never freed.

- Eric

  reply	other threads:[~2020-09-22 18:59 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
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 [this message]
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=20200922185931.GA1616407@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.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.