Re: [kvmtool PATCH] riscv: Fix no params with nodefault segfault

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Drew,

On Thu, Jan 23, 2025 at 04:13:40PM +0100, Andrew Jones wrote:
> Fix segfault received when using --nodefault without --params.
> 
> Fixes: 7c9aac003925 ("riscv: Generate FDT at runtime for Guest/VM")
> Suggested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  riscv/fdt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/riscv/fdt.c b/riscv/fdt.c
> index 85c8f95604f6..f6a702533258 100644
> --- a/riscv/fdt.c
> +++ b/riscv/fdt.c
> @@ -256,9 +256,10 @@ static int setup_fdt(struct kvm *kvm)
>  		if (kvm->cfg.kernel_cmdline)
>  			_FDT(fdt_property_string(fdt, "bootargs",
>  						 kvm->cfg.kernel_cmdline));
> -	} else
> +	} else if (kvm->cfg.real_cmdline) {

Looks good to me, matches how arm/fdt.c handles real_cmdline:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Some context here. Before the patch that introduced --nodefaults, in commit
5613ae26b998 ("Add --nodefaults command line argument"), kvmtool would
always fiddle with the kernel command line to make it more likely that a
kernel boots - kvm_cmd_run_init() called kvm_run_set_real_cmdline()
unconditionally, which made it that kvm->cfg.real_cmdline was always
pointing to a valid string. Hence no check was needed here.

After --nodefaults was introduced, if the user specifies --nodefaults and
doesn't set the kernel command line (via -p/--params), real_cmdline ends up
being NULL and the fdt generator segfaults below.

The riscv port was developed in parallel to the series that introduced
--nodefaults, so I guess this check was missed during integration.

Thanks,
Alex

>  		_FDT(fdt_property_string(fdt, "bootargs",
>  					 kvm->cfg.real_cmdline));
> +	}
>  
>  	_FDT(fdt_property_string(fdt, "stdout-path", "serial0"));
>  
> -- 
> 2.48.1
> 

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv