[PATCH v3] s390x/pci: prevent null pointer dereference during zpci hot unplug

Aby Sam Ross posted 1 patch 1 month, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/b45cefc3147c2c8446772dab0f53d030fb92406a.1770963150.git.abysamross@ibm.com
Maintainers: Matthew Rosato <mjrosato@linux.ibm.com>, Farhan Ali <alifm@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>
hw/s390x/s390-pci-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v3] s390x/pci: prevent null pointer dereference during zpci hot unplug
Posted by Aby Sam Ross 1 month, 3 weeks ago
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