Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Fix crash when running without CPU

[Thomas Huth <thuth@redhat.com>]

On 26.01.2017 00:26, Alistair Francis wrote:
> On Wed, Jan 25, 2017 at 12:52 PM, Laurent Vivier <laurent@vivier.eu> wrote:
>> Le 25/01/2017 à 21:45, Thomas Huth a écrit :
>>> When running QEMU with "-M none -device loader,file=kernel.elf", it
>>> currently crashes with a segmentation fault, because the "none"-machine
>>> does not have any CPU by default and the generic loader code tries
>>> to dereference s->cpu. Fix it by adding an appropriate check for a
>>> NULL pointer.
>>>
>>> Reported-by: Laurent Vivier <laurent@vivier.eu>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  hw/core/generic-loader.c | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
>>> index 58f1f02..4601267 100644
>>> --- a/hw/core/generic-loader.c
>>> +++ b/hw/core/generic-loader.c
>>> @@ -137,20 +137,21 @@ static void generic_loader_realize(DeviceState *dev, Error **errp)
>>>  #endif
>>>
>>>      if (s->file) {
>>> +        AddressSpace *as = s->cpu ? s->cpu->as :  NULL;
>>
>> Should we just abort if s->cpu is NULL?
> 
> I agree, what is the use case where you are loading images without a CPU?
> 
> If there is a use case (maybe some KVM thing?) then this patch looks fine to me.

I think there is no real use case yet. But this fix is 1) simpler than
doing an error_report() + exit() here, and 2) maybe the vision of
constructing machines on the fly with QEMU will eventually come true one
day in the distant future, so with that patch here, the code would
already be prepared for the case when QEMU gets started without CPUs and
the CPUs are then later added via QOM...
Well, I don't mind ... if you prefer an error message instead, feel free
to suggest another patch. I'm fine as long as we do not simply crash
with a segmentation fault here.

 Thomas