[PATCH v1 0/5] KVM s390x PCI fixes

[PATCH v1 0/5] KVM s390x PCI fixes

[Farhan Ali]

Hi,

This series attempts to fixe the pre-existing issues[1] found by sashiko.

[1] https://lore.kernel.org/all/20260624063447.85DF51F000E9@smtp.kernel.org/


Farhan Ali (5):
  KVM: s390: pci: Fix refcount leak in memory accounting functions
  KVM: s390: pci: Fix missing error codes and memory unaccounting
  KVM: s390: pci: Fix NULL dereference on AIBV allocation failure
  KVM: s390: pci: Fix resource leak on IRQ registration failure
  KVM: s390: pci: Fix AIBV and AISB spanning multiple pages

 arch/s390/kvm/pci.c | 130 +++++++++++++++++++++++++++++++++++---------
 arch/s390/kvm/pci.h |   5 ++
 2 files changed, 109 insertions(+), 26 deletions(-)

-- 
2.43.0

[PATCH v1 1/5] KVM: s390: pci: Fix refcount leak in memory accounting functions

[Farhan Ali]

The account_mem() and unaccount_mem() functions call get_uid() which
increments the reference count of struct user_struct on every invocation.
But we don't decrement the count by calling free_uid().
Fix this by calling the free_uid() appropriately.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 720bb58cabe2..5bbbb1de4b5a 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -198,25 +198,32 @@ static inline void unaccount_mem(unsigned long nr_pages)
 		atomic_long_sub(nr_pages, &user->locked_vm);
 	if (current->mm)
 		atomic64_sub(nr_pages, &current->mm->pinned_vm);
+
+	free_uid(user);
 }
 
 static inline int account_mem(unsigned long nr_pages)
 {
 	struct user_struct *user = get_uid(current_user());
 	unsigned long page_limit, cur_pages, new_pages;
+	int rc = 0;
 
 	page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
 
 	cur_pages = atomic_long_read(&user->locked_vm);
 	do {
 		new_pages = cur_pages + nr_pages;
-		if (new_pages > page_limit)
-			return -ENOMEM;
+		if (new_pages > page_limit) {
+			rc = -ENOMEM;
+			goto out;
+		}
 	} while (!atomic_long_try_cmpxchg(&user->locked_vm, &cur_pages, new_pages));
 
 	atomic64_add(nr_pages, &current->mm->pinned_vm);
 
-	return 0;
+out:
+	free_uid(user);
+	return rc;
 }
 
 static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
-- 
2.43.0

[PATCH v1 2/5] KVM: s390: pci: Fix missing error codes and memory unaccounting

[Farhan Ali]

In kvm_s390_pci_aif_enable() two error paths failed to set error code,
causing the function to return 0 on failure. It also failed to rollback
memory accounting on failure. Fix both by propagating error code on
failure and calling unaccount_mem() in the cleanup path.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 5bbbb1de4b5a..42db20c2afca 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -282,14 +282,17 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	}
 
 	/* Account for pinned pages, roll back on failure */
-	if (account_mem(pcount))
+	rc = account_mem(pcount);
+	if (rc)
 		goto unpin2;
 
 	/* AISB must be allocated before we can fill in GAITE */
 	mutex_lock(&aift->aift_lock);
 	bit = airq_iv_alloc_bit(aift->sbv);
-	if (bit == -1UL)
+	if (bit == -1UL) {
+		rc = -ENOMEM;
 		goto unlock;
+	}
 	zdev->aisb = bit; /* store the summary bit number */
 	zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
 				    AIRQ_IV_BITLOCK |
@@ -333,6 +336,8 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	return rc;
 
 unlock:
+	if (pcount > 0)
+		unaccount_mem(pcount);
 	mutex_unlock(&aift->aift_lock);
 unpin2:
 	if (fib->fmt0.sum == 1)
-- 
2.43.0

[PATCH v1 3/5] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

[Farhan Ali]

The airq_iv_create() can return NULL on failure, but the return value was
never checked. If it fails, zdev->aibv will be NULL and fail when
derefenced in kvm_zpci_set_airq(). Add a NULL check and free the previously
allocated AISB bit and zdev->aisb on failure.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 42db20c2afca..b3ffb8c25510 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -299,6 +299,11 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 				    AIRQ_IV_GUESTVEC,
 				    phys_to_virt(fib->fmt0.aibv));
 
