[PATCH] m68k: virt: pass RNG seed via bootinfo block

[PATCH] m68k: virt: pass RNG seed via bootinfo block

[Jason A. Donenfeld]

Other virt VMs can pass RNG seeds via the "rng-seed" device tree
property or via UEFI, but m68k doesn't have either. Instead it has its
own bootinfo protocol. So this commit adds support for receiving a RNG
seed from it, which will be used at the earliest possible time in boot,
just like device tree.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
 arch/m68k/virt/config.c                    | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
index e4db7e2213ab..7c3044acdf4a 100644
--- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
+++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
@@ -12,6 +12,7 @@
 #define BI_VIRT_GF_TTY_BASE	0x8003
 #define BI_VIRT_VIRTIO_BASE	0x8004
 #define BI_VIRT_CTRL_BASE	0x8005
+#define BI_VIRT_RNG_SEED	0x8006
 
 #define VIRT_BOOTI_VERSION	MK_BI_VERSION(2, 0)
 
diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
index 632ba200ad42..ad71af8273ec 100644
--- a/arch/m68k/virt/config.c
+++ b/arch/m68k/virt/config.c
@@ -2,6 +2,7 @@
 
 #include <linux/reboot.h>
 #include <linux/serial_core.h>
+#include <linux/random.h>
 #include <clocksource/timer-goldfish.h>
 
 #include <asm/bootinfo.h>
@@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
 		data += 4;
 		virt_bi_data.virtio.irq = be32_to_cpup(data);
 		break;
+	case BI_VIRT_RNG_SEED:
+		add_bootloader_randomness(data + 4, be32_to_cpup(data));
+		break;
 	default:
 		unknown = 1;
 		break;
-- 
2.35.1

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Laurent Vivier]

Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
> Other virt VMs can pass RNG seeds via the "rng-seed" device tree
> property or via UEFI, but m68k doesn't have either. Instead it has its
> own bootinfo protocol. So this commit adds support for receiving a RNG
> seed from it, which will be used at the earliest possible time in boot,
> just like device tree.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
>   arch/m68k/virt/config.c                    | 4 ++++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> index e4db7e2213ab..7c3044acdf4a 100644
> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> @@ -12,6 +12,7 @@
>   #define BI_VIRT_GF_TTY_BASE	0x8003
>   #define BI_VIRT_VIRTIO_BASE	0x8004
>   #define BI_VIRT_CTRL_BASE	0x8005
> +#define BI_VIRT_RNG_SEED	0x8006
>   
>   #define VIRT_BOOTI_VERSION	MK_BI_VERSION(2, 0)
>   
> diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
> index 632ba200ad42..ad71af8273ec 100644
> --- a/arch/m68k/virt/config.c
> +++ b/arch/m68k/virt/config.c
> @@ -2,6 +2,7 @@
>   
>   #include <linux/reboot.h>
>   #include <linux/serial_core.h>
> +#include <linux/random.h>
>   #include <clocksource/timer-goldfish.h>
>   
>   #include <asm/bootinfo.h>
> @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
>   		data += 4;
>   		virt_bi_data.virtio.irq = be32_to_cpup(data);
>   		break;
> +	case BI_VIRT_RNG_SEED:
> +		add_bootloader_randomness(data + 4, be32_to_cpup(data));
> +		break;
>   	default:
>   		unknown = 1;
>   		break;

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Laurent Vivier]

Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
> Other virt VMs can pass RNG seeds via the "rng-seed" device tree
> property or via UEFI, but m68k doesn't have either. Instead it has its
> own bootinfo protocol. So this commit adds support for receiving a RNG
> seed from it, which will be used at the earliest possible time in boot,
> just like device tree.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
>   arch/m68k/virt/config.c                    | 4 ++++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> index e4db7e2213ab..7c3044acdf4a 100644
> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> @@ -12,6 +12,7 @@
>   #define BI_VIRT_GF_TTY_BASE	0x8003
>   #define BI_VIRT_VIRTIO_BASE	0x8004
>   #define BI_VIRT_CTRL_BASE	0x8005
> +#define BI_VIRT_RNG_SEED	0x8006
>   
>   #define VIRT_BOOTI_VERSION	MK_BI_VERSION(2, 0)
>   
> diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
> index 632ba200ad42..ad71af8273ec 100644
> --- a/arch/m68k/virt/config.c
> +++ b/arch/m68k/virt/config.c
> @@ -2,6 +2,7 @@
>   
>   #include <linux/reboot.h>
>   #include <linux/serial_core.h>
> +#include <linux/random.h>
>   #include <clocksource/timer-goldfish.h>
>   
>   #include <asm/bootinfo.h>
> @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
>   		data += 4;
>   		virt_bi_data.virtio.irq = be32_to_cpup(data);
>   		break;
> +	case BI_VIRT_RNG_SEED:
> +		add_bootloader_randomness(data + 4, be32_to_cpup(data));

In fact, why don't you use the record->size to get the size of the buffer?

It seems useless to encode twice the length of the buffer, the second time on a 32bit while the 
length cannot exceed a 16bit value.

Thanks,
Laurent

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Jason A. Donenfeld]

On Sat, Jun 25, 2022 at 6:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
> > Other virt VMs can pass RNG seeds via the "rng-seed" device tree
> > property or via UEFI, but m68k doesn't have either. Instead it has its
> > own bootinfo protocol. So this commit adds support for receiving a RNG
> > seed from it, which will be used at the earliest possible time in boot,
> > just like device tree.
> >
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> >   arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
> >   arch/m68k/virt/config.c                    | 4 ++++
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > index e4db7e2213ab..7c3044acdf4a 100644
> > --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > @@ -12,6 +12,7 @@
> >   #define BI_VIRT_GF_TTY_BASE 0x8003
> >   #define BI_VIRT_VIRTIO_BASE 0x8004
> >   #define BI_VIRT_CTRL_BASE   0x8005
> > +#define BI_VIRT_RNG_SEED     0x8006
> >
> >   #define VIRT_BOOTI_VERSION  MK_BI_VERSION(2, 0)
> >
> > diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
> > index 632ba200ad42..ad71af8273ec 100644
> > --- a/arch/m68k/virt/config.c
> > +++ b/arch/m68k/virt/config.c
> > @@ -2,6 +2,7 @@
> >
> >   #include <linux/reboot.h>
> >   #include <linux/serial_core.h>
> > +#include <linux/random.h>
> >   #include <clocksource/timer-goldfish.h>
> >
> >   #include <asm/bootinfo.h>
> > @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
> >               data += 4;
> >               virt_bi_data.virtio.irq = be32_to_cpup(data);
> >               break;
> > +     case BI_VIRT_RNG_SEED:
> > +             add_bootloader_randomness(data + 4, be32_to_cpup(data));
>
> In fact, why don't you use the record->size to get the size of the buffer?
>
> It seems useless to encode twice the length of the buffer, the second time on a 32bit while the
> length cannot exceed a 16bit value.

Doesn't that make the length ambiguous because of required alignment?
Would rather keep this general. As is, it's also much more like the
others and more uniform to keep it that way. You were able to review
it and see that it was right after glancing for a second. That seems
superior to any imaginary gains we'd get by overloading the record
size.

Jason

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Laurent Vivier]

