didn't read the patch yet but just formatting comments:
On Wed, Feb 22, 2023 at 10:39:10PM -0800, Daniel Hoffman wrote:
> The 'microvm' machine type allows for disabling ACPI, in which case
> the VirtIO device configuration is passed via appending it to the
> kernel cmdline.
>
> If no cmdline parameter was passed, then a null pointer is dereferenced when
> the new cmdline is copied back. A solution is to always define the cmdline
> in the fw_cfg so the read to append happens before the first write in the
> multiboot case, and to explcitly re-write the value to update the length.
explicitly
>
> Fixes: eac7a7791b
format is:
Fixes: hash ("subject")
>
> Signed-off-by: Daniel Hoffman <dhoff749@gmail.com>
> ---
> hw/i386/microvm.c | 3 ++-
> hw/i386/x86.c | 4 ++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 29f30dd6d3..be64280530 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -417,7 +417,8 @@ static void microvm_fix_kernel_cmdline(MachineState *machine)
> if (len > VIRTIO_CMDLINE_TOTAL_MAX_LEN + strlen(existing_cmdline)) {
> fprintf(stderr, "qemu: virtio mmio cmdline too large, skipping\n");
> } else {
> - memcpy(existing_cmdline, cmdline, len + 1);
> + fw_cfg_modify_i32(x86ms->fw_cfg, FW_CFG_CMDLINE_SIZE, len + 1);
> + fw_cfg_modify_string(x86ms->fw_cfg, FW_CFG_CMDLINE_DATA, cmdline);
Pls use spaces not tabs same as surrounding code.
> }
> g_free(cmdline);
> }
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index eaff4227bd..7dd02b7409 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -827,6 +827,10 @@ void x86_load_linux(X86MachineState *x86ms,
> /* Make a copy, since we might append arbitrary bytes to it later. */
> kernel_cmdline = g_strndup(machine->kernel_cmdline, cmdline_size);
>
> + /* If the cmdline is undefined, set it as an empty allocated value */
> + fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, cmdline_size);
> + fw_cfg_add_bytes(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline, cmdline_size);
> +
> /* load the kernel header */
> f = fopen(kernel_filename, "rb");
> if (!f) {
> --
> 2.37.2