linux-kernel - Re: [PATCH] random: use correct memory barriers for crng_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200917165802.GC855@sol.localdomain>
Date:   Thu, 17 Sep 2020 09:58:02 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Herbert Xu <herbert@...dor.apana.org.au>
Cc:     tytso@....edu, linux-kernel@...r.kernel.org,
        linux-crypto@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] random: use correct memory barriers for crng_node_pool

On Thu, Sep 17, 2020 at 05:26:44PM +1000, Herbert Xu wrote:
> Eric Biggers <ebiggers@...nel.org> wrote:
> > From: Eric Biggers <ebiggers@...gle.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