vfio-pci hostdev realize during zpci hot plug fails (in `vfio_pci_realize()`)
if the vfio group file in `/dev/vfio/` lacks appropriate permissions and the
hostdev[/properties] addition doesn't reach the point where it could be
associated with previously added zpci device (in `s390_pcihost_plug()`).
As a result, zpci iommu pointer remains null. The zpci hot unplug following the
failed hostdev addition assumes zpci iommu pointer was assigned and tries to
make use of it to end the dma count resulting in a null pointer dereference.
In the non-hotplug scenario, `qdev_unplug()` for the zpci device is not called
after hostdev addition failure and this issue is not encountered.

Fixes: 37fa32de7073 ("s390x/pci: Honor DMA limits set by vfio")

Signed-off-by: Aby Sam Ross <abysamross@ibm.com>
Acked-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Reviewed-by: Farhan Ali <alifm@linux.ibm.com>
---
Thank you, Eric, Matthew & Farhan for looking into this.

v2:
Added subsystem name to the commit message.
Updated the `Fixes` tag to include the reference commit's message.
---
 hw/s390x/s390-pci-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index b438d63c44..3166b91c46 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -1248,7 +1248,7 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
         pbdev->fid = 0;
         QTAILQ_REMOVE(&s->zpci_devs, pbdev, link);
         g_hash_table_remove(s->zpci_table, &pbdev->idx);
