2022-08-31 20:44:17

by Matthew Rosato

Subject: [PATCH v4 0/2] iommu/s390: fixes related to repeat attach_dev calls

This series contains a few fixes related to supporting multiple unmanaged
iommu domains on s390.

Changelog since v3:
- add a 2nd patch to fix issues related to leaking s390_domain_device

Matthew Rosato (2):
iommu/s390: Fix race with release_device ops
iommu/s390: fix leak of s390_domain_device

arch/s390/include/asm/pci.h | 1 +
arch/s390/pci/pci.c | 1 +
drivers/iommu/s390-iommu.c | 62 ++++++++++++++++++++++++++++++++++---
3 files changed, 59 insertions(+), 5 deletions(-)

--
2.37.2

2022-08-31 21:06:09

by Matthew Rosato

Subject: [PATCH v4 2/2] iommu/s390: fix leak of s390_domain_device

Since allowing multiple domains to be attached via fa7e9ecc5e1c, it's now
possible that a kfree of s390_domain_device is missed, either because a
corresponding detach_dev was never called or because a repeat attach_dev
was called for the same device and domain pair (resulting in unnecessary
duplicates). Check for duplicates during attach_dev and ensure the list
of s390_domain_device structures is cleared up when the domain is freed.

Fixes: fa7e9ecc5e1c ("iommu/s390: Tolerate repeat attach_dev calls")
Signed-off-by: Matthew Rosato <[email protected]>
---
drivers/iommu/s390-iommu.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index 1137d669e849..db4dfbcf161b 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -78,7 +78,17 @@ static struct iommu_domain *s390_domain_alloc(unsigned domain_type)
static void s390_domain_free(struct iommu_domain *domain)
{
struct s390_domain *s390_domain = to_s390_domain(domain);
+ struct s390_domain_device *domain_device, *tmp;
+ unsigned long flags;

+ /* Ensure all device entries are cleaned up */
+ spin_lock_irqsave(&s390_domain->list_lock, flags);
+ list_for_each_entry_safe(domain_device, tmp, &s390_domain->devices,
+ list) {
+ list_del(&domain_device->list);
+ kfree(domain_device);
+ }
+ spin_unlock_irqrestore(&s390_domain->list_lock, flags);
dma_cleanup_tables(s390_domain->dma_table);
kfree(s390_domain);
}
@@ -88,7 +98,7 @@ static int s390_iommu_attach_device(struct iommu_domain *domain,
{
struct s390_domain *s390_domain = to_s390_domain(domain);
struct zpci_dev *zdev = to_zpci_dev(dev);
- struct s390_domain_device *domain_device;
+ struct s390_domain_device *domain_device, *ddev;
unsigned long flags;
int cc, rc;

@@ -140,7 +150,16 @@ static int s390_iommu_attach_device(struct iommu_domain *domain,
goto out_restore;
}
domain_device->zdev = zdev;
- list_add(&domain_device->list, &s390_domain->devices);
+ /* If already attached don't add another instance */
+ list_for_each_entry(ddev, &s390_domain->devices, list) {
+ if (ddev->zdev == zdev) {
+ kfree(domain_device);
+ domain_device = NULL;
+ break;
+ }
+ }
+ if (domain_device)
+ list_add(&domain_device->list, &s390_domain->devices);
spin_unlock_irqrestore(&s390_domain->list_lock, flags);
zdev->s390_domain = s390_domain;
mutex_unlock(&zdev->dma_domain_lock);
--
2.37.2