Re: [Qemu-devel] [PATCH 01/23] ppc/pnv: introduce a new intc_create() operation to the chip model

From: "Cédric Le Goater" <clg@kaod.org>
To: Peter Maydell <peter.maydell@linaro.org>,
	David Gibson <david@gibson.dropbear.id.au>
Cc: Alexander Graf <agraf@suse.de>,
	Alexey Kardashevskiy <aik@ozlabs.ru>, Greg Kurz <groug@kaod.org>,
	Laurent Vivier <lvivier@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	qemu-ppc <qemu-ppc@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH 01/23] ppc/pnv: introduce a new intc_create() operation to the chip model
Date: Tue, 26 Jun 2018 11:31:03 +0200	[thread overview]
Message-ID: <0826a256-5c9d-5b7d-7ec2-fcba748e2124@kaod.org> (raw)
In-Reply-To: <CAFEAcA_qv=PEzDOh8FB5F-uhbbtHrr7nRWPjXehyjkmj_cJ-AQ@mail.gmail.com>

On 06/26/2018 11:07 AM, Peter Maydell wrote:
> On 22 June 2018 at 05:24, David Gibson <david@gibson.dropbear.id.au> wrote:
>> From: Cédric Le Goater <clg@kaod.org>
>>
>> On Power9, the thread interrupt presenter has a different type and is
>> linked to the chip owning the cores.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  hw/ppc/pnv.c         | 21 +++++++++++++++++++--
>>  hw/ppc/pnv_core.c    | 18 +++++++++---------
>>  include/hw/ppc/pnv.h |  1 +
>>  3 files changed, 29 insertions(+), 11 deletions(-)
> 
> Hi; Coverity points out a bug (CID 1393617) in this patch
> (which is commit d35aefa9ae150a):
> 
>> @@ -143,13 +144,12 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
>>      void *obj;
>>      int i, j;
>>      char name[32];
>> -    Object *xi;
>> +    Object *chip;
>>
>> -    xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
>> -    if (!xi) {
>> -        error_setg(errp, "%s: required link 'xics' not found: %s",
>> -                   __func__, error_get_pretty(local_err));
>> -        return;
>> +    chip = object_property_get_link(OBJECT(dev), "chip", &local_err);
>> +    if (!chip) {
>> +        error_propagate(errp, local_err);
>> +        error_prepend(errp, "required link 'chip' not found: ");
>>      }
> 
> We check for a NULL 'chip' pointer, but forget the 'return', so
> execution will plough on through to the code below and eventually
> dereference the NULL pointer and segfault.

arg. My fault. I will send a fix.

Thanks,

C. 