On Mon, Jan 23, 2023 at 06:37:21AM -0600, Jason A. Donenfeld wrote:
> On Mon, Jan 23, 2023 at 6:12 AM Michael S. Tsirkin <m...@redhat.com> wrote:
> >
> > On Sun, Jan 22, 2023 at 08:21:30PM -0800, Eric Biggers wrote:
> > > Hi Michael,
> > >
> > > On Tue, Jan 10, 2023 at 12:50:42PM -0500, Michael S. Tsirkin wrote:
> > > > On Tue, Jan 10, 2023 at 04:34:49PM +0100, Jason A. Donenfeld wrote:
> > > > > Hi Michael,
> > > > >
> > > > > Could you queue up this patch and mark it as a fix for 7.2.1? It is a
> > > > > straight-up bug fix for a 7.2 regression that's now affected several
> > > > > users.
> > > >
> > > > OK. In the future pls cc me if you want me to merge a patch. Thanks!
> > > >
> > > > > - It has two Tested-by tags on the thread.
> > > > > - hpa, the maintainer of the kernel side of this, confirmed on one of
> > > > > the various tributary threads that this approach is a correct one.
> > > > > - It doesn't introduce any new functionality.
> > > > >
> > > > > For your convenience, you can grab this out of lore here:
> > > > >
> > > > >
> > > > > https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/
> > > > >
> > > > > Or if you want to yolo it:
> > > > >
> > > > > curl
> > > > > https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/raw
> > > > > | git am -s
> > > > >
> > > > > It's now sat silent on the mailing list for a while. So let's please
> > > > > get
> > > > > this committed and backported so that the bug reports stop coming in.
> > > > >
> > >
> > > This patch still isn't on QEMU's master branch. What happened to it?
> > >
> > > - Eric
> >
> > Indeed though I remember picking it up. Tagged again now. Thanks!
>
> Thanks. What branch is this in? I didn't see it on:
> https://gitlab.com/mstredhat/qemu/-/branches/active
> https://github.com/mstsirkin/qemu/branches
I don't use github really. And it was not pushed to gitlab as I was
figuring out issues with other patches before starting CI as CI minutes
are limited. BTW as checkpatch was unhappy I applied a fixup -
making checkpatch happier and in the process the code change a bit
smaller. If you want to do cleanups on top be my guest but pls
make it pass checkpatch. Thanks!
commit a00d99e04c4481fca3ee2d7c40d42993b7b059c2
Author: Michael S. Tsirkin <m...@redhat.com>
Date: Sat Jan 28 06:08:43 2023 -0500
fixup! x86: don't let decompressed kernel image clobber setup_data
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 1b19d28c02..29f30dd6d3 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -378,7 +378,7 @@ static void microvm_fix_kernel_cmdline(MachineState
*machine)
MicrovmMachineState *mms = MICROVM_MACHINE(machine);
BusState *bus;
BusChild *kid;
- char *cmdline, *existing_cmdline = fw_cfg_read_bytes_ptr(x86ms->fw_cfg,
FW_CFG_CMDLINE_DATA);
+ char *cmdline, *existing_cmdline;
size_t len;
/*
@@ -388,6 +388,7 @@ static void microvm_fix_kernel_cmdline(MachineState
*machine)
* Yes, this is a hack, but one that heavily improves the UX without
* introducing any significant issues.
*/
+ existing_cmdline = fw_cfg_read_bytes_ptr(x86ms->fw_cfg,
FW_CFG_CMDLINE_DATA);
cmdline = g_strdup(existing_cmdline);
bus = sysbus_get_default();
QTAILQ_FOREACH(kid, &bus->children, sibling) {
@@ -413,10 +414,11 @@ static void microvm_fix_kernel_cmdline(MachineState
*machine)
}
len = strlen(cmdline);
- if (len > VIRTIO_CMDLINE_TOTAL_MAX_LEN + strlen(existing_cmdline))
+ if (len > VIRTIO_CMDLINE_TOTAL_MAX_LEN + strlen(existing_cmdline)) {
fprintf(stderr, "qemu: virtio mmio cmdline too large, skipping\n");
- else
+ } else {
memcpy(existing_cmdline, cmdline, len + 1);
+ }
g_free(cmdline);
}
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b57a993596..eaff4227bd 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -803,7 +803,7 @@ void x86_load_linux(X86MachineState *x86ms,
bool linuxboot_dma_enabled =
X86_MACHINE_GET_CLASS(x86ms)->fwcfg_dma_enabled;
uint16_t protocol;
int setup_size, kernel_size, cmdline_size;
- int dtb_size;
+ int dtb_size, setup_data_offset;
uint32_t initrd_max;
uint8_t header[8192], *setup, *kernel;
hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0,
first_setup_data = 0;
@@ -818,8 +818,10 @@ void x86_load_linux(X86MachineState *x86ms,
SevKernelLoaderContext sev_load_ctx = {};
enum { RNG_SEED_LENGTH = 32 };
- /* Add the NUL terminator, some padding for the microvm cmdline fiddling
- * hack, and then align to 16 bytes as a paranoia measure */
+ /*
+ * Add the NUL terminator, some padding for the microvm cmdline fiddling
+ * hack, and then align to 16 bytes as a paranoia measure
+ */
cmdline_size = (strlen(machine->kernel_cmdline) + 1 +
VIRTIO_CMDLINE_TOTAL_MAX_LEN + 16) & ~15;
/* Make a copy, since we might append arbitrary bytes to it later. */
@@ -1090,22 +1092,24 @@ void x86_load_linux(X86MachineState *x86ms,
exit(1);
}
+ setup_data_offset = cmdline_size;
cmdline_size += sizeof(SetupData) + dtb_size;
kernel_cmdline = g_realloc(kernel_cmdline, cmdline_size);
- setup_data = (void *)kernel_cmdline + cmdline_size -
(sizeof(SetupData) + dtb_size);
+ setup_data = (void *)kernel_cmdline + setup_data_offset;
setup_data->next = cpu_to_le64(first_setup_data);
- first_setup_data = cmdline_addr + ((void *)setup_data - (void
*)kernel_cmdline);
+ first_setup_data = cmdline_addr + setup_data_offset;
setup_data->type = cpu_to_le32(SETUP_DTB);
setup_data->len = cpu_to_le32(dtb_size);
load_image_size(dtb_filename, setup_data->data, dtb_size);
}
if (!legacy_no_rng_seed && protocol >= 0x209) {
+ setup_data_offset = cmdline_size;
cmdline_size += sizeof(SetupData) + RNG_SEED_LENGTH;
kernel_cmdline = g_realloc(kernel_cmdline, cmdline_size);
- setup_data = (void *)kernel_cmdline + cmdline_size -
(sizeof(SetupData) + RNG_SEED_LENGTH);
+ setup_data = (void *)kernel_cmdline + setup_data_offset;
setup_data->next = cpu_to_le64(first_setup_data);
- first_setup_data = cmdline_addr + ((void *)setup_data - (void
*)kernel_cmdline);
+ first_setup_data = cmdline_addr + setup_data_offset;
setup_data->type = cpu_to_le32(SETUP_RNG_SEED);
setup_data->len = cpu_to_le32(RNG_SEED_LENGTH);
qemu_guest_getrandom_nofail(setup_data->data, RNG_SEED_LENGTH);
