[PATCH v3 0/4] KVM s390x PCI fixes

[PATCH v3 0/4] KVM s390x PCI fixes

[Farhan Ali]

Hi,

This series attempts to fix some the pre-existing issues[1] found by sashiko.
In v1 of the series, sashiko correctly pointed out some issues with the fix
for AISB/AIBV spanning multiple pages and resource leaks that can happen
over multiple ioctl calls for the same device. Fixing these will require 
some more rework and will be address later.  

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

Thanks
Farhan

ChangeLog
---------
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 (4):
  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

 arch/s390/kvm/pci.c | 82 ++++++++++++++++++++++++++++++++++-----------
 arch/s390/kvm/pci.h |  2 ++
 2 files changed, 65 insertions(+), 19 deletions(-)

-- 
2.43.0

[PATCH v3 1/4] 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 720bb58cabe2..dd17f8a7b473 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,
@@ -275,7 +293,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 */
@@ -396,7 +414,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

Re: [PATCH v3 1/4] 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] Repeated KVM_S390_ZPCIOP_REG_AEN ioctl calls overwrite kzdev->mm_account and kzdev->user_account without dropping previous references, permanently leaking mm_struct and user_struct.

Pre-existing issues:
- [High] kvm_s390_pci_aif_enable returns silent success (0) when memory accounting fails.
--

--- Patch [1]: [PATCH v3 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages ---
commit 200dcce54ba1446ee99b11172f6ae91eeaacea27
Author: Farhan Ali <alifm@linux.ibm.com>

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

This commit changes the memory accounting functions to store the pinning
process's user_struct and mm_struct directly in the kvm_zdev structure.
This ensures that unaccount_mem() correctly decrements the references
from the original process context, rather than the context of the caller
performing the unpinning.

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

[ ... ]

> +	mmgrab(current->mm);
>  	atomic64_add(nr_pages, &current->mm->pinned_vm);
> +	kzdev->user_account = user;
> +	kzdev->mm_account = current->mm;

[Severity: High]
If userspace repeatedly calls the ioctl that triggers
kvm_s390_pci_aif_enable() for a device, can this overwrite
kzdev->mm_account and kzdev->user_account without dropping the previous
references?

Looking at kvm_s390_pci_aif_enable(), there doesn't appear to be a check to
see if the device is already registered before calling account_mem(). Calling
it multiple times would execute mmgrab() and get_uid() on each invocation,
unconditionally overwriting the pointers here and leaking the previous
mm_struct and user_struct.

>  
>  	return 0;
> +
> +out:
> +	free_uid(user);
> +	return rc;
>  }
>  
>  static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
> @@ -275,7 +293,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 error path return 0 (success) to
userspace when memory accounting fails?

In kvm_s390_pci_aif_enable(), the return code rc is initialized to 0. If
account_mem() fails and returns -ENOMEM, the code jumps to the unpin2
cleanup path without assigning the error code to rc.

This seems like it would mislead userspace into believing the interrupt
forwarding setup succeeded.

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

[PATCH v3 2/4] 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 dd17f8a7b473..9fdb6e383b18 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -293,14 +293,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 |
@@ -344,6 +347,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 v3 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling KVM_S390_ZPCIOP_REG_AEN multiple times on the same device causes severe, unbounded resource leaks and state corruption.
--

--- Patch [2]: [PATCH v3 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting ---
commit bb1306d6a0da97f9d5b884d1e67a7c8d12b6a908
Author: Farhan Ali <alifm@linux.ibm.com>

KVM: s390: pci: Fix missing error codes and memory unaccounting

This commit fixes error paths in kvm_s390_pci_aif_enable() to properly
propagate error codes on failure and rollback memory accounting.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index dd17f8a7b4734..9fdb6e383b189 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c

[ ... ]

> @@ -293,14 +293,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);

[Severity: High]
This is a pre-existing issue, but can calling the KVM_S390_ZPCIOP_REG_AEN ioctl
multiple times on the same device cause resource leaks?

Looking at kvm_s390_pci_aif_enable(), there doesn't appear to be a check to
verify if Adapter Interruption Forwarding (AIF) is already enabled for the
device. If it is called repeatedly, it seems account_mem() will blindly
overwrite kzdev->user_account and kzdev->mm_account:

arch/s390/kvm/pci.c:account_mem() {
    ...
    kzdev->user_account = user;
    kzdev->mm_account = current->mm;

    return 0;
}

Does this leave the previously allocated airq_iv and sbv bits leaked, along
with the previous user_struct and mmgrab references? It also looks like this
would permanently elevate the locked_vm and pinned_vm counts for those pages,
which could potentially lead to memory exhaustion on the host.

> +	if (rc)
>  		goto unpin2;
>  
>  	/* AISB must be allocated before we can fill in GAITE */
>  	mutex_lock(&aift->aift_lock);

[ ... ]

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

[PATCH v3 3/4] 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 9fdb6e383b18..85f4fb5d36b3 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -310,6 +310,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;
 
