Re: [qemu-s390x] [PATCH v2] s390: avoid potential null dereference in s390_pcihost_unplug()

[Cornelia Huck]

On Fri, 4 Jan 2019 16:05:15 +0100
Halil Pasic <address@hidden> wrote:

> On Fri, 4 Jan 2019 15:10:05 +0100
> Cornelia Huck <address@hidden> wrote:
> 
> > On Thu,  3 Jan 2019 07:16:12 -0800
> > Li Qiang <address@hidden> wrote:
> >   
> > > When getting the 'pbdev', the if...else has no default branch.
> > > From Coverity, the 'pbdev' maybe null when the 'dev' is not
> > > the TYPE_PCI_BRIDGE/TYPE_PCI_DEVICE/TYPE_S390_PCI_DEVICE.
> > > This patch adds a default branch for device plug and unplug.
> > > 
> > > Spotted by Coverity: CID 1398593
> > > 
> > > Signed-off-by: Li Qiang <address@hidden>
> > > ---
> > > Adds a default branch for device plug per Cornelia's review.
> > > 
> > >  hw/s390x/s390-pci-bus.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > > index 15759b6514..fe48a36ff6 100644
> > > --- a/hw/s390x/s390-pci-bus.c
> > > +++ b/hw/s390x/s390-pci-bus.c
> > > @@ -912,6 +912,9 @@ static void s390_pcihost_plug(HotplugHandler 
> > > *hotplug_dev, DeviceState *dev,
> > >          pbdev->fh = pbdev->idx;
> > >          QTAILQ_INSERT_TAIL(&s->zpci_devs, pbdev, link);
> > >          g_hash_table_insert(s->zpci_table, &pbdev->idx, pbdev);
> > > +    } else {
> > > +        error_setg(errp, "s390: device plug request for not supported 
> > > device"
> > > +                   " type: %s", object_get_typename(OBJECT(dev)));  
> > 
> > Maybe make this "s390/pci: plugging device type <%s> is not supported"?
> >   
> 
> Under what circumstances could/does this happen? I mean how can this
> be triggered by the user?

Probably only if a new type that can be plugged has been added, but the
s390 pci code has not been updated. We could also assert, not sure what
would make it easier to figure out what went wrong.