Message ID | 20220121213852.30243-1-danielhb413@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | spapr.c: check bus != NULL in spapr_get_fw_dev_path() | expand |
On 1/21/22 22:38, Daniel Henrique Barboza wrote: > spapr_get_fw_dev_path() is an impl of > FWPathProviderClass::get_dev_path(). This interface is used by > hw/core/qdev-fw.c via fw_path_provider_try_get_dev_path() in two > functions: > > - static char *qdev_get_fw_dev_path_from_handler(), which is used only in > qdev_get_fw_dev_path_helper() and it's guarded by "if (dev && > dev->parent_bus)"; > > - char *qdev_get_own_fw_dev_path_from_handler(), which is used in > softmmu/bootdevice.c in get_boot_device_path() like this: > > if (dev) { > d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev); > > This means that, when called via softmmu/bootdevice.c, there's no check > of 'dev->parent_bus' being not NULL. The result is that the "BusState > *bus" arg of spapr_get_fw_dev_path() can potentially be NULL and if, at > the same time, "SCSIDevice *d" is not NULL, we'll hit this line: > > void *spapr = CAST(void, bus->parent, "spapr-vscsi"); > > And we'll SIGINT because 'bus' is NULL and we're accessing bus->parent. > > Adding a simple 'bus != NULL' check to guard the instances where we > access 'bus->parent' can avoid this altogether. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/ppc/spapr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 72f5dce751..3d6ec309dd 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3053,7 +3053,7 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus, > VHostSCSICommon *vsc = CAST(VHostSCSICommon, dev, TYPE_VHOST_SCSI_COMMON); > PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE); > > - if (d) { > + if (d && bus) { > void *spapr = CAST(void, bus->parent, "spapr-vscsi"); > VirtIOSCSI *virtio = CAST(VirtIOSCSI, bus->parent, TYPE_VIRTIO_SCSI); > USBDevice *usb = CAST(USBDevice, bus->parent, TYPE_USB_DEVICE); >
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 72f5dce751..3d6ec309dd 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3053,7 +3053,7 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus, VHostSCSICommon *vsc = CAST(VHostSCSICommon, dev, TYPE_VHOST_SCSI_COMMON); PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE); - if (d) { + if (d && bus) { void *spapr = CAST(void, bus->parent, "spapr-vscsi"); VirtIOSCSI *virtio = CAST(VirtIOSCSI, bus->parent, TYPE_VIRTIO_SCSI); USBDevice *usb = CAST(USBDevice, bus->parent, TYPE_USB_DEVICE);
spapr_get_fw_dev_path() is an impl of FWPathProviderClass::get_dev_path(). This interface is used by hw/core/qdev-fw.c via fw_path_provider_try_get_dev_path() in two functions: - static char *qdev_get_fw_dev_path_from_handler(), which is used only in qdev_get_fw_dev_path_helper() and it's guarded by "if (dev && dev->parent_bus)"; - char *qdev_get_own_fw_dev_path_from_handler(), which is used in softmmu/bootdevice.c in get_boot_device_path() like this: if (dev) { d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev); This means that, when called via softmmu/bootdevice.c, there's no check of 'dev->parent_bus' being not NULL. The result is that the "BusState *bus" arg of spapr_get_fw_dev_path() can potentially be NULL and if, at the same time, "SCSIDevice *d" is not NULL, we'll hit this line: void *spapr = CAST(void, bus->parent, "spapr-vscsi"); And we'll SIGINT because 'bus' is NULL and we're accessing bus->parent. Adding a simple 'bus != NULL' check to guard the instances where we access 'bus->parent' can avoid this altogether. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/spapr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)