@@ -346,6 +351,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 v3 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Unbounded resource and memory leak on duplicate AEN registrations due to missing state check.
--

--- Patch [3]: [PATCH v3 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure ---
commit 9b0515651fd32d61d1da226af14173f1630435f4
Author: Farhan Ali <alifm@linux.ibm.com>

KVM: s390: pci: Fix NULL dereference on AIBV allocation failure

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.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 9fdb6e383b189..85f4fb5d36b38 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -310,6 +310,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;
> +	}
> +

[Severity: High]
This isn't a bug introduced by this patch, but does kvm_s390_pci_aif_enable()
lack a state check to prevent duplicate registrations?

If a guest VM or userspace process issues the KVM_S390_ZPCIOP_REG_AEN ioctl
multiple times for the same device, it appears we proceed through the
function and allocate new resources without freeing previous ones.

Could this result in overwriting pointers like zdev->aibv and zdev->aisb,
along with overwriting account info in account_mem():

arch/s390/kvm/pci.c:account_mem() {
        ...
        kzdev->user_account = user;
        kzdev->mm_account = current->mm;
        ...
}

If the pointers are overwritten on a duplicate call, does this permanently
leak the host resources, including previously pinned pages,
mm_struct/user_struct reference counts, and airq_iv structures?

[ ... ]

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

[PATCH v3 4/4] 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 85f4fb5d36b3..d328bbd0dd99 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -337,19 +337,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 v3 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The `aisbo` (Adapter Interrupt Summary Bit Offset) provided by userspace is not bounds-checked, leading to a host kernel memory out-of-bounds write.
- [Critical] The `aibv` address and `noi` (number of interrupts) provided by userspace are not bounds-checked, allowing hardware DMA to write out-of-bounds into unpinned host physical memory.
- [High] Missing check for already-enabled AIF allows massive resource leak, accounting exhaustion, and subsequent Use-After-Free on double registration.
--

--- Patch [4]: [PATCH v3 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure ---
commit 54ecbcfac4858add9c2dd09fc30b8b84a8fbfca3
Author: Farhan Ali <alifm@linux.ibm.com>

KVM: s390: pci: Fix resource leak on IRQ registration failure

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.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 85f4fb5d36b38..d328bbd0dd997 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -337,19 +337,32 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,

[Severity: High]
This is a pre-existing issue, but does kvm_s390_pci_zpci_reg_aen() prevent
double registration?

If a malicious or buggy userspace repeatedly calls the KVM_S390_ZPCIOP_REG_AEN
ioctl on the same PCI device, kvm_s390_pci_aif_enable() unconditionally
allocates new resources:

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    bit = airq_iv_alloc_bit(aift->sbv);
    ...
    zdev->aibv = airq_iv_create(...);
    ...
}

Because it doesn't check if AIF is already enabled, it blindly pins new host
pages and overwrites the old zdev->aibv and zdev->aisb pointers.

Could this permanently leak the previous allocations and repeatedly increment
the memory accounting on user->locked_vm until it exhausts the limit?


[Severity: Critical]
This is a pre-existing issue, but does this code bounds-check the DMA length
against the pinned page?

kvm_s390_pci_aif_enable() calculates the physical base address for the
interrupt bit vector using a user-supplied page offset:

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    gaddr = page_to_phys(aibv_page) + (fib->fmt0.aibv & ~PAGE_MASK);
    fib->fmt0.aibv = gaddr;
    ...
}

This physical address is passed directly to the hardware via
kvm_zpci_set_airq().

If the guest places the base address near the end of the page (e.g., offset
4095) and zdev->max_msi requires 2+ bytes, will the hardware DMA cross the
single pinned page's boundary?

Could this silently corrupt adjacent unpinned host physical pages?


[Severity: Critical]
This isn't a bug introduced by this patch, but is the aisbo value from
userspace properly bounds-checked?

In kvm_s390_pci_aif_enable(), gaite->aisbo is set directly from the
user-controlled fib->fmt0.aisbo:

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    if (fib->fmt0.sum == 1) {
        gaite->aisbo = fib->fmt0.aisbo;
        gaite->aisb = virt_to_phys(page_address(aisb_page) +
                                   (fib->fmt0.aisb & ~PAGE_MASK));
    }
    ...
}

Upon an adapter interrupt, aen_host_forward() executes an out-of-bounds
bitwise write operation:

arch/s390/kvm/interrupt.c:aen_host_forward() {
    ...
    set_bit_inv(gaite->aisbo, phys_to_virt(gaite->aisb));
    ...
}

Since gaite->aisbo can be up to 255 (a 31-byte offset) and the guest offset
within the page can be up to 4095, the calculation (offset) + (aisbo / 8)
can reach 4126.

Could this cross the 4096-byte page boundary and perform a bitwise write up to
31 bytes out-of-bounds into the adjacent physical page?

>  	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;
> +	}

[ ... ]

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