Le 25/06/2022 à 18:19, Jason A. Donenfeld a écrit :
> On Sat, Jun 25, 2022 at 6:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
>>> Other virt VMs can pass RNG seeds via the "rng-seed" device tree
>>> property or via UEFI, but m68k doesn't have either. Instead it has its
>>> own bootinfo protocol. So this commit adds support for receiving a RNG
>>> seed from it, which will be used at the earliest possible time in boot,
>>> just like device tree.
>>>
>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>>> ---
>>>    arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
>>>    arch/m68k/virt/config.c                    | 4 ++++
>>>    2 files changed, 5 insertions(+)
>>>
>>> diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
>>> index e4db7e2213ab..7c3044acdf4a 100644
>>> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
>>> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
>>> @@ -12,6 +12,7 @@
>>>    #define BI_VIRT_GF_TTY_BASE 0x8003
>>>    #define BI_VIRT_VIRTIO_BASE 0x8004
>>>    #define BI_VIRT_CTRL_BASE   0x8005
>>> +#define BI_VIRT_RNG_SEED     0x8006
>>>
>>>    #define VIRT_BOOTI_VERSION  MK_BI_VERSION(2, 0)
>>>
>>> diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
>>> index 632ba200ad42..ad71af8273ec 100644
>>> --- a/arch/m68k/virt/config.c
>>> +++ b/arch/m68k/virt/config.c
>>> @@ -2,6 +2,7 @@
>>>
>>>    #include <linux/reboot.h>
>>>    #include <linux/serial_core.h>
>>> +#include <linux/random.h>
>>>    #include <clocksource/timer-goldfish.h>
>>>
>>>    #include <asm/bootinfo.h>
>>> @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
>>>                data += 4;
>>>                virt_bi_data.virtio.irq = be32_to_cpup(data);
>>>                break;
>>> +     case BI_VIRT_RNG_SEED:
>>> +             add_bootloader_randomness(data + 4, be32_to_cpup(data));
>>
>> In fact, why don't you use the record->size to get the size of the buffer?
>>
>> It seems useless to encode twice the length of the buffer, the second time on a 32bit while the
>> length cannot exceed a 16bit value.
> 
> Doesn't that make the length ambiguous because of required alignment?

I agree, it's why I understand reviewing the QEMU part of your patch.

> Would rather keep this general. As is, it's also much more like the
> others and more uniform to keep it that way. You were able to review
> it and see that it was right after glancing for a second. That seems
> superior to any imaginary gains we'd get by overloading the record
> size.

And what about using a 16bit field rather than a 32bit field as the encoded length cannot be greater 
than the record length?

Thanks,
Laurent

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Jason A. Donenfeld]

On Sat, Jun 25, 2022 at 6:24 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 25/06/2022 à 18:19, Jason A. Donenfeld a écrit :
> > On Sat, Jun 25, 2022 at 6:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
> >>
> >> Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
> >>> Other virt VMs can pass RNG seeds via the "rng-seed" device tree
> >>> property or via UEFI, but m68k doesn't have either. Instead it has its
> >>> own bootinfo protocol. So this commit adds support for receiving a RNG
> >>> seed from it, which will be used at the earliest possible time in boot,
> >>> just like device tree.
> >>>
> >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> >>> ---
> >>>    arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
> >>>    arch/m68k/virt/config.c                    | 4 ++++
> >>>    2 files changed, 5 insertions(+)
> >>>
> >>> diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> >>> index e4db7e2213ab..7c3044acdf4a 100644
> >>> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> >>> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> >>> @@ -12,6 +12,7 @@
> >>>    #define BI_VIRT_GF_TTY_BASE 0x8003
> >>>    #define BI_VIRT_VIRTIO_BASE 0x8004
> >>>    #define BI_VIRT_CTRL_BASE   0x8005
> >>> +#define BI_VIRT_RNG_SEED     0x8006
> >>>
> >>>    #define VIRT_BOOTI_VERSION  MK_BI_VERSION(2, 0)
> >>>
> >>> diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
> >>> index 632ba200ad42..ad71af8273ec 100644
> >>> --- a/arch/m68k/virt/config.c
> >>> +++ b/arch/m68k/virt/config.c
> >>> @@ -2,6 +2,7 @@
> >>>
> >>>    #include <linux/reboot.h>
> >>>    #include <linux/serial_core.h>
> >>> +#include <linux/random.h>
> >>>    #include <clocksource/timer-goldfish.h>
> >>>
> >>>    #include <asm/bootinfo.h>
> >>> @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
> >>>                data += 4;
> >>>                virt_bi_data.virtio.irq = be32_to_cpup(data);
> >>>                break;
> >>> +     case BI_VIRT_RNG_SEED:
> >>> +             add_bootloader_randomness(data + 4, be32_to_cpup(data));
> >>
> >> In fact, why don't you use the record->size to get the size of the buffer?
> >>
> >> It seems useless to encode twice the length of the buffer, the second time on a 32bit while the
> >> length cannot exceed a 16bit value.
> >
> > Doesn't that make the length ambiguous because of required alignment?
>
> I agree, it's why I understand reviewing the QEMU part of your patch.
>
> > Would rather keep this general. As is, it's also much more like the
> > others and more uniform to keep it that way. You were able to review
> > it and see that it was right after glancing for a second. That seems
> > superior to any imaginary gains we'd get by overloading the record
> > size.
>
> And what about using a 16bit field rather than a 32bit field as the encoded length cannot be greater
> than the record length?

I guess but that's different from all other length fields, and means
we can't expand past 65k if somebody wants to use this for something
more interesting later. Again I wonder what stinginess here gets us.
This is just a boot parameter... No need to go crazy optimizing it.

Jason

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Laurent Vivier]

Le 25/06/2022 à 18:26, Jason A. Donenfeld a écrit :
> On Sat, Jun 25, 2022 at 6:24 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 25/06/2022 à 18:19, Jason A. Donenfeld a écrit :
>>> On Sat, Jun 25, 2022 at 6:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>>>
>>>> Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
>>>>> Other virt VMs can pass RNG seeds via the "rng-seed" device tree
>>>>> property or via UEFI, but m68k doesn't have either. Instead it has its
>>>>> own bootinfo protocol. So this commit adds support for receiving a RNG
>>>>> seed from it, which will be used at the earliest possible time in boot,
>>>>> just like device tree.
>>>>>
>>>>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>>>>> ---
>>>>>     arch/m68k/include/uapi/asm/bootinfo-virt.h | 1 +
>>>>>     arch/m68k/virt/config.c                    | 4 ++++
>>>>>     2 files changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
>>>>> index e4db7e2213ab..7c3044acdf4a 100644
>>>>> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
>>>>> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
>>>>> @@ -12,6 +12,7 @@
>>>>>     #define BI_VIRT_GF_TTY_BASE 0x8003
>>>>>     #define BI_VIRT_VIRTIO_BASE 0x8004
>>>>>     #define BI_VIRT_CTRL_BASE   0x8005
>>>>> +#define BI_VIRT_RNG_SEED     0x8006
>>>>>
>>>>>     #define VIRT_BOOTI_VERSION  MK_BI_VERSION(2, 0)
>>>>>
>>>>> diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
>>>>> index 632ba200ad42..ad71af8273ec 100644
>>>>> --- a/arch/m68k/virt/config.c
>>>>> +++ b/arch/m68k/virt/config.c
>>>>> @@ -2,6 +2,7 @@
>>>>>
>>>>>     #include <linux/reboot.h>
>>>>>     #include <linux/serial_core.h>
>>>>> +#include <linux/random.h>
>>>>>     #include <clocksource/timer-goldfish.h>
>>>>>
>>>>>     #include <asm/bootinfo.h>
>>>>> @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
>>>>>                 data += 4;
>>>>>                 virt_bi_data.virtio.irq = be32_to_cpup(data);
>>>>>                 break;
>>>>> +     case BI_VIRT_RNG_SEED:
>>>>> +             add_bootloader_randomness(data + 4, be32_to_cpup(data));
>>>>
>>>> In fact, why don't you use the record->size to get the size of the buffer?
>>>>
>>>> It seems useless to encode twice the length of the buffer, the second time on a 32bit while the
>>>> length cannot exceed a 16bit value.
>>>
>>> Doesn't that make the length ambiguous because of required alignment?
>>
>> I agree, it's why I understand reviewing the QEMU part of your patch.
>>
>>> Would rather keep this general. As is, it's also much more like the
>>> others and more uniform to keep it that way. You were able to review
>>> it and see that it was right after glancing for a second. That seems
>>> superior to any imaginary gains we'd get by overloading the record
>>> size.
>>
>> And what about using a 16bit field rather than a 32bit field as the encoded length cannot be greater
>> than the record length?
> 
> I guess but that's different from all other length fields, and means
> we can't expand past 65k if somebody wants to use this for something
> more interesting later. Again I wonder what stinginess here gets us.
> This is just a boot parameter... No need to go crazy optimizing it.

I agree too.

Thanks,
Laurent

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Geert Uytterhoeven]

Hi Jason,

Thanks for your patch!

On Sat, Jun 25, 2022 at 6:26 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> On Sat, Jun 25, 2022 at 6:24 PM Laurent Vivier <laurent@vivier.eu> wrote:
> > Le 25/06/2022 à 18:19, Jason A. Donenfeld a écrit :
> > > On Sat, Jun 25, 2022 at 6:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
> > >> Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
> > >>> Other virt VMs can pass RNG seeds via the "rng-seed" device tree

FTR, "rng-seed" does not seem to be documented anywhere, not under
Documentation/devicetree/bindings/, and not in the Devicetree
Specification?

> > >>> property or via UEFI, but m68k doesn't have either. Instead it has its
> > >>> own bootinfo protocol. So this commit adds support for receiving a RNG
> > >>> seed from it, which will be used at the earliest possible time in boot,
> > >>> just like device tree.
> > >>>
> > >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > >>> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > >>> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > >>> @@ -12,6 +12,7 @@
> > >>>    #define BI_VIRT_GF_TTY_BASE 0x8003
> > >>>    #define BI_VIRT_VIRTIO_BASE 0x8004
> > >>>    #define BI_VIRT_CTRL_BASE   0x8005
> > >>> +#define BI_VIRT_RNG_SEED     0x8006

Please add a comment documenting the record format.

Laurent: Would be nice if you could add this for the other BI_*, too.

> > >>>
> > >>>    #define VIRT_BOOTI_VERSION  MK_BI_VERSION(2, 0)
> > >>>
> > >>> diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
> > >>> index 632ba200ad42..ad71af8273ec 100644
> > >>> --- a/arch/m68k/virt/config.c
> > >>> +++ b/arch/m68k/virt/config.c
> > >>> @@ -2,6 +2,7 @@
> > >>>
> > >>>    #include <linux/reboot.h>
> > >>>    #include <linux/serial_core.h>
> > >>> +#include <linux/random.h>
> > >>>    #include <clocksource/timer-goldfish.h>
> > >>>
> > >>>    #include <asm/bootinfo.h>
> > >>> @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
> > >>>                data += 4;
> > >>>                virt_bi_data.virtio.irq = be32_to_cpup(data);
> > >>>                break;
> > >>> +     case BI_VIRT_RNG_SEED:
> > >>> +             add_bootloader_randomness(data + 4, be32_to_cpup(data));
> > >>
> > >> In fact, why don't you use the record->size to get the size of the buffer?
> > >>
> > >> It seems useless to encode twice the length of the buffer, the second time on a 32bit while the
> > >> length cannot exceed a 16bit value.
> > >
> > > Doesn't that make the length ambiguous because of required alignment?
> >
> > I agree, it's why I understand reviewing the QEMU part of your patch.
> >
> > > Would rather keep this general. As is, it's also much more like the
> > > others and more uniform to keep it that way. You were able to review
> > > it and see that it was right after glancing for a second. That seems
> > > superior to any imaginary gains we'd get by overloading the record
> > > size.
> >
> > And what about using a 16bit field rather than a 32bit field as the encoded length cannot be greater
> > than the record length?
>
> I guess but that's different from all other length fields, and means
> we can't expand past 65k if somebody wants to use this for something
> more interesting later. Again I wonder what stinginess here gets us.
> This is just a boot parameter... No need to go crazy optimizing it.

