* [PATCH v2 0/4] KVM s390x PCI fixes
@ 2026-07-16 17:52 Farhan Ali
2026-07-16 17:52 ` [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages Farhan Ali
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Farhan Ali @ 2026-07-16 17:52 UTC (permalink / raw)
To: linux-kernel, linux-s390, kvm; +Cc: alifm, mjrosato, borntraeger, farman
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
---------
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 | 75 +++++++++++++++++++++++++++++++++++++--------
arch/s390/kvm/pci.h | 2 ++
2 files changed, 64 insertions(+), 13 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
2026-07-16 17:52 [PATCH v2 0/4] KVM s390x PCI fixes Farhan Ali
@ 2026-07-16 17:52 ` Farhan Ali
2026-07-16 18:08 ` sashiko-bot
2026-07-16 17:52 ` [PATCH v2 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting Farhan Ali
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Farhan Ali @ 2026-07-16 17:52 UTC (permalink / raw)
To: linux-kernel, linux-s390, kvm; +Cc: alifm, mjrosato, borntraeger, farman
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, ¤t->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, ¤t->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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting
2026-07-16 17:52 [PATCH v2 0/4] KVM s390x PCI fixes Farhan Ali
2026-07-16 17:52 ` [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages Farhan Ali
@ 2026-07-16 17:52 ` Farhan Ali
2026-07-16 18:06 ` sashiko-bot
2026-07-16 17:52 ` [PATCH v2 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure Farhan Ali
2026-07-16 17:52 ` [PATCH v2 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure Farhan Ali
3 siblings, 1 reply; 9+ messages in thread
From: Farhan Ali @ 2026-07-16 17:52 UTC (permalink / raw)
To: linux-kernel, linux-s390, kvm; +Cc: alifm, mjrosato, borntraeger, farman
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure
2026-07-16 17:52 [PATCH v2 0/4] KVM s390x PCI fixes Farhan Ali
2026-07-16 17:52 ` [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages Farhan Ali
2026-07-16 17:52 ` [PATCH v2 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting Farhan Ali
@ 2026-07-16 17:52 ` Farhan Ali
2026-07-16 18:06 ` sashiko-bot
2026-07-16 17:52 ` [PATCH v2 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure Farhan Ali
3 siblings, 1 reply; 9+ messages in thread
From: Farhan Ali @ 2026-07-16 17:52 UTC (permalink / raw)
To: linux-kernel, linux-s390, kvm; +Cc: alifm, mjrosato, borntraeger, farman
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure
2026-07-16 17:52 [PATCH v2 0/4] KVM s390x PCI fixes Farhan Ali
` (2 preceding siblings ...)
2026-07-16 17:52 ` [PATCH v2 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure Farhan Ali
@ 2026-07-16 17:52 ` Farhan Ali
2026-07-16 18:05 ` sashiko-bot
3 siblings, 1 reply; 9+ messages in thread
From: Farhan Ali @ 2026-07-16 17:52 UTC (permalink / raw)
To: linux-kernel, linux-s390, kvm; +Cc: alifm, mjrosato, borntraeger, farman
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 85f4fb5d36b3..f85d856b58c6 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -345,11 +345,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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure
2026-07-16 17:52 ` [PATCH v2 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure Farhan Ali
@ 2026-07-16 18:05 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-07-16 18:05 UTC (permalink / raw)
To: Farhan Ali
Cc: Heiko Carstens, kvm, linux-s390, Vasily Gorbik, Alexander Gordeev
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The newly added error path for `kvm_zpci_set_airq()` failure uses the wrong ISC value when unregistering the guest ISC, leaking the guest ISC reference and potentially corrupting the host NISC reference count.
Pre-existing issues:
- [High] Missing state check allows multiple concurrent `KVM_S390_ZPCIOP_REG_AEN` ioctls, causing memory leaks of pinned pages, `mmgrab` references, and exhaustion of global `airq_iv` structures.
--
--- Patch [4]: [PATCH v2 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure ---
commit 9f5448d2e142dd7c73a306c74cf6bad6443d3218
Author: Farhan Ali <alifm@linux.ibm.com>
KVM: s390: pci: Fix resource leak on IRQ registration failure
This commit adds cleanup code in kvm_s390_pci_aif_enable() to free the
associated adapter interrupt resources and unpin any pinned pages if
kvm_zpci_set_airq() fails. It does this by adding an error path that
resets the state and falls through to the existing cleanup labels.
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 85f4fb5d36b38..f85d856b58c65 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -345,11 +345,29 @@ 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_aif_enable() lack a check
to verify if interrupt forwarding is already enabled?
If a userspace process repeatedly calls the registration ioctl on a single
KVM PCI device without deregistering first, the kernel appears to
unconditionally allocate new resources like pinned pages and airq_iv
structures.
Because the pointers to the previous allocations are overwritten without being
freed, could this allow a user to exhaust the host-wide global adapter
interrupt table bitmask (aift->sbv) and permanently leak pinned memory?
> /* 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;
[Severity: High]
Does this new error path unregister the host ISC instead of the guest ISC?
Earlier in this function, the guest ISC in fib->fmt0.isc is replaced with the
host ISC:
fib->fmt0.isc = gisc;
When kvm_zpci_set_airq() fails, the code now falls through to the out label:
out:
kvm_s390_gisc_unregister(kvm, fib->fmt0.isc);
Since fib->fmt0.isc was overwritten, wouldn't this decrement the host ISC
reference count instead of the guest ISC reference count?
This could corrupt the host's alert mask and potentially disable interrupt
forwarding for other independent PCI devices sharing the host ISC, while
leaving the guest ISC reference permanently leaked.
>
> free_aisb:
> airq_iv_free_bit(aift->sbv, zdev->aisb);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716175241.1039-1-alifm@linux.ibm.com?part=4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting
2026-07-16 17:52 ` [PATCH v2 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting Farhan Ali
@ 2026-07-16 18:06 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-07-16 18:06 UTC (permalink / raw)
To: Farhan Ali
Cc: Heiko Carstens, kvm, linux-s390, Alexander Gordeev, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Repeated KVM_S390_ZPCIOP_REG_AEN ioctls on the same device leak previously allocated resources and references.
--
--- Patch [2]: [PATCH v2 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting ---
commit d30a6fbb3e26fd01708c6bf05d003dbb34124513
Author: Farhan Ali <alifm@linux.ibm.com>
KVM: s390: pci: Fix missing error codes and memory unaccounting
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.
> 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);
> + 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 |
[Severity: High]
This isn't a bug introduced by this patch, but does calling
KVM_S390_ZPCIOP_REG_AEN multiple times on the same device leak previously
allocated resources?
If a user-space process calls the ioctl multiple times,
kvm_s390_pci_aif_enable() appears to unconditionally allocate and pin new
resources.
Will this overwrite zdev->aisb and zdev->aibv without freeing the
previously allocated airq_iv structures?
Additionally, when this calls account_mem(), does it overwrite
kzdev->user_account and kzdev->mm_account without releasing the previous
references, such as by using free_uid and mmdrop?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716175241.1039-1-alifm@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure
2026-07-16 17:52 ` [PATCH v2 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure Farhan Ali
@ 2026-07-16 18:06 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-07-16 18:06 UTC (permalink / raw)
To: Farhan Ali
Cc: Alexander Gordeev, kvm, Vasily Gorbik, Heiko Carstens, linux-s390
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Guest-controlled AIBV can cross a page boundary, leading to host memory corruption.
- [High] Missing cleanup when kvm_zpci_set_airq() fails in kvm_s390_pci_aif_enable().
- [High] Missing state check allows unbounded resource leaks via repeated KVM_S390_ZPCIOP_REG_AEN ioctls.
--
--- Patch [3]: [PATCH v2 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure ---
commit 6cf8d654914206b2e0a5d8af6cb9a7272f96c280
Author: Farhan Ali <alifm@linux.ibm.com>
KVM: s390: pci: Fix NULL dereference on AIBV allocation failure
This commit adds a check to ensure airq_iv_create() does not return NULL,
preventing a subsequent NULL pointer dereference in kvm_zpci_set_airq().
If the allocation fails, it frees the previously allocated AISB bit and
returns -ENOMEM.
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -240,6 +240,9 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
> bool assist)
> {
[Severity: High]
This isn't a bug introduced by this patch, but is there a missing state
check here before proceeding with the allocations?
If the ioctl for AEN registration is called repeatedly for the same PCI
device, this function appears to unconditionally pin new memory pages,
allocate new aift->sbv bits, and overwrite the previous pointers in zdev
without freeing them. Could this lead to exhausting aift->sbv bits and
leaking pinned memory pages?
> struct page *pages[1], *aibv_page, *aisb_page = NULL;
> unsigned int msi_vecs, idx;
[ ... ]
> /* Replace AIBV address */
> 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);
> srcu_read_unlock(&kvm->srcu, idx);
> if (npages < 1) {
> rc = -EIO;
> goto out;
> }
> aibv_page = pages[0];
> pcount++;
> 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 safely handle the
case where the bit vector crosses a page boundary?
The code pins exactly one page, but the interrupt bit vector size depends
on msi_vecs. If fib->fmt0.aibv is provided near the end of a page
boundary (for example, offset 4088) and msi_vecs requires more bytes than
what is left in the page, will the hardware and host software read or write
out-of-bounds into the adjacent unmapped physical page?
[ ... ]
> @@ -346,6 +351,9 @@ 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;
>
> /* Issue the clp to setup the irq now */
> rc = kvm_zpci_set_airq(zdev);
> return rc;
[Severity: High]
This isn't a bug introduced by this patch, but does returning directly
here bypass the required cleanup logic?
If kvm_zpci_set_airq() fails, returning rc directly means we skip the
free_aisb and unlock labels. Could this leave the pages pinned, the
accounted memory incremented, and the aift->sbv bits allocated, causing
these resources to be permanently lost?
>
> +free_aisb:
> + airq_iv_free_bit(aift->sbv, zdev->aisb);
> + zdev->aisb = 0;
> unlock:
> if (pcount > 0)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716175241.1039-1-alifm@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
2026-07-16 17:52 ` [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages Farhan Ali
@ 2026-07-16 18:08 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-07-16 18:08 UTC (permalink / raw)
To: Farhan Ali
Cc: linux-s390, Alexander Gordeev, kvm, Heiko Carstens, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Sequential calls to the KVM_S390_ZPCIOP_REG_AEN ioctl on the same device overwrite kzdev->user_account and kzdev->mm_account without dropping previous references, causing a permanent leak of mm_struct references and pinned pages.
--
--- Patch [1]: [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages ---
commit cbbebe9ad7b7a6af2ee4a68dd67b2e520770bd20
Author: Farhan Ali <alifm@linux.ibm.com>
KVM: s390: pci: Fix memory accounting for pinned/unpinned pages
This patch addresses a bug where unaccount_mem() could be called from a
different process context than the one that initially pinned the pages. It
resolves this by storing the pinning process's user_struct and mm_struct
when accounting for pinned pages, so they can be properly freed later.
> 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
[ ... ]
> @@ -206,17 +220,24 @@ static inline int account_mem(struct kvm_zdev *kzdev, unsigned long nr_pages)
> 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, ¤t->mm->pinned_vm);
> + kzdev->user_account = user;
> + kzdev->mm_account = current->mm;
[Severity: High]
If kvm_s390_pci_aif_enable() is called multiple times for the same device,
will this overwrite kzdev->user_account and kzdev->mm_account and leak
the previously acquired references?
It looks like kvm_s390_pci_aif_enable() doesn't check if the device is
already enabled before proceeding. If a userspace process calls the
KVM_S390_ZPCIOP_REG_AEN ioctl twice on the same device, the first call
will successfully populate these pointers.
The second call would then overwrite them here, leaking the original
mm_struct reference and permanently stranding the initially pinned pages.
If that second call subsequently fails, unaccount_mem() drops the new
reference and sets the pointers to NULL, leaving the original references
permanently orphaned.
Should there be a check in kvm_s390_pci_aif_enable() to bail out if it
is already enabled, or a check here to ensure we aren't overwriting an
existing reference?
>
> return 0;
> +
> +out:
> + free_uid(user);
> + return rc;
> }
>
> static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716175241.1039-1-alifm@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-16 18:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 17:52 [PATCH v2 0/4] KVM s390x PCI fixes Farhan Ali
2026-07-16 17:52 ` [PATCH v2 1/4] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages Farhan Ali
2026-07-16 18:08 ` sashiko-bot
2026-07-16 17:52 ` [PATCH v2 2/4] KVM: s390: pci: Fix missing error codes and memory unaccounting Farhan Ali
2026-07-16 18:06 ` sashiko-bot
2026-07-16 17:52 ` [PATCH v2 3/4] KVM: s390: pci: Fix NULL dereference on AIBV allocation failure Farhan Ali
2026-07-16 18:06 ` sashiko-bot
2026-07-16 17:52 ` [PATCH v2 4/4] KVM: s390: pci: Fix resource leak on IRQ registration failure Farhan Ali
2026-07-16 18:05 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox