Re: [REGRESSION][BISECTED] Boot stall from merge tag 'net-next-6.2'

[Ard Biesheuvel <ardb@kernel.org>]

On Fri, 23 Jun 2023 at 23:52, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, 23 Jun 2023 at 13:31, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > We always have to write when using so that we don't credit the same
> > seed twice, so it's gotta be used at a stage when SetVariable is
> > somewhat working.
>
> This code isn't even the code that "uses" the alleged entropy from
> that EFI variable in the first place. That's the code in
> efi_random_get_seed() in the EFI boot sequence, and appends it to the
> bootup randomness buffers.
>
> And that code already seems to clear the EFI variable (or seems to
> append to it).
>

It reads the variable twice (once to obtain the size and once to grab
the data), and replaces it with a zero-length string, which causes the
variable to disappear. (This is typically NOR flash with spare blocks
managed by a fault tolerant write layer in software, and so really
wiping the seed or overwriting it is not generally possible)

Using SetVariable() from boot services to delete a variable is highly
unlikely to regress older systems in a similar way.

> So this argument seems to be complete garbage - we absolutely do not
> have to write it, and your patch already just wrote it in the wrong
> place anyway.
>
> Don't make excuses. That code caused boot failures, it was all done in
> the wrong place, and at entirely the wrong time.
>

With the revert applied, the kernel/EFI stub only consumes the
variable and deletes it, but never creates it by itself, and so the
code does nothing if the variable is never created in the first place.

If we leave it up to user space to create it, we won´t need any policy
or quirks handling in the kernel at all, which I´d prefer. The only
thing we should do is special case the variable's scope GUID in
efivarfs so the file is not created world-readable like we do for
other variables. (This predates my involvement but I think this was an
oversight). Using efivarfs will also ensure that the 'storage
paranoia' logic is used on x86. (This is something I failed to take
into account when I reviewed Jason's patch)