You cannot extend this past (64 KiB - sizeof(struct bi_record))
anyway, as the total record size is limited to 64 KiB, regardless of
the additional buffer size you try to encode inside your own 32-bit
size field.

So either just store the data inside the record, rely on bi_record.size,
and live with random data that must be a number of even bytes (does
it really hurt to drop the last byte, or add a dummy byte?), or store
a pointer/size, like is done for e.g. BI_RAMDISK.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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

Re: [PATCH] m68k: virt: pass RNG seed via bootinfo block

[Jason A. Donenfeld]

Hi Geert,

On Sun, Jun 26, 2022 at 11:39:46AM +0200, Geert Uytterhoeven wrote:
> Hi Jason,
> 
> Thanks for your patch!
> 
> On Sat, Jun 25, 2022 at 6:26 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > On Sat, Jun 25, 2022 at 6:24 PM Laurent Vivier <laurent@vivier.eu> wrote:
> > > Le 25/06/2022 à 18:19, Jason A. Donenfeld a écrit :
> > > > On Sat, Jun 25, 2022 at 6:08 PM Laurent Vivier <laurent@vivier.eu> wrote:
> > > >> Le 25/06/2022 à 17:38, Jason A. Donenfeld a écrit :
> > > >>> Other virt VMs can pass RNG seeds via the "rng-seed" device tree
> 
> FTR, "rng-seed" does not seem to be documented anywhere, not under
> Documentation/devicetree/bindings/, and not in the Devicetree
> Specification?

Good point. It's quite old, this field, so odd it was missed. I'll send
in a separate patch for that.

> 
> > > >>> property or via UEFI, but m68k doesn't have either. Instead it has its
> > > >>> own bootinfo protocol. So this commit adds support for receiving a RNG
> > > >>> seed from it, which will be used at the earliest possible time in boot,
> > > >>> just like device tree.
> > > >>>
> > > >>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > > >>> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > > >>> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> > > >>> @@ -12,6 +12,7 @@
> > > >>>    #define BI_VIRT_GF_TTY_BASE 0x8003
> > > >>>    #define BI_VIRT_VIRTIO_BASE 0x8004
> > > >>>    #define BI_VIRT_CTRL_BASE   0x8005
> > > >>> +#define BI_VIRT_RNG_SEED     0x8006
> 
> Please add a comment documenting the record format.

Ack.

> 
> Laurent: Would be nice if you could add this for the other BI_*, too.
> 
> > > >>>
> > > >>>    #define VIRT_BOOTI_VERSION  MK_BI_VERSION(2, 0)
> > > >>>
> > > >>> diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
> > > >>> index 632ba200ad42..ad71af8273ec 100644
> > > >>> --- a/arch/m68k/virt/config.c
> > > >>> +++ b/arch/m68k/virt/config.c
> > > >>> @@ -2,6 +2,7 @@
> > > >>>
> > > >>>    #include <linux/reboot.h>
> > > >>>    #include <linux/serial_core.h>
> > > >>> +#include <linux/random.h>
> > > >>>    #include <clocksource/timer-goldfish.h>
> > > >>>
> > > >>>    #include <asm/bootinfo.h>
> > > >>> @@ -92,6 +93,9 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
> > > >>>                data += 4;
> > > >>>                virt_bi_data.virtio.irq = be32_to_cpup(data);
> > > >>>                break;
> > > >>> +     case BI_VIRT_RNG_SEED:
> > > >>> +             add_bootloader_randomness(data + 4, be32_to_cpup(data));
> > > >>
> > > >> In fact, why don't you use the record->size to get the size of the buffer?
> > > >>
> > > >> It seems useless to encode twice the length of the buffer, the second time on a 32bit while the
> > > >> length cannot exceed a 16bit value.
> > > >
> > > > Doesn't that make the length ambiguous because of required alignment?
> > >
> > > I agree, it's why I understand reviewing the QEMU part of your patch.
> > >
> > > > Would rather keep this general. As is, it's also much more like the
> > > > others and more uniform to keep it that way. You were able to review
> > > > it and see that it was right after glancing for a second. That seems
> > > > superior to any imaginary gains we'd get by overloading the record
> > > > size.
> > >
> > > And what about using a 16bit field rather than a 32bit field as the encoded length cannot be greater
> > > than the record length?
> >
> > I guess but that's different from all other length fields, and means
> > we can't expand past 65k if somebody wants to use this for something
> > more interesting later. Again I wonder what stinginess here gets us.
> > This is just a boot parameter... No need to go crazy optimizing it.
> 
> You cannot extend this past (64 KiB - sizeof(struct bi_record))
> anyway, as the total record size is limited to 64 KiB, regardless of
> the additional buffer size you try to encode inside your own 32-bit
> size field.
> 
> So either just store the data inside the record, rely on bi_record.size,
> and live with random data that must be a number of even bytes (does
> it really hurt to drop the last byte, or add a dummy byte?), or store
> a pointer/size, like is done for e.g. BI_RAMDISK.

