Hi Ard,
Looking good! Thanks. A few brief comments.
On Wed, Nov 09, 2022 at 10:55:58AM +0100, Ard Biesheuvel wrote:
> The recommended seed
> size is 32 bytes, anything beyond that is disregarded when the seeds are
> concatenated.
This should read, "The recommended seed size is 32 bytes, and seeds
larger than 512 bytes are considered corrupted and ignored entirely."
> Cc: Ilias Apalodimas <ilias.apalodimas@xxxxxxxxxx>
> Cc: Jason A. Donenfeld <Jason@xxxxxxxxx>
> Cc: Lennart Poettering <lennart@xxxxxxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
I was thinking it might be best to add:
Cc: stable@xxxxxxxxxxxxxxx
"Don't clobber existing GUID" seems like it's arguably a fix.
> status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
> - sizeof(*seed) + EFI_RANDOM_SEED_SIZE,
> + struct_size(seed, bits, seed_size),
> (void **)&seed);
> if (status != EFI_SUCCESS)
> return status;
Should this print "Failed to something something, retaining
bootloader-supplied seed only", like the err_freepool case?
> + efi_warn("Failed to obtain seed from EFI_RNG_PROTOCOL%s\n",
> + prev_seed ? ", retaining bootloader supplied seed only" : "");
"bootloader-supplied" with the hyphen, right?
If you make any of the above changes, feel free to do it when you commit
this. IOW, my comments are nits, so:
Reviewed-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
Also, I verified that kexec correctly sets everything back to 32 bytes
with this line:
size = min(seed->size, EFI_RANDOM_SEED_SIZE);
So that's good.
Jason
