[PATCH v5 0/6] KVM s390x PCI fixes

[PATCH v5 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
---------
v4: https://lore.kernel.org/all/20260722170621.1686-1-alifm@linux.ibm.com/
v4 -> v5
   - Fix off by one error (patch 6).
   - Add check for current->mm in account_mem() (patch 2).
   - Fix commit message as suggested by Matt (patch 3, 4, 5).

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

-- 
2.43.0

[PATCH v5 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")
Cc: stable@vger.kernel.org
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
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

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing bounds check for guest-provided Adapter Interrupt Bit Vector (AIBV) can lead to out-of-bounds DMA writes.
- [High] account_mem() and unaccount_mem() unconditionally leak a user_struct reference on every call.
- [High] The error path for kvm_zpci_set_airq() failure does not unwind allocated resources and permanently blocks future enablement.
- [High] Failure paths for memory accounting and vector allocation silently return success to userspace and leak process memory limits.
- [High] Missing NULL check on airq_iv_create() return value leads to kernel panic.
--

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

KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

The MPCIFC instruction doesn't allow registering adapter interrupts without
first unregistering. This patch rejects any request to enable interrupt
forwarding if it is already enabled for the zPCI device, fixing resource
leaks when the ioctl is called multiple times.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 720bb58cabe2e..d2a11cdf6941d 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;
> +

[Severity: High]
This is a pre-existing issue, but does the error path for kvm_zpci_set_airq()
failure permanently block future enablement?

In kvm_s390_pci_aif_enable(), if kvm_zpci_set_airq(zdev) fails and returns
an error immediately, the state variables like zdev->kzdev->fib.fmt0.aibv
might be left non-zero, and pinned pages are not unpinned.

With the new check introduced in this patch, wouldn't any future attempts to
enable AIF hit this new aibv != 0 check and fail with -EINVAL?

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

Looking at account_mem():

arch/s390/kvm/pci.c:account_mem() {
    ...
    struct user_struct *user = get_uid(current_user());
    ...
    kzdev->user_account = user;
    ...
}

It appears get_uid() increments the reference count, but neither account_mem()
nor unaccount_mem() ever call free_uid() on success or failure, potentially
leading to a refcount overflow and memory leak when KVM_S390_PCI_AIF_ENABLE is
called repeatedly. Is there a missing free_uid() call here?

[Severity: High]
This is a pre-existing issue, but do the failure paths for memory accounting
and vector allocation silently return success to userspace?

If account_mem() fails:

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    if (account_mem(pcount))
        goto unpin2;
    ...
}

Or if airq_iv_alloc_bit() fails:

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    bit = airq_iv_alloc_bit(aift->sbv);
    if (bit == -1UL)
        goto unlock;
    ...
}

It looks like the code jumps to unpin2 or unlock without setting rc to an
error code, returning 0. Could this exhaust process memory limits while
silently failing to set up AIF completely?

[Severity: High]
This is a pre-existing issue, but is there a missing NULL check on the return
value of airq_iv_create() that could lead to a kernel panic?

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA | AIRQ_IV_BITLOCK | AIRQ_IV_GUESTVEC, phys_to_virt(fib->fmt0.aibv));
    ...
}

If airq_iv_create() returns NULL under memory pressure, it unconditionally
assigns NULL to zdev->aibv. Shortly after, kvm_zpci_set_airq() dereferences
this pointer:

arch/s390/kvm/pci.c:kvm_zpci_set_airq() {
    ...
    fib.fmt0.noi = airq_iv_end(zdev->aibv);
    ...
}

Can this lead to a NULL pointer dereference?

[Severity: Critical]
This is a pre-existing issue, but is there a missing bounds check for the
guest-provided Adapter Interrupt Bit Vector (AIBV) that could lead to
out-of-bounds DMA writes?

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);
    ...
}

KVM explicitly pins only 1 page regardless of whether the requested bit vector
spans across a page boundary. If the guest provides a physical address very
close to the end of a page and the size of the bit vector requires more bytes,
could the hardware perform DMA writes spanning into the unpinned adjacent
page, potentially corrupting host memory?