+	if (!zdev->aibv) {
+		rc = -ENOMEM;
+		goto free_aisb;
+	}
+
 	spin_lock_irq(&aift->gait_lock);
 	gaite = aift->gait + zdev->aisb;
 
@@ -335,6 +340,9 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	rc = kvm_zpci_set_airq(zdev);
 	return rc;
 
+free_aisb:
+	airq_iv_free_bit(aift->sbv, zdev->aisb);
+	zdev->aisb = 0;
 unlock:
 	if (pcount > 0)
 		unaccount_mem(pcount);
-- 
2.43.0

[PATCH v1 4/5] KVM: s390: pci: Fix resource leak on IRQ registration failure

[Farhan Ali]

Currently if kvm_zpci_set_airq() fails, kvm_s390_pci_aif_enable() returns
the error code but doesn't do any resource cleanup thus leaking resources.
Fix this by cleaning up all the resources such as the GAITE, AIBV, AISB and
unpinning any pinned pages.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index b3ffb8c25510..9a16a2eac65b 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -334,11 +334,29 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	/* Save some guest fib values in the host for later use */
 	zdev->kzdev->fib.fmt0.isc = fib->fmt0.isc;
 	zdev->kzdev->fib.fmt0.aibv = fib->fmt0.aibv;
-	mutex_unlock(&aift->aift_lock);
 
 	/* Issue the clp to setup the irq now */
 	rc = kvm_zpci_set_airq(zdev);
-	return rc;
+	if (!rc) {
+		mutex_unlock(&aift->aift_lock);
+		return rc;
+	}
+
+	/* Start cleanup */
+	zdev->kzdev->fib.fmt0.isc = 0;
+	zdev->kzdev->fib.fmt0.aibv = 0;
+
+	spin_lock_irq(&aift->gait_lock);
+	gaite->count--;
+	gaite->aisb = 0;
+	gaite->gisc = 0;
+	gaite->aisbo = 0;
+	gaite->gisa = 0;
+	aift->kzdev[zdev->aisb] = NULL;
+	spin_unlock_irq(&aift->gait_lock);
+
+	airq_iv_release(zdev->aibv);
+	zdev->aibv = NULL;
 
 free_aisb:
 	airq_iv_free_bit(aift->sbv, zdev->aisb);
-- 
2.43.0

[PATCH v1 5/5] KVM: s390: pci: Fix AIBV and AISB spanning multiple pages

[Farhan Ali]

The AIBV (adapter interrupt bit vector) holds one bit per MSI-X vector for
a given function. With a maximum of 2048 MSI-X vectors per function, the
AIBV can be up to 256 bytes. Because a guest is free to place the AIBV at
any alignment, it may cross a page boundary. Pin all pages covered by the
AIBV to handle this correctly.

The AISB (adapter interrupt summary bit) holds one bit per zPCI function.
Its total size is configuration-dependent and not architecturally bounded,
so the AISB offset is used to detect whether it crosses a page boundary
and pin accordingly.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 78 ++++++++++++++++++++++++++++++++++-----------
 arch/s390/kvm/pci.h |  5 +++
 2 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 9a16a2eac65b..5fe07181f3d6 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -229,13 +229,15 @@ static inline int account_mem(unsigned long nr_pages)
 static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 				   bool assist)
 {
-	struct page *pages[1], *aibv_page, *aisb_page = NULL;
-	unsigned int msi_vecs, idx;
+	struct page *aibv_page, *aisb_page = NULL;
+	int gisc, npages, npinned, pcount = 0;
+	unsigned int msi_vecs, idx, size;
 	struct zpci_gaite *gaite;
 	unsigned long hva, bit;
+	struct kvm_zdev *kzdev;
 	struct kvm *kvm;
 	phys_addr_t gaddr;
-	int rc = 0, gisc, npages, pcount = 0;
+	int rc = 0;
 
 	/*
 	 * Interrupt forwarding is only applicable if the device is already
@@ -244,6 +246,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	if (zdev->gisa == 0)
 		return -EINVAL;
 
+	kzdev = zdev->kzdev;
 	kvm = zdev->kzdev->kvm;
 	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
 
@@ -253,32 +256,63 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 		return gisc;
 
 	/* Replace AIBV address */
+	size = BITS_TO_LONGS(msi_vecs) * sizeof(unsigned long);
+	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
+	if (npages > MAX_AIF_PAGES) {
+		rc = -EINVAL;
+		goto out;
+	}
+
 	idx = srcu_read_lock(&kvm->srcu);
 	hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aibv));
