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: <CAMuHMdXPcXq_F0dQdJ5nV2io1eDhJs7BZf+TmXWPqGVn9xeLNA@mail.gmail.com>
Date:   Mon, 26 Sep 2022 12:27:13 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
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 Jason,

On Fri, Sep 23, 2022 at 7:03 PM Jason A. Donenfeld <Jason@...c4.com> wrote:
> This is useful on !virt platforms for kexec, so change things from
> BI_VIRT_RNG_SEED to be BI_RNG_SEED, and simply remove BI_VIRT_RNG_SEED
> because it only ever lasted one release, and nothing is broken by not
> having it. At the same time, keep a comment noting that it's been
> removed, so that ID isn't reused.
>
> Suggested-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> Cc: Laurent Vivier <laurent@...ier.eu>
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>

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].

> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> @@ -13,13 +13,8 @@
>  #define BI_VIRT_VIRTIO_BASE    0x8004
>  #define BI_VIRT_CTRL_BASE      0x8005
>
> -/*
> - * 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 ]
> - */
> -#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.

> --- 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.

> + */
> +#define BI_RNG_SEED            0x0008
>
>      /*
>       *  Linux/m68k Architectures (BI_MACHTYPE)

[1] https://github.com/geertu/m68kboot/blob/master/common/bootinf.c#L171
[2] https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/m68k/bootinfo.c#n60
[3] https://git.qemu.org/?p=qemu.git;a=blob;f=hw/m68k/bootinfo.h;h=bd8b212fd35ce216917e07eb67783b5b69f1e11b;hb=HEAD#l57
[4] https://git.qemu.org/?p=qemu.git;a=blob;f=hw/m68k/bootinfo.h;h=bd8b212fd35ce216917e07eb67783b5b69f1e11b;hb=HEAD#l73
[5] https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/tree/kexec/arch/m68k/bootinfo.c#n106
[6] https://github.com/geertu/m68k-bootinfo/blob/master/m68k-bootinfo.c#L466

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds