[PATCH v4 0/6] KVM s390x PCI fixes

[PATCH v4 0/6] KVM s390x PCI fixes

[Farhan Ali]

Hi,

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

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

Thanks
Farhan

ChangeLog
---------
v3: https://lore.kernel.org/all/20260720175819.1723-1-alifm@linux.ibm.com/
v3 -> v4
   - Add validation checks for AISB/AIBV spanning more than a page.
   - Reject multiple ioctl call for the same device, if adapter interrupt
   forwarding is already enabled for the device.
   - Rebase on 7.2-rc4.

v2: https://lore.kernel.org/all/20260716175241.1039-1-alifm@linux.ibm.com/
v2 -> v3
   - Remove overwriting guest FIB since we don't use it for
   re-issue (patch 4).

v1: https://lore.kernel.org/all/20260713172600.1284-1-alifm@linux.ibm.com/
v1 -> v2
   - Drop fix handling AISB/AIBV spanning multiple pages.
   - Fix memory accounting functions for the case when interrupt forwarding
   is enabled by one process but disabled by a different process (patch 1).


Farhan Ali (6):
  KVM: s390: pci: Reject adapter interrupt forwarding if already enabled
  KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
  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: Validate AIBV and AISB before pinning guest pages

 arch/s390/kvm/pci.c | 102 +++++++++++++++++++++++++++++++++++---------
 arch/s390/kvm/pci.h |   2 +
 2 files changed, 84 insertions(+), 20 deletions(-)

-- 
2.43.0

[PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

[Farhan Ali]

The MPCIFC instruction doesn't allow registering adapter interrupts without
first unregistering. So reject any request to enable interrupt forwarding
if its already enabled for the zPCI device. This also fixes overwriting and
thus leaking resources when the ioctl is called multiple times for the same
device.

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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 720bb58cabe2..d2a11cdf6941 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -237,6 +237,10 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	if (zdev->gisa == 0)
 		return -EINVAL;
 
+	/* AIF already enabled for the device */
+	if (zdev->kzdev->fib.fmt0.aibv != 0)
+		return -EINVAL;
+
 	kvm = zdev->kzdev->kvm;
 	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
 
-- 
2.43.0

[PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[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(). It also
accounted/unaccounted the pages against the current->mm. But its possible
the unaccount_mem() can be called from a different process context than the
one that originally pinned the pages.

Let's fix this by storing the pinning process user_struct and mm_struct
when accounting for pinned pages, and subsequently free these resources
when the pages are unpinned.

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 | 38 ++++++++++++++++++++++++++++----------
 arch/s390/kvm/pci.h |  2 ++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index d2a11cdf6941..1b3114c7cfbb 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -190,33 +190,51 @@ static int kvm_zpci_clear_airq(struct zpci_dev *zdev)
 	return cc ? -EIO : 0;
 }
 
-static inline void unaccount_mem(unsigned long nr_pages)
+static inline void unaccount_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
 {
-	struct user_struct *user = get_uid(current_user());
+	struct user_struct *user = kzdev->user_account;
+	struct mm_struct *mm_account = kzdev->mm_account;
 
-	if (user)
+	if (user) {
 		atomic_long_sub(nr_pages, &user->locked_vm);
-	if (current->mm)
-		atomic64_sub(nr_pages, &current->mm->pinned_vm);
+		free_uid(user);
+		kzdev->user_account = NULL;
+	}
+
+	if (mm_account) {
+		atomic64_sub(nr_pages, &mm_account->pinned_vm);
+		mmdrop(mm_account);
+		kzdev->mm_account = NULL;
+	}
 }
 
-static inline int account_mem(unsigned long nr_pages)
+static inline int account_mem(struct kvm_zdev *kzdev, 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));
 
+	mmgrab(current->mm);
 	atomic64_add(nr_pages, &current->mm->pinned_vm);
+	kzdev->user_account = user;
+	kzdev->mm_account = current->mm;
 
 	return 0;
+
+out:
+	free_uid(user);
+	return rc;
 }
 
 static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
@@ -279,7 +297,7 @@ 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))
+	if (account_mem(zdev->kzdev, pcount))
 		goto unpin2;
 
 	/* AISB must be allocated before we can fill in GAITE */
@@ -400,7 +418,7 @@ static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
 		pcount++;
 	}
 	if (pcount > 0)
-		unaccount_mem(pcount);
+		unaccount_mem(kzdev, pcount);
 out:
 	mutex_unlock(&aift->aift_lock);
 
diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
index ff0972dd5e71..544e6aa75e38 100644
--- a/arch/s390/kvm/pci.h
+++ b/arch/s390/kvm/pci.h
@@ -21,6 +21,8 @@ struct kvm_zdev {
 	struct zpci_dev *zdev;
 	struct kvm *kvm;
 	struct zpci_fib fib;
+	struct user_struct *user_account;
+	struct mm_struct *mm_account;
 	struct list_head entry;
 };
 
-- 
2.43.0

[PATCH v4 3/6] 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 1b3114c7cfbb..33abc15aa685 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -297,14 +297,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(zdev->kzdev, pcount))
+	rc = account_mem(zdev->kzdev, 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 |
@@ -348,6 +351,8 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	return rc;
 
 unlock:
+	if (pcount > 0)
+		unaccount_mem(zdev->kzdev, pcount);
 	mutex_unlock(&aift->aift_lock);
 unpin2:
 	if (fib->fmt0.sum == 1)
-- 
2.43.0

[PATCH v4 4/6] 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 33abc15aa685..231a4236fc3c 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -314,6 +314,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;
 
@@ -350,6 +355,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(zdev->kzdev, pcount);
-- 
2.43.0

[PATCH v4 5/6] 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 | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 231a4236fc3c..d76b2c5484ac 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -341,19 +341,32 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 	aift->kzdev[zdev->aisb] = zdev->kzdev;
 	spin_unlock_irq(&aift->gait_lock);
 
-	/* Update guest FIB for re-issue */
-	fib->fmt0.aisbo = zdev->aisb & 63;
-	fib->fmt0.aisb = virt_to_phys(aift->sbv->vector) + (zdev->aisb / 64) * 8;
-	fib->fmt0.isc = gisc;
-
 	/* Save some guest fib values in the host for later use */
-	zdev->kzdev->fib.fmt0.isc = fib->fmt0.isc;
+	zdev->kzdev->fib.fmt0.isc = gisc;
 	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 v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[Farhan Ali]

The AIBV holds one bit per MSI-X vector for a given function. The size of
the bit vector is derived from the NOI and the AIBVO. If the size of the
AIBV exceeds a single page boundary, then reject the request as we cannot
safely pin the guest AIBV.

Similarly reject the request if the AISB address is not 8-byte aligned as
the architecture requires doubleword alignment for the summary bit address.
Since the AISBO can address up to 64 bits, the size of the AISB can only be
8 bytes for the function. This also ensures the AISB doesn't exceed a
single page boundary.

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 | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index d76b2c5484ac..e55e75b81b51 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -241,7 +241,7 @@ 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;
+	unsigned int msi_vecs, idx, size;
 	struct zpci_gaite *gaite;
 	unsigned long hva, bit;
 	struct kvm *kvm;
@@ -268,6 +268,14 @@ 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);
+	size += (fib->fmt0.aibvo / 8) + 1;
+	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
+	if (npages > 1) {
+		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);
@@ -283,6 +291,12 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 
 	/* Pin the guest AISB if one was specified */
 	if (fib->fmt0.sum == 1) {
+
+		if (fib->fmt0.aisb & 0x7) {
+			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,
-- 
2.43.0

Re: [PATCH v4 5/6] KVM: s390: pci: Fix resource leak on IRQ registration failure

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722170621.1686-1-alifm@linux.ibm.com?part=5

Re: [PATCH v4 3/6] KVM: s390: pci: Fix missing error codes and memory unaccounting

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722170621.1686-1-alifm@linux.ibm.com?part=3

Re: [PATCH v4 4/6] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722170621.1686-1-alifm@linux.ibm.com?part=4

Re: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition between KVM_S390_ZPCI_OP ioctl and device hot-unplug leads to Use-After-Free on `zdev`.
--

--- Patch [2]: [PATCH v4 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages ---
commit 2ef683f1261aa42729e17194a74f67aefe6f5d45
Author: Farhan Ali <alifm@linux.ibm.com>

KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
    
The account_mem() and unaccount_mem() functions call get_uid() which
increments the reference count of struct user_struct on every invocation.
This commit fixes this by storing the pinning process user_struct and
mm_struct when accounting for pinned pages, and subsequently freeing these
resources when the pages are unpinned.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d2a11cdf6941d..1b3114c7cfbb8 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but can a race condition between
the KVM_S390_ZPCI_OP ioctl and device hot-unplug lead to a use-after-free on
the zdev structure?

Looking at get_zdev_from_kvm_by_fh() in arch/s390/kvm/pci.c:

			zdev = kzdev->zdev;
			break;
		}
	}
	spin_unlock(&kvm->arch.kzdev_list_lock);

	return zdev;

The zdev pointer is returned without taking a reference, leaving it
vulnerable to being freed once the list lock is dropped.

Then in kvm_s390_pci_zpci_op():

	zdev = get_zdev_from_kvm_by_fh(kvm, args->fh);
	if (!zdev)
		return -ENODEV;

	mutex_lock(&zdev->kzdev_lock);

The ioctl thread dereferences the potentially freed zdev structure by
attempting to acquire its lock, outside of any RCU protection.

If a device hot-unplug is initiated on the host, and QEMU closes the VFIO
file descriptor after the ioctl thread drops the list lock but before it
acquires kzdev_lock, the device can be unregistered and freed.

Because the ioctl thread cannot be in an RCU read-side critical section
while it prepares to sleep on a mutex, the RCU grace period can elapse.
This could allow the ioctl thread to resume and attempt to call mutex_lock()
on the freed zdev structure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722170621.1686-1-alifm@linux.ibm.com?part=2

Re: [PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing NULL check for the result of `airq_iv_create()` in `kvm_s390_pci_aif_enable`.
- [Critical] Missing page boundary check for guest-provided AIBV address allows an out-of-bounds write into host physical memory.
- [High] Reference count leak of `struct user_struct` in `account_mem` and `unaccount_mem`.
- [High] Memory un-accounting uses the current thread's credentials instead of the original ones, causing counter underflows.
- [High] Complete cleanup bypass when `kvm_zpci_set_airq()` fails in `kvm_s390_pci_aif_enable`.
- [High] Use-After-Free of `zdev` structure in `kvm_s390_pci_zpci_op()` due to lack of reference counting.
--

--- Patch [1]: [PATCH v4 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled ---
commit 32372cae1fd4bf986fc6da1a214da1e85e7be6ad
Author: Farhan Ali <alifm@linux.ibm.com>
KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

This commit prevents registering adapter interrupts via the MPCIFC instruction
if they are already registered. It avoids leaking resources by rejecting
requests to enable interrupt forwarding if it is already enabled for the zPCI
device.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
[ ... ]
>  	list_for_each_entry(kzdev, &kvm->arch.kzdev_list, entry) {
>  		if (kzdev->zdev->fh == fh) {
>  			zdev = kzdev->zdev;
>  			break;
>  		}
>  	}
>  	spin_unlock(&kvm->arch.kzdev_list_lock);
>  	return zdev;

[Severity: High]
This isn't a bug introduced by this patch, but is the returned zdev pointer
vulnerable to a use-after-free?

The pointer is retrieved and returned without elevating the device's reference
count via zpci_zdev_get(). If a concurrent hot-unplug occurs, the PCI core
might remove and free the device.

If the KVM ioctl thread then executes mutex_lock(&zdev->kzdev_lock) in
kvm_s390_pci_zpci_op() on the freed memory, can it lead to memory corruption
or a host crash?

[ ... ]
>  	struct user_struct *user = get_uid(current_user());
[ ... ]
>  	return 0;

[Severity: High]
This is a pre-existing issue, but does account_mem() leak the user_struct
reference?

The get_uid(current_user()) call fetches the user and increments its reference
count. However, the function returns 0 without ever pairing this with a
free_uid(user) call on the success path.

Could an unprivileged guest repeatedly enable and disable AIF to leak kernel
memory continuously and eventually overflow the reference counter?

[ ... ]
>  	struct user_struct *user = get_uid(current_user());
[ ... ]
>  	if (current->mm)
>  		atomic64_sub(nr_pages, &current->mm->pinned_vm);

[Severity: High]
This isn't a bug introduced by this patch, but does unaccount_mem()
incorrectly rely on the current thread's credentials?

When kvm_s390_pci_unregister_kvm() is invoked by the PCI core during device
hot-unplug or VFIO unbind, it executes in a system worker thread, not the
original KVM vcpu thread. It then calls unaccount_mem().

Does unaccount_mem() use current_user() and current->mm of the worker thread
instead of the original ones? Could this corrupt the memory accounting limits
of random processes, cause locked_vm to underflow, and permanently leak the
locked memory quota of the original KVM process?

[ ... ]
> @@ -237,6 +237,10 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  	if (zdev->gisa == 0)
>  		return -EINVAL;
>  
> +	/* AIF already enabled for the device */
> +	if (zdev->kzdev->fib.fmt0.aibv != 0)
> +		return -EINVAL;
> +
>  	kvm = zdev->kzdev->kvm;
>  	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
[ ... ]
>  	npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);
[ ... ]
>  	gaddr = page_to_phys(aibv_page) + (fib->fmt0.aibv & ~PAGE_MASK);
>  	fib->fmt0.aibv = gaddr;

[Severity: Critical]
This is a pre-existing issue, but does this calculation allow an out-of-bounds
write into host physical memory?

Exactly one page is pinned, but the physical address is constructed by adding
an unaligned offset (fib->fmt0.aibv & ~PAGE_MASK). It doesn't appear to verify
if the required bit-vector size (msi_vecs bits) fits within the remaining
bounds of this single pinned page.

If a guest provides an address with a high intra-page offset, could the
hardware page-table walker or DMA overwrite the adjacent, unpinned physical
page and cause severe host memory corruption?

[ ... ]
>  	zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
>  				    AIRQ_IV_BITLOCK |
>  				    AIRQ_IV_GUESTVEC,
>  				    phys_to_virt(fib->fmt0.aibv));
>  
>  	spin_lock_irq(&aift->gait_lock);

[Severity: Critical]
This isn't a bug introduced by this patch, but does airq_iv_create() return a
pointer that is used without a NULL check?

If airq_iv_create() dynamically allocates memory and returns NULL on failure,
the pointer seems to be assigned to zdev->aibv and then dereferenced shortly
after in kvm_zpci_set_airq() via airq_iv_end().

Can an unprivileged guest trigger this path via KVM_S390_ZPCIOP_REG_AEN and
crash the host system if it is under memory pressure?

[ ... ]
>  	/* Issue the clp to setup the irq now */
>  	rc = kvm_zpci_set_airq(zdev);
>  	return rc;
>  
>  unlock:
>  	mutex_unlock(&aift->aift_lock);

[Severity: High]
This is a pre-existing issue, but does the function bypass the cleanup block
if kvm_zpci_set_airq() fails?

If the hardware rejects the configuration and kvm_zpci_set_airq() returns an
error, the function appears to return rc directly instead of jumping to the
cleanup labels (unlock:, unpin2:, out:).

Could this leave pinned pages, memory accounting, GAIT entries, and airq_iv
structures leaked? Since zdev->kzdev->fib.fmt0.aibv is left with a non-zero
value, does this persistently break the device for the guest?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722170621.1686-1-alifm@linux.ibm.com?part=1

Re: [PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The AIBV size calculation systematically overestimates the required memory size, leading to false rejection of valid AIBV configurations located near page boundaries.
--

--- Patch [6]: [PATCH v4 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages ---
commit 4b6ef516a235f953d1559a623689c91ffb836199
Author: Farhan Ali <alifm@linux.ibm.com>

KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

This commit adds validation checks for the guest-provided AIBV and AISB 
addresses before pinning them. It ensures the AIBV does not exceed a single 
page boundary and that the AISB is aligned properly to 8 bytes.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d76b2c5484aca..e55e75b81b516 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c

[ ... ]

> @@ -268,6 +268,14 @@ 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);
> +	size += (fib->fmt0.aibvo / 8) + 1;
> +	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
> +	if (npages > 1) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +

[Severity: High]
Does this size calculation artificially inflate the required AIBV memory space 
and falsely reject valid guest configurations?

If a guest places a 1-MSI vector AIBV near the end of a physical page, such 
as at page offset 4088, the size formula calculates at least 9 bytes (8 
bytes from BITS_TO_LONGS(1) plus 1 arbitrary byte). 

This inflated size (4088 + 9 = 4097) crosses the page boundary, causing KVM 
to mistakenly reject a perfectly valid, 8-byte aligned memory placement with 
-EINVAL. Can this formula be adjusted to calculate the exact size required?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722170621.1686-1-alifm@linux.ibm.com?part=6