Re: [PATCH v4] efi: random: combine bootloader provided RNG seed with RNG protocol output

["Jason A. Donenfeld" <Jason@zx2c4.com>]

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@linaro.org>
> Cc: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Lennart Poettering <lennart@poettering.net>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

I was thinking it might be best to add:

   Cc: stable@vger.kernel.org

"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@zx2c4.com>

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