On Mon, Apr 23, 2018 at 09:21:48AM +0200, Salvatore Bonaccorso wrote:
> Hi
>
> On Sun, Apr 22, 2018 at 11:28:52PM +0100, Ben Hutchings wrote:
> > On Sun, 2018-04-22 at 15:53 +0200, Greg Kroah-Hartman wrote:
> > > 4.9-stable review patch. If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: Theodore Ts'o <tytso@xxxxxxx>
> > >
> > > commit 8ef35c866f8862df074a49a93b0309725812dea8 upstream.
> > >
> > > Until the primary_crng is fully initialized, don't initialize the NUMA
> > > crng nodes. Otherwise users of /dev/urandom on NUMA systems before
> > > the CRNG is fully initialized can get very bad quality randomness. Of
> > > course everyone should move to getrandom(2) where this won't be an
> > > issue, but there's a lot of legacy code out there. This related to
> > > CVE-2018-1108.
> > >
> > > Reported-by: Jann Horn <jannh@xxxxxxxxxx>
> > > Fixes: 1e7f583af67b ("random: make /dev/urandom scalable for silly...")
> > > Cc: stable@xxxxxxxxxx # 4.8+
> > > Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >
> > In 4.9 (and probably older branches too) this leads to a deadlock:
> >
> > crng_reseed(primary_crng, ...) takes primary_crng.lock
> > -> numa_rcng_init()
> > -> crng_initialize()
> > -> get_random_bytes()
> > -> extract_crng()
> > -> _extract_crng(primary_crng, ...) tries to take primary_crng.lock
> >
> > I think this can be fixed by backporting commit 4a072c71f49b
> > "random: silence compiler warnings and fix race" but I'm not sure
> > whether that depends on other changes.
>
> That is, the following test patch on top of the 4.9-stable review
> queue seem to resolve the issue. The commit message of the original
> commit 4a072c71f49b0a0e495ea13423bdb850da73c58c would though not match
> anymore.
>
> Regards,
> Salvatore
> From: "Jason A. Donenfeld" <Jason@xxxxxxxxx>
> Date: Thu, 15 Jun 2017 00:45:26 +0200
> Subject: random: silence compiler warnings and fix race
> Origin: https://git.kernel.org/linus/4a072c71f49b0a0e495ea13423bdb850da73c58c
>
> Odd versions of gcc for the sh4 architecture will actually warn about
> flags being used while uninitialized, so we set them to zero. Non crazy
> gccs will optimize that out again, so it doesn't make a difference.
>
> Next, over aggressive gccs could inline the expression that defines
> use_lock, which could then introduce a race resulting in a lock
> imbalance. By using READ_ONCE, we prevent that fate. Finally, we make
> that assignment const, so that gcc can still optimize a nice amount.
>
> Finally, we fix a potential deadlock between primary_crng.lock and
> batched_entropy_reset_lock, where they could be called in opposite
> order. Moving the call to invalidate_batched_entropy to outside the lock
> rectifies this issue.
>
> Fixes: b169c13de473a85b3c859bb36216a4cb5f00a54a
> Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> [Salvatore Bonaccorso: backport to 4.9: context changes, only apply change to
> address potential deadlock]
> ---
> drivers/char/random.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> Index: linux-stable/drivers/char/random.c
> ===================================================================
> --- linux-stable.orig/drivers/char/random.c
> +++ linux-stable/drivers/char/random.c
> @@ -810,12 +810,12 @@ static int crng_fast_load(const char *cp
> p[crng_init_cnt % CHACHA20_KEY_SIZE] ^= *cp;
> cp++; crng_init_cnt++; len--;
> }
> + spin_unlock_irqrestore(&primary_crng.lock, flags);
> if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
> crng_init = 1;
> wake_up_interruptible(&crng_init_wait);
> pr_notice("random: fast init done\n");
> }
> - spin_unlock_irqrestore(&primary_crng.lock, flags);
> return 1;
> }
>
> @@ -873,6 +873,7 @@ static void crng_reseed(struct crng_stat
> }
> memzero_explicit(&buf, sizeof(buf));
> crng->init_time = jiffies;
> + spin_unlock_irqrestore(&crng->lock, flags);
> if (crng == &primary_crng && crng_init < 2) {
> numa_crng_init();
> crng_init = 2;
> @@ -880,7 +881,6 @@ static void crng_reseed(struct crng_stat
> wake_up_interruptible(&crng_init_wait);
> pr_notice("random: crng init done\n");
> }
> - spin_unlock_irqrestore(&crng->lock, flags);
> }
>
> static inline void maybe_reseed_primary_crng(void)
As this depends on other patches, that are not in 4.9, I don't think
this will solve the problem correctly. I've dropped the offending patch
for now, hopefully that should be good for the moment.
thanks,
greg k-h