-        if (pbdev->iommu->dma_limit) {
+        if (pbdev->iommu && pbdev->iommu->dma_limit) {
             s390_pci_end_dma_count(s, pbdev->iommu->dma_limit);
         }
         qdev_unrealize(dev);
-- 
2.52.0

On Thu, 12 Feb 2026 06:47:36 -0500
Aby Sam Ross <abysamross@ibm.com> wrote:

> vfio-pci hostdev realize during zpci hot plug fails (in `vfio_pci_realize()`)
> if the vfio group file in `/dev/vfio/` lacks appropriate permissions and the
> hostdev[/properties] addition doesn't reach the point where it could be
> associated with previously added zpci device (in `s390_pcihost_plug()`).
> As a result, zpci iommu pointer remains null. The zpci hot unplug following the
> failed hostdev addition assumes zpci iommu pointer was assigned and tries to
> make use of it to end the dma count resulting in a null pointer dereference.
> In the non-hotplug scenario, `qdev_unplug()` for the zpci device is not called
> after hostdev addition failure and this issue is not encountered.


Maybe add a word or two why the other dereferences of pbdev->iommu
not guarded by a null check are safe.

I think we have:
* s390_pci_sclp_deconfigure
* s390_pci_msix_init
* s390_pcihost_reset
* s390_pci_device_reset
* mpcifc_service_call
* stpcifc_service_call
* s390_pci_read_base

and more. My guess is that the device never gets into a state where
these operations are permissible, and the code makes sure
those functions won't be called on a device that has
pbdev->iommu == NULL. But that is just my guess.

DISCLAIMER: I didn't look at this properly, just asking based
on a quick look. Some of these may contain explicit or implicit
checking...

Regards,
Halil

> 
> Fixes: 37fa32de7073 ("s390x/pci: Honor DMA limits set by vfio")
> 
> Signed-off-by: Aby Sam Ross <abysamross@ibm.com>
> Acked-by: Eric Farman <farman@linux.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Reviewed-by: Farhan Ali <alifm@linux.ibm.com>

On 2/12/26 10:50 AM, Halil Pasic wrote:
> On Thu, 12 Feb 2026 06:47:36 -0500
> Aby Sam Ross <abysamross@ibm.com> wrote:
> 
>> vfio-pci hostdev realize during zpci hot plug fails (in `vfio_pci_realize()`)
>> if the vfio group file in `/dev/vfio/` lacks appropriate permissions and the
>> hostdev[/properties] addition doesn't reach the point where it could be
>> associated with previously added zpci device (in `s390_pcihost_plug()`).
>> As a result, zpci iommu pointer remains null. The zpci hot unplug following the
>> failed hostdev addition assumes zpci iommu pointer was assigned and tries to
>> make use of it to end the dma count resulting in a null pointer dereference.
>> In the non-hotplug scenario, `qdev_unplug()` for the zpci device is not called
>> after hostdev addition failure and this issue is not encountered.
> 
> 
> Maybe add a word or two why the other dereferences of pbdev->iommu
> not guarded by a null check are safe.
> 
> I think we have:
> * s390_pci_sclp_deconfigure
> * s390_pci_msix_init
> * s390_pcihost_reset
> * s390_pci_device_reset
> * mpcifc_service_call
> * stpcifc_service_call
> * s390_pci_read_base
> 
> and more. My guess is that the device never gets into a state where
> these operations are permissible, and the code makes sure
> those functions won't be called on a device that has
> pbdev->iommu == NULL. But that is just my guess.
> 
> DISCLAIMER: I didn't look at this properly, just asking based
> on a quick look. Some of these may contain explicit or implicit
> checking...

I mentioned in response to v1 as part of my review that I did look through all references of pbdev->iommu, as I was also concerned about whether we needed additional NULL checks.  But so far I'm not seeing it - it is largely implicit, but we don't drive the routines until the device is plugged, not in reserved|standby and iommu is associated.

This particular case is because we reach unplug (which also has to happen after plug of course) but the swizzle is we are reaching unplug exactly because we are giving up without actually having -successfully- plugged both the zpci and pci device.

But anyway, yes, I do think it would be good to add a small blurb to the commit message.

On Thu, 12 Feb 2026 11:55:15 -0500
Matthew Rosato <mjrosato@linux.ibm.com> wrote:

> > 
> > Maybe add a word or two why the other dereferences of pbdev->iommu
> > not guarded by a null check are safe.
> > 
> > I think we have:
> > * s390_pci_sclp_deconfigure
> > * s390_pci_msix_init
> > * s390_pcihost_reset
> > * s390_pci_device_reset
> > * mpcifc_service_call
> > * stpcifc_service_call
> > * s390_pci_read_base
> > 
> > and more. My guess is that the device never gets into a state where
> > these operations are permissible, and the code makes sure
> > those functions won't be called on a device that has
> > pbdev->iommu == NULL. But that is just my guess.
> > 
> > DISCLAIMER: I didn't look at this properly, just asking based
> > on a quick look. Some of these may contain explicit or implicit
> > checking...  
> 
> I mentioned in response to v1 as part of my review that I did look through all references of pbdev->iommu, as I was also concerned about whether we needed additional NULL checks.  But so far I'm not seeing it - it is largely implicit, but we don't drive the routines until the device is plugged, not in reserved|standby and iommu is associated.
> 
> This particular case is because we reach unplug (which also has to happen after plug of course) but the swizzle is we are reaching unplug exactly because we are giving up without actually having -successfully- plugged both the zpci and pci device.
> 
> But anyway, yes, I do think it would be good to add a small blurb to the commit message. 

Thanks! I have also assumed that I'm not the first one having this
thought.

Regards,
Halil

vfio-pci hostdev realize during zpci hot plug fails (in `vfio_pci_realize()`)
if the vfio group file in `/dev/vfio/` lacks appropriate permissions and the
hostdev[/properties] addition doesn't reach the point where it could be
associated with previously added zpci device (in `s390_pcihost_plug()`).
As a result, zpci iommu pointer remains null. The zpci hot unplug following the
failed hostdev addition assumes zpci iommu pointer was assigned and tries to
make use of it to end the dma count resulting in a null pointer dereference.
In the non-hotplug scenario, `qdev_unplug()` for the zpci device is not called
after hostdev addition failure and this issue is not encountered.

All other uses of zpci iommu without null check happens after both the zpci and
hostdev(pci) devices are plugged and are safe from null dereference.

Fixes: 37fa32de7073 ("s390x/pci: Honor DMA limits set by vfio")

Signed-off-by: Aby Sam Ross <abysamross@ibm.com>
Acked-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Reviewed-by: Farhan Ali <alifm@linux.ibm.com>
Suggested-by: Halil Pasic <pasic@linux.ibm.com> 
---
Thank you, Eric, Matthew & Farhan for looking into this.

v2:
Added subsystem name to the commit message.
Updated the `Fixes` tag to include the reference commit's message.

v3:
Added a note about other uses of zpci iommu without null check in the
description as suggested by Halil.
Matthew verified the zpci iommu uses during v1 review.
---
 hw/s390x/s390-pci-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index b438d63c44..3166b91c46 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -1248,7 +1248,7 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
         pbdev->fid = 0;
         QTAILQ_REMOVE(&s->zpci_devs, pbdev, link);
         g_hash_table_remove(s->zpci_table, &pbdev->idx);
-        if (pbdev->iommu->dma_limit) {
+        if (pbdev->iommu && pbdev->iommu->dma_limit) {
             s390_pci_end_dma_count(s, pbdev->iommu->dma_limit);
         }
         qdev_unrealize(dev);
-- 
2.52.0