I modeled this on BOOTINFOSTR, which benefits from null termination.
I'll just reduce the length field to 2 bytes. I really don't want to
play padding games here, and anyway the length field needs to be
separate for reasons that will become apparent in v2 (zeroing for
kexec).

Jason

[PATCH v2] m68k: virt: use RNG seed from bootinfo block

[Jason A. Donenfeld]

Other virt VMs can pass RNG seeds via the "rng-seed" device tree
property or via UEFI, but m68k doesn't have either. Instead it has its
own bootinfo protocol. So this commit adds support for receiving a RNG
seed from it, which will be used at the earliest possible time in boot,
just like device tree.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/m68k/include/uapi/asm/bootinfo-virt.h | 7 +++++++
 arch/m68k/virt/config.c                    | 9 +++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
index e4db7e2213ab..0cb2c2a41610 100644
--- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
+++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
@@ -13,6 +13,13 @@
 #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 ]
+ */
+#define BI_VIRT_RNG_SEED	0x8006
+
 #define VIRT_BOOTI_VERSION	MK_BI_VERSION(2, 0)
 
 #endif /* _UAPI_ASM_M68K_BOOTINFO_MAC_H */
diff --git a/arch/m68k/virt/config.c b/arch/m68k/virt/config.c
index 632ba200ad42..645acc6918b2 100644
--- a/arch/m68k/virt/config.c
+++ b/arch/m68k/virt/config.c
@@ -2,6 +2,7 @@
 
 #include <linux/reboot.h>
 #include <linux/serial_core.h>
+#include <linux/random.h>
 #include <clocksource/timer-goldfish.h>
 
 #include <asm/bootinfo.h>
@@ -92,6 +93,14 @@ int __init virt_parse_bootinfo(const struct bi_record *record)
 		data += 4;
 		virt_bi_data.virtio.irq = be32_to_cpup(data);
 		break;
+	case BI_VIRT_RNG_SEED: {
+		u16 len = be16_to_cpup(data);
+		add_bootloader_randomness(data + 2, len);
+		/* Zero the data to preserve forward secrecy, and zero the
+		 * length to prevent kexec from using it. */
+		memzero_explicit((void *)data, len + 2);
+		break;
+	}
 	default:
 		unknown = 1;
 		break;
-- 
2.35.1

Re: [PATCH v2] m68k: virt: use RNG seed from bootinfo block

[Geert Uytterhoeven]

On Sun, Jun 26, 2022 at 1:15 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Other virt VMs can pass RNG seeds via the "rng-seed" device tree
> property or via UEFI, but m68k doesn't have either. Instead it has its
> own bootinfo protocol. So this commit adds support for receiving a RNG
> seed from it, which will be used at the earliest possible time in boot,
> just like device tree.
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

> --- a/arch/m68k/include/uapi/asm/bootinfo-virt.h
> +++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
> @@ -13,6 +13,13 @@
>  #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"

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
i.e. will queue in the m68k for-v5.20 branch with the above fixed.
No need to resend.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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