linux-kernel - Re: [PATCH v3 3/3] m68k: rework BI_VIRT_RNG_SEED as BI_RNG

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YzGIO4IK9WXFyXsD@zx2c4.com>
Date:   Mon, 26 Sep 2022 13:08:43 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     linux-m68k@...ts.linux-m68k.org, linux-kernel@...r.kernel.org,
        Laurent Vivier <laurent@...ier.eu>
Subject: Re: [PATCH v3 3/3] m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED

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