Re: [PATCH v3 3/3] m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED

Jason A. Donenfeld
2022-09-26T12:31:51Z

Hi Geert,

On Mon, Sep 26, 2022 at 12:27:13PM +0200, Geert Uytterhoeven wrote:
> The approach LGTM, but given my doubts on [PATCH v3 2/3], I think this
> patch should be moved up, to not depend on [2/3].

I'll do that. But also, I'm not sure I yet agree with you about 2/3, so
we should keep discussing that. (I'll still move it to 3/3 though.)

> > -#define BI_VIRT_RNG_SEED 0x8006
> > +/* No longer used -- replaced with BI_RNG_SEED -- but don't reuse this index:
> > + * #define BI_VIRT_RNG_SEED 0x8006 */
>
> Please drop the space before TAB.

Ack.

>
> > --- a/arch/m68k/include/uapi/asm/bootinfo.h
> > +++ b/arch/m68k/include/uapi/asm/bootinfo.h
> > @@ -64,7 +64,13 @@ struct mem_info {
> > /* (struct mem_info) */
> > #define BI_COMMAND_LINE 0x0007 /* kernel command line parameters */
> > /* (string) */
> > -
> > +/*
> > + * A random seed used to initialize the RNG. Record format:
> > + *
> > + * - length [ 2 bytes, 16-bit big endian ]
> > + * - seed data [ `length` bytes, padded to preserve 2-byte alignment ]
>
> While experimenting, I noticed this must actually be a 4-byte alignment,
> as data[] is an integral multiple of __be32 words:
>
> struct bi_record {
> __be16 tag; /* tag ID */
> __be16 size; /* size of record (in bytes) */
> __be32 data[]; /* data */
> };
>
> m68kboot[1] and kexec-boot[2] do implement the correct rounding,
> but qemu's BOOTINFOSTR()[3] and BOOTINFODATA()[4] do not.
> Sorry for missing that before, I guess I was misled by the existing
> bug in BOOTINFOSTR().
>
> Consequently, both kexec-tools[5] and the m68k-bootinfo tool[6] cannot
> parse /proc/bootinfo if the size is not a multiple of 4, which can
> easily be triggered by changing the command line of the m68k virt
> machine.

In my brief experience in working with this structure, I've just made
sure that data[] is casted to (void*) or (u8*) or something, and then
everything is fine. But the tools you've linked to expect 4 byte
alignment. So, it sounds like what you're saying is that while we're at
it with fixing the constant, I should also make sure seed data preserves
4 byte alignment, and update the comment too? That's easy enough. I'll
do that for v+1.

Jason