linux-kernel - Re: [PATCH v6] random: fix crash on multiple early calls to add_bootloader

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAHmME9qC2iX5EvDLj5YBZBdDJ_n8xTUYZT73qonsEhbKCwizFw@mail.gmail.com>
Date:   Tue, 28 Dec 2021 15:06:05 +0100
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Dominik Brodowski <linux@...inikbrodowski.net>
Cc:     "Theodore Ts'o" <tytso@....edu>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        "Ivan T. Ivanov" <iivanov@...e.de>,
        Ard Biesheuvel <ardb@...nel.org>, linux-efi@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6] random: fix crash on multiple early calls to add_bootloader_randomness()

Hi Dominik,

Thanks. I like where this is going. It seems like we can tackle this
problem and the other one separately like this as you've done. It'd be
helpful though if you could send the patches for all the problems in
one patchset so that I'm not left wondering, "hey what about this
other thing; did you forget it?" Since these issues are kind of
intermingled, it'd be nice to see how the puzzle pieces fit together.

One question about this patch:
- If crng_reseed is called early, before system_wq is non-NULL, that
block will be skipped. When will it be called again?
- There's no call to it in crng_initialize_primary/rand_initialize
when not trusting the CPU and such, and that block there isn't quite
the same as crng_reseed either.

Also, something I noticed when looking at this, which I'm not sure has
come up yet in the various problems identified:
- If crng_reseed is called early, but not too early, and that block is
called, we'll set crng_init=2 and do various things and print, "crng
init done\n".
- Later, when rand_initialize is called, if we're trusting the CPU and
such, we'll re-initialize it and print, "crng done (trusting CPU's
manufacturer)\n".
That seems like a problem, though I assume we haven't hit that yet
because the race window is pretty small, so we've mostly been crashing
on a NULL system_wq instead, or having crng_reseed called _after_
crng_initialize_primary/rand_initialize anyway.

Thanks a lot for working on this. If you get tired of this back and
forth, by the way, and want me to start proposing patches for you to
look at instead, we can trade off. I'm also happy to keep looking at
what you send; it's just that as we're already on v6 here, I'm hoping
you won't get too frustrated.

Jason