-	npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);
+	npinned = pin_user_pages_fast(hva, npages, FOLL_WRITE | FOLL_LONGTERM,
+				      kzdev->aibv_pages);
 	srcu_read_unlock(&kvm->srcu, idx);
-	if (npages < 1) {
+	if (npinned <= 0) {
 		rc = -EIO;
 		goto out;
 	}
-	aibv_page = pages[0];
-	pcount++;
+
+	kzdev->aibv_npages = npinned;
+	if (npinned < npages) {
+		rc = -EIO;
+		goto unpin1;
+	}
+
+	pcount += npinned;
+	aibv_page = kzdev->aibv_pages[0];
 	gaddr = page_to_phys(aibv_page) + (fib->fmt0.aibv & ~PAGE_MASK);
 	fib->fmt0.aibv = gaddr;
 
 	/* Pin the guest AISB if one was specified */
 	if (fib->fmt0.sum == 1) {
+		size = (fib->fmt0.aisbo / 8) + 1;
+		npages = DIV_ROUND_UP((fib->fmt0.aisb & ~PAGE_MASK) + size,
+				      PAGE_SIZE);
+
+		if (npages > MAX_AIF_PAGES) {
+			rc = -EINVAL;
+			goto unpin1;
+		}
+
 		idx = srcu_read_lock(&kvm->srcu);
 		hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aisb));
-		npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM,
-					     pages);
+		npinned = pin_user_pages_fast(hva, npages,
+					     FOLL_WRITE | FOLL_LONGTERM,
+					     kzdev->aisb_pages);
 		srcu_read_unlock(&kvm->srcu, idx);
-		if (npages < 1) {
+		if (npinned <= 0) {
 			rc = -EIO;
 			goto unpin1;
 		}
-		aisb_page = pages[0];
-		pcount++;
+
+		kzdev->aisb_npages = npinned;
+		if (npinned < npages) {
+			rc = -EIO;
+			goto unpin2;
+		}
+		aisb_page = kzdev->aisb_pages[0];
+		pcount += npinned;
 	}
 
 	/* Account for pinned pages, roll back on failure */
@@ -366,10 +400,14 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 		unaccount_mem(pcount);
 	mutex_unlock(&aift->aift_lock);
 unpin2:
-	if (fib->fmt0.sum == 1)
-		unpin_user_page(aisb_page);
+	if (fib->fmt0.sum == 1) {
+		unpin_user_pages(kzdev->aisb_pages,
+				 kzdev->aisb_npages);
+		kzdev->aisb_npages = 0;
+	}
 unpin1:
-	unpin_user_page(aibv_page);
+	unpin_user_pages(kzdev->aibv_pages, kzdev->aibv_npages);
+	kzdev->aibv_npages = 0;
 out:
 	kvm_s390_gisc_unregister(kvm, fib->fmt0.isc);
 	return rc;
@@ -426,12 +464,14 @@ static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
 	kzdev->fib.fmt0.aibv = 0;
 
 	if (vpage) {
-		unpin_user_page(vpage);
-		pcount++;
+		unpin_user_pages(kzdev->aibv_pages, kzdev->aibv_npages);
+		pcount += kzdev->aibv_npages;
+		kzdev->aibv_npages = 0;
 	}
 	if (spage) {
-		unpin_user_page(spage);
-		pcount++;
+		unpin_user_pages(kzdev->aisb_pages, kzdev->aisb_npages);
+		pcount += kzdev->aisb_npages;
+		kzdev->aisb_npages = 0;
 	}
 	if (pcount > 0)
 		unaccount_mem(pcount);
diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
index ff0972dd5e71..a2d550f06b6c 100644
--- a/arch/s390/kvm/pci.h
+++ b/arch/s390/kvm/pci.h
@@ -17,10 +17,15 @@
 #include <asm/airq.h>
 #include <asm/cpu.h>
 
+#define MAX_AIF_PAGES 2
 struct kvm_zdev {
 	struct zpci_dev *zdev;
 	struct kvm *kvm;
 	struct zpci_fib fib;
+	struct page *aibv_pages[MAX_AIF_PAGES];
+	int aibv_npages;
+	struct page *aisb_pages[MAX_AIF_PAGES];
+	int aisb_npages;
 	struct list_head entry;
 };
 
-- 
2.43.0