[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH v2] hw/ppc/spapr: Fix segfault when i

From: Thomas Huth
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH v2] hw/ppc/spapr: Fix segfault when instantiating a 'pc-dimm' without 'memdev'
Date: Mon, 21 Aug 2017 10:00:56 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0


On 21.08.2017 09:36, Pankaj Gupta wrote:
> Hello Thomas,
>> QEMU currently crashes when trying to use a 'pc-dimm' on the pseries
>> machine without specifying its 'memdev' property. This happens because
>> pc_dimm_get_memory_region() does not check whether the 'memdev' property
>> has properly been set by the user. Looking closer at this function, it's
>> also obvious that it is using &error_abort to call another function - and
>> this is bad in a function that is used in the hot-plugging calling chain
>> since this can also cause QEMU to exit unexpectedly.
> In place of checking if 'memdev' is already allocated/present while fetching 
> 'get_functions' every place. If 'memdev' is required for memory hotplug 
> operation 
> can we just put a check at some common place and don't start or instantiate 
> if 
> 'memdev' is not present.
> I can see:
> 'pc_dimm_realize' function checks for : in my case prints warning
> and avoid Qemu to start.
> ...
> ...
> if (!dimm->hostmem) {
>         error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
>         return;
>     } 
> ...

That check unfortunately does not apply here: The crash happens in the
pre_plug() handler of the spapr machine, and this is called *before* the
realize function of the pc-dimm is called!

But even if we could find another common place where we could check
dimm->hostmem first, you still have the problem that
host_memory_backend_get_memory() could return an error and thus
pc_dimm_get_memory_region() still needs a way to fail gracefully during
the hotplug (or also hot-unplug) operation. So I think my patch is the
right way to go here.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]