[PATCH] hw/i386: pass RNG seed to e820 setup table

[PATCH] hw/i386: pass RNG seed to e820 setup table

[Jason A. Donenfeld]

Tiny machines optimized for fast boot time generally don't use EFI,
which means a random seed has to be supplied some other way, in this
case by the e820 setup table, which supplies a place for one. This
commit adds passing this random seed via the table. It is confirmed to
be working with the Linux patch in the link.

Link: https://lore.kernel.org/lkml/20220630113300.1892799-1-Jason@zx2c4.com/
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/i386/x86.c                                | 19 ++++++++++++++-----
 include/standard-headers/asm-x86/bootparam.h |  1 +
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6003b4b2df..0724759eec 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -26,6 +26,7 @@
 #include "qemu/cutils.h"
 #include "qemu/units.h"
 #include "qemu/datadir.h"
+#include "qemu/guest-random.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qapi-visit-common.h"
@@ -1045,6 +1046,16 @@ void x86_load_linux(X86MachineState *x86ms,
     }
     fclose(f);
 
+    setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
+    kernel_size = setup_data_offset + sizeof(struct setup_data) + 32;
+    kernel = g_realloc(kernel, kernel_size);
+    stq_p(header + 0x250, prot_addr + setup_data_offset);
+    setup_data = (struct setup_data *)(kernel + setup_data_offset);
+    setup_data->next = 0;
+    setup_data->type = cpu_to_le32(SETUP_RNG_SEED);
+    setup_data->len = cpu_to_le32(32);
+    qemu_guest_getrandom_nofail(setup_data->data, 32);
+
     /* append dtb to kernel */
     if (dtb_filename) {
         if (protocol < 0x209) {
@@ -1059,13 +1070,11 @@ void x86_load_linux(X86MachineState *x86ms,
             exit(1);
         }
 
-        setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
-        kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
+        kernel_size += sizeof(struct setup_data) + dtb_size;
         kernel = g_realloc(kernel, kernel_size);
 
-        stq_p(header + 0x250, prot_addr + setup_data_offset);
-
-        setup_data = (struct setup_data *)(kernel + setup_data_offset);
+        setup_data->next = prot_addr + setup_data_offset + sizeof(*setup_data) + setup_data->len;
+        ++setup_data;
         setup_data->next = 0;
         setup_data->type = cpu_to_le32(SETUP_DTB);
         setup_data->len = cpu_to_le32(dtb_size);
diff --git a/include/standard-headers/asm-x86/bootparam.h b/include/standard-headers/asm-x86/bootparam.h
index 072e2ed546..b8cb1fa313 100644
--- a/include/standard-headers/asm-x86/bootparam.h
+++ b/include/standard-headers/asm-x86/bootparam.h
@@ -10,6 +10,7 @@
 #define SETUP_EFI			4
 #define SETUP_APPLE_PROPERTIES		5
 #define SETUP_JAILHOUSE			6
+#define SETUP_RNG_SEED			8
 
 #define SETUP_INDIRECT			(1<<31)
 
-- 
2.35.1

[PATCH v2] hw/i386: pass RNG seed to e820 setup table

[Jason A. Donenfeld]

Tiny machines optimized for fast boot time generally don't use EFI,
which means a random seed has to be supplied some other way, in this
case by the e820 setup table, which supplies a place for one. This
commit adds passing this random seed via the table. It is confirmed to
be working with the Linux patch in the link.

Link: https://lore.kernel.org/lkml/20220708113907.891319-1-Jason@zx2c4.com/
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 hw/i386/x86.c                                | 19 ++++++++++++++-----
 include/standard-headers/asm-x86/bootparam.h |  1 +
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6003b4b2df..0724759eec 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -26,6 +26,7 @@
 #include "qemu/cutils.h"
 #include "qemu/units.h"
 #include "qemu/datadir.h"
+#include "qemu/guest-random.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qapi-visit-common.h"
@@ -1045,6 +1046,16 @@ void x86_load_linux(X86MachineState *x86ms,
     }
     fclose(f);
 
+    setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
+    kernel_size = setup_data_offset + sizeof(struct setup_data) + 32;
+    kernel = g_realloc(kernel, kernel_size);
+    stq_p(header + 0x250, prot_addr + setup_data_offset);
+    setup_data = (struct setup_data *)(kernel + setup_data_offset);
+    setup_data->next = 0;
+    setup_data->type = cpu_to_le32(SETUP_RNG_SEED);
+    setup_data->len = cpu_to_le32(32);
+    qemu_guest_getrandom_nofail(setup_data->data, 32);
+
     /* append dtb to kernel */
     if (dtb_filename) {
         if (protocol < 0x209) {
@@ -1059,13 +1070,11 @@ void x86_load_linux(X86MachineState *x86ms,
             exit(1);
         }
 
-        setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
-        kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
+        kernel_size += sizeof(struct setup_data) + dtb_size;
         kernel = g_realloc(kernel, kernel_size);
 
-        stq_p(header + 0x250, prot_addr + setup_data_offset);
-
-        setup_data = (struct setup_data *)(kernel + setup_data_offset);
+        setup_data->next = prot_addr + setup_data_offset + sizeof(*setup_data) + setup_data->len;
+        ++setup_data;
         setup_data->next = 0;
         setup_data->type = cpu_to_le32(SETUP_DTB);
         setup_data->len = cpu_to_le32(dtb_size);
diff --git a/include/standard-headers/asm-x86/bootparam.h b/include/standard-headers/asm-x86/bootparam.h
index 072e2ed546..b2aaad10e5 100644
--- a/include/standard-headers/asm-x86/bootparam.h
+++ b/include/standard-headers/asm-x86/bootparam.h
@@ -10,6 +10,7 @@
 #define SETUP_EFI			4
 #define SETUP_APPLE_PROPERTIES		5
 #define SETUP_JAILHOUSE			6
+#define SETUP_RNG_SEED			9
 
 #define SETUP_INDIRECT			(1<<31)
 
-- 
2.35.1

Re: [PATCH] hw/i386: pass RNG seed to e820 setup table

[Daniel P. Berrangé]

On Thu, Jun 30, 2022 at 01:37:17PM +0200, Jason A. Donenfeld wrote:
> Tiny machines optimized for fast boot time generally don't use EFI,
> which means a random seed has to be supplied some other way, in this
> case by the e820 setup table, which supplies a place for one. This
> commit adds passing this random seed via the table. It is confirmed to
> be working with the Linux patch in the link.

IIUC, this approach will only expose the random seed when QEMU
is booted using -kernel + -initrd args.

I agree with what you say about most VMs not using UEFI right now.
I'd say the majority of general purpose VMs are using SeaBIOS
still. The usage of -kernel + -initrd, is typically for more
specialized use cases. 

IOW, exposing random seed via the setup table feels like it'll
have a somewhat limited benefit.

Can we get an approach that exposes a random seed regardless of
whether using -kernel, or seabios, or uefi, or $whatever firmware ?

Perhaps (ab)use 'fw_cfg', which is exposed for any x86 VM no matter
what config it has for booting ?

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] hw/i386: pass RNG seed to e820 setup table

[Jason A. Donenfeld]

Hi Daniel,

On Fri, Jul 8, 2022 at 2:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Jun 30, 2022 at 01:37:17PM +0200, Jason A. Donenfeld wrote:
> > Tiny machines optimized for fast boot time generally don't use EFI,
> > which means a random seed has to be supplied some other way, in this
> > case by the e820 setup table, which supplies a place for one. This
> > commit adds passing this random seed via the table. It is confirmed to
> > be working with the Linux patch in the link.
>
> IIUC, this approach will only expose the random seed when QEMU
> is booted using -kernel + -initrd args.
>
> I agree with what you say about most VMs not using UEFI right now.
> I'd say the majority of general purpose VMs are using SeaBIOS
> still. The usage of -kernel + -initrd, is typically for more
> specialized use cases.

Highly disagree, based on seeing a lot of real world deployment.
Furthermore, this is going to be used within Linux itself for kexec,
so it makes sense to use it here too.

> Can we get an approach that exposes a random seed regardless of
> whether using -kernel, or seabios, or uefi, or $whatever firmware ?

No.

> Perhaps (ab)use 'fw_cfg', which is exposed for any x86 VM no matter
> what config it has for booting ?

That approach is super messy and doesn't work. I've already gone down
that route.

The entire point here is to include the seed on this part of the boot
protocol. There might be other opportunities for doing it elsewhere.
For example, EFI already has a thing.

Please don't sink a good idea because it doesn't handle every possible
use case. That type of mentality is just going to result in nothing
ever getting done anywhere, making a decades old problem last for
another decade. This patch here is simple and makes a tangible
incremental advance toward something good, and fits the pattern of how
it's done on all other platforms.

Thanks,
Jason

Re: [PATCH] hw/i386: pass RNG seed to e820 setup table

[Daniel P. Berrangé]

On Fri, Jul 08, 2022 at 02:04:40PM +0200, Jason A. Donenfeld wrote:
> Hi Daniel,
> 
> On Fri, Jul 8, 2022 at 2:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Thu, Jun 30, 2022 at 01:37:17PM +0200, Jason A. Donenfeld wrote:
> > > Tiny machines optimized for fast boot time generally don't use EFI,
> > > which means a random seed has to be supplied some other way, in this
> > > case by the e820 setup table, which supplies a place for one. This
> > > commit adds passing this random seed via the table. It is confirmed to
> > > be working with the Linux patch in the link.
> >
> > IIUC, this approach will only expose the random seed when QEMU
> > is booted using -kernel + -initrd args.
> >
> > I agree with what you say about most VMs not using UEFI right now.
> > I'd say the majority of general purpose VMs are using SeaBIOS
> > still. The usage of -kernel + -initrd, is typically for more
> > specialized use cases.
> 
> Highly disagree, based on seeing a lot of real world deployment.

I guess we're looking at different places then, as all of the large
scale virt mgmt apps I've experianced with KVM (OpenStack, oVirt,
KubeVirt), along with the small scale ones (GNOME Boxes, virt-manager,
virt-install, Cockpit), etc all primarily use SeaBIOS, and in more
recently times a bit of UEFI.  Direct kernel/initrd boot is usualy
reserved for special cases, since users like to be able to manage
their kernel/initrd inside the guest image.

> Furthermore, this is going to be used within Linux itself for kexec,
> so it makes sense to use it here too.

Ok, useful info.

> > Can we get an approach that exposes a random seed regardless of
> > whether using -kernel, or seabios, or uefi, or $whatever firmware ?
> 
> No.
> 
> > Perhaps (ab)use 'fw_cfg', which is exposed for any x86 VM no matter
> > what config it has for booting ?
> 
> That approach is super messy and doesn't work. I've already gone down
> that route.

What's the problem with it ? fw_cfg is a pretty straightforward
mechanism for injecting data into the guest OS, that we already
use for alot of stuff.

> The entire point here is to include the seed on this part of the boot
> protocol. There might be other opportunities for doing it elsewhere.
> For example, EFI already has a thing.
> 
> Please don't sink a good idea because it doesn't handle every possible
> use case. That type of mentality is just going to result in nothing
> ever getting done anywhere, making a decades old problem last for
> another decade. This patch here is simple and makes a tangible
> incremental advance toward something good, and fits the pattern of how
> it's done on all other platforms.

I'm not trying to sink an idea. If this turns out to be the best
idea, I've no problem with that.

I merely asked some reasonable questions about whether there were
alternative approaches that could solve more broadly useful scenarios,
given the narrow usage of direct kernel boot, in context of the common
VM deployments I've seen at large scale. You can't expect reviewers to
blindly accept any proposal without considering it broader context.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|