>  	kvm = zdev->kzdev->kvm;
>  	msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
>

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

[PATCH v5 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")
Cc: stable@vger.kernel.org
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 43 ++++++++++++++++++++++++++++++++-----------
 arch/s390/kvm/pci.h |  2 ++
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index d2a11cdf6941..44c00e5e32c5 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -190,33 +190,54 @@ 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));
 
-	atomic64_add(nr_pages, &current->mm->pinned_vm);
+	if (current->mm) {
+		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 +300,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 +421,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..fdf8c7bf4ed0 100644
--- a/arch/s390/kvm/pci.h
+++ b/arch/s390/kvm/pci.h
@@ -22,6 +22,8 @@ struct kvm_zdev {
 	struct kvm *kvm;
 	struct zpci_fib fib;
 	struct list_head entry;
+	struct user_struct *user_account;
+	struct mm_struct *mm_account;
 };
 
 struct zpci_gaite {
-- 
2.43.0

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

[sashiko-bot]

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

New issues:
- [High] Missing `unaccount_mem()` on the `airq_iv_alloc_bit` error path in `kvm_s390_pci_aif_enable()` leaks `mm_struct` and `user_struct` references.

Pre-existing issues:
- [High] `kvm_s390_pci_aif_enable()` silently swallows error codes on failure paths, returning success to userspace.
--

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

KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

The account_mem() and unaccount_mem() functions leak struct user_struct
references by not calling free_uid(). They also incorrectly accounted pages
against current->mm, which might not be the original context during unpinning.
This patch stores the pinning process user_struct and mm_struct to properly
free these resources.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index d2a11cdf6941d..44c00e5e32c57 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
[ ... ]
> @@ -279,7 +300,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;

[Severity: High]
This is a pre-existing issue, but does this silently swallow the error code
if account_mem() fails?

If account_mem() returns an error like -ENOMEM, we jump to unpin2, but the
return value isn't assigned to the local rc variable.

Does this also happen if airq_iv_alloc_bit() fails later in the function? If
the error code isn't assigned to rc before jumping to unlock,
kvm_s390_pci_aif_enable() would return 0 (success) to userspace even though
interrupt forwarding failed to initialize. Could this leave the guest hanging
while waiting for interrupts?

[Severity: High]
Does this error path leak the mm_struct and user_struct references?

When account_mem() succeeds, it takes references via mmgrab() and get_uid(),
and stores them in kzdev. If a subsequent operation fails later in
kvm_s390_pci_aif_enable() (such as airq_iv_alloc_bit() failing), the error
path jumps to unlock:

	bit = airq_iv_alloc_bit(aift->sbv);
	if (bit == -1UL)
		goto unlock;
...
unlock:
	mutex_unlock(&aift->aift_lock);
unpin2:

This skips calling unaccount_mem(), which would drop the references and memory
accounting limits. Since kzdev->fib.fmt0.aibv remains 0, the device teardown
path will also skip unaccounting this memory.

Could this be repeatedly triggered (e.g. by exhausting airq_iv bits) to
reliably leak references on each attempt, eventually overflowing the 32-bit
counters and causing a use-after-free?

>  
>  	/* AISB must be allocated before we can fill in GAITE */

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

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

[Matthew Rosato]

On 7/23/26 2:34 PM, Farhan Ali wrote:
> 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")
> Cc: stable@vger.kernel.org
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

With one note below:


>  
>  static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
> @@ -279,7 +300,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;
>  

Sashiko marked a failure to unaccount on error paths after this point as
a regression, but it's a pre-existing issue AND resolved with patch 3 of
this series.

>  	/* AISB must be allocated before we can fill in GAITE */
> @@ -400,7 +421,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..fdf8c7bf4ed0 100644
> --- a/arch/s390/kvm/pci.h
> +++ b/arch/s390/kvm/pci.h
> @@ -22,6 +22,8 @@ struct kvm_zdev {
>  	struct kvm *kvm;
>  	struct zpci_fib fib;
>  	struct list_head entry;
> +	struct user_struct *user_account;
> +	struct mm_struct *mm_account;
>  };
>  
>  struct zpci_gaite {

[PATCH v5 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 an error code,
causing the function to return 0 on failure. It also failed to rollback
memory accounting on failure. Fix both by propagating an error code on
failure and calling unaccount_mem() in the cleanup path.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Cc: stable@vger.kernel.org
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
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 44c00e5e32c5..f58d7ec0bc7e 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -300,14 +300,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 |
@@ -351,6 +354,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

Re: [PATCH v5 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 an error code,
> causing the function to return 0 on failure. It also failed to rollback
> memory accounting on failure. Fix both by propagating an error code on
> failure and calling unaccount_mem() in the cleanup path.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Cc: stable@vger.kernel.org
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> 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/20260723183440.3271-1-alifm@linux.ibm.com?part=3

[PATCH v5 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
dereferenced 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")
Cc: stable@vger.kernel.org
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
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 f58d7ec0bc7e..602534e3cc10 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -317,6 +317,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;
 
@@ -353,6 +358,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

Re: [PATCH v5 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
> dereferenced 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")
> Cc: stable@vger.kernel.org
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> 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/20260723183440.3271-1-alifm@linux.ibm.com?part=4

[PATCH v5 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
an 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. While at it, remove dead code that stored FIB
values that were never referenced.

As part of the cleanup, we are also holding the aift_lock while executing 
the MPCIFC instruction. Though this is not strictly necessary, it means 
we don't have to drop and re-acquire in the error case.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Cc: stable@vger.kernel.org
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
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 602534e3cc10..ca8ef76bcf6b 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -344,19 +344,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

Re: [PATCH v5 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
> an 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. While at it, remove dead code that stored FIB
> values that were never referenced.
> 
> As part of the cleanup, we are also holding the aift_lock while executing
> the MPCIFC instruction. Though this is not strictly necessary, it means
> we don't have to drop and re-acquire in the error case.
> 
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Cc: stable@vger.kernel.org
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> 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/20260723183440.3271-1-alifm@linux.ibm.com?part=5

[PATCH v5 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")
Cc: stable@vger.kernel.org
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index ca8ef76bcf6b..25185d04ca28 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -244,7 +244,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;
@@ -271,6 +271,15 @@ 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);
+	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
+	/* AIBV cannot span more than 1 page */
+	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);
@@ -286,6 +295,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) {
+		/* AISB must be dword aligned */
+		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 v5 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[sashiko-bot]

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

New issues:
- [Critical] The AIBV size calculation underestimates the required memory footprint when the bit vector has a non-zero bit offset, allowing an unaligned AIBV to silently cross a page boundary and cause host memory corruption via hardware DMA.

Pre-existing issues:
- [High] Guest AIBV and AISB pages are unpinned without being marked dirty, leading to potential silent data loss of hardware DMA writes.
--

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

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

The AIBV holds one bit per MSI-X vector for a given function. If the size
of the AIBV exceeds a single page boundary, or if the AISB address is not
8-byte aligned, the request is rejected to prevent unsafe pinning of the
guest pages.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index ca8ef76bcf6b3..25185d04ca28e 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c

[ ... ]

> @@ -271,6 +271,15 @@ 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);
> +	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);

[Severity: Critical]
Does this calculation underestimate the true byte footprint of the bit vector
when aibvo is non-zero?

If a guest provides msi_vecs = 64, aibvo = 63, and an aibv GPA with a page
offset of 4081, the calculated size is 15 bytes instead of 16. The npages
calculation yields DIV_ROUND_UP(4081 + 15, 4096) = 1, which incorrectly
passes the npages > 1 boundary check.

This would result in KVM pinning only one page, but the hardware could access
the 16th byte, writing to the first byte of the next unpinned host physical
page, which could lead to host memory corruption.

> +	/* AIBV cannot span more than 1 page */
> +	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);

[Severity: High]
This isn't a bug introduced by this patch, but does the unpinning of guest
AIBV and AISB pages in kvm_s390_pci_aif_disable() need to mark the pages
as dirty?

The pages are pinned with FOLL_WRITE in kvm_s390_pci_aif_enable(), and PCI
hardware modifies the pinned pages directly via DMA. When unpinning these
pages later in kvm_s390_pci_aif_disable():

arch/s390/kvm/pci.c:kvm_s390_pci_aif_disable() {
    ...
	if (vpage) {
		unpin_user_page(vpage);
		pcount++;
	}
	if (spage) {
		unpin_user_page(spage);
		pcount++;
	}
    ...
}

Without marking them dirty, if the host's memory management subsystem
later reclaims or swaps out these pages, the hardware DMA writes could be
silently lost. Could this lead to lost interrupts and guest malfunctions?

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

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

[Farhan Ali]

On 7/23/2026 11:48 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Critical] The AIBV size calculation underestimates the required memory footprint when the bit vector has a non-zero bit offset, allowing an unaligned AIBV to silently cross a page boundary and cause host memory corruption via hardware DMA.
>
> Pre-existing issues:
> - [High] Guest AIBV and AISB pages are unpinned without being marked dirty, leading to potential silent data loss of hardware DMA writes.
> --
>
> --- Patch [6]: [PATCH v5 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages ---
> commit ce58b81776049069915a6d5914d02bc982861691
> Author: Farhan Ali <alifm@linux.ibm.com>
>
> KVM: s390: pci: Validate AIBV and AISB before pinning guest pages
>
> The AIBV holds one bit per MSI-X vector for a given function. If the size
> of the AIBV exceeds a single page boundary, or if the AISB address is not
> 8-byte aligned, the request is rejected to prevent unsafe pinning of the
> guest pages.
>
>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>> index ca8ef76bcf6b3..25185d04ca28e 100644
>> --- a/arch/s390/kvm/pci.c
>> +++ b/arch/s390/kvm/pci.c
> [ ... ]
>
>> @@ -271,6 +271,15 @@ 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);
>> +	npages = DIV_ROUND_UP((fib->fmt0.aibv & ~PAGE_MASK) + size, PAGE_SIZE);
> [Severity: Critical]
> Does this calculation underestimate the true byte footprint of the bit vector
> when aibvo is non-zero?
>
> If a guest provides msi_vecs = 64, aibvo = 63, and an aibv GPA with a page
> offset of 4081, the calculated size is 15 bytes instead of 16. The npages
> calculation yields DIV_ROUND_UP(4081 + 15, 4096) = 1, which incorrectly
> passes the npages > 1 boundary check.
>
> This would result in KVM pinning only one page, but the hardware could access
> the 16th byte, writing to the first byte of the next unpinned host physical
> page, which could lead to host memory corruption.

I think Sashiko does have a valid point here (sigh...). The right fix 
would be to correctly round up both msi_vecs and aibvo bits so:

size = BITS_TO_LONGS(msi_vecs + fib->fmt0.aibvo) * sizeof(unsigned long);

and remove size += (fib->fmt0.aibvo / 8);

Thanks

Farhan

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

[Matthew Rosato]

>> [Severity: Critical]
>> Does this calculation underestimate the true byte footprint of the bit
>> vector
>> when aibvo is non-zero?
>>
>> If a guest provides msi_vecs = 64, aibvo = 63, and an aibv GPA with a
>> page
>> offset of 4081, the calculated size is 15 bytes instead of 16. The npages
>> calculation yields DIV_ROUND_UP(4081 + 15, 4096) = 1, which incorrectly
>> passes the npages > 1 boundary check.
>>
>> This would result in KVM pinning only one page, but the hardware could
>> access
>> the 16th byte, writing to the first byte of the next unpinned host
>> physical
>> page, which could lead to host memory corruption.
> 
> I think Sashiko does have a valid point here (sigh...). The right fix
> would be to correctly round up both msi_vecs and aibvo bits so:
> 
> size = BITS_TO_LONGS(msi_vecs + fib->fmt0.aibvo) * sizeof(unsigned long);
> 
> and remove size += (fib->fmt0.aibvo / 8);
>
Yeah, that sounds right to me.