Re: [Qemu-devel] [PATCH for-2.11] pc: fix crash on attempted cpu unplug

[Eduardo Habkost]

On Mon, Nov 20, 2017 at 03:34:13PM +0100, Igor Mammedov wrote:
> when qemu is started with '-no-acpi' CLI option, an attempt
> to unplug a CPU using device_del results in null pointer
> dereference at:
> 
>   #0 object_get_class
>   #1 pc_machine_device_unplug_request_cb
>   #2 qmp_marshal_device_del
> 
> which is caused by pcms->acpi_dev == NULL due to ACPI support
> being disabled.
> 
> Considering that ACPI support is necessary for unplug to work,
> check that it's enabled and fail unplug request gracefully
> if no acpi device were found.
> 
> Signed-off-by: Igor Mammedov <address@hidden>

Reviewed-by: Eduardo Habkost <address@hidden>

But I have one small suggestion:

> ---
>  hw/i386/pc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c3afe5b..d80cec3 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1842,6 +1842,11 @@ static void pc_cpu_unplug_request_cb(HotplugHandler 
> *hotplug_dev,
>      X86CPU *cpu = X86_CPU(dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>  
> +    if (!pcms->acpi_dev) {
> +        error_setg(&local_err, "not supported without acpi");

I suggest "CPU hot unplug not supported without ACPI" instead.

> +        goto out;
> +    }
> +
>      pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
>      assert(idx != -1);
>      if (idx == 0) {
> -- 
> 2.7.4
> 

-- 
Eduardo