Linux s390 Architecture development
 help / color / mirror / Atom feed
* [GIT PULL 0/5] KVM: s390: Fixes for 7.2
@ 2026-07-13  8:30 Christian Borntraeger
  2026-07-13  8:30 ` [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure Christian Borntraeger
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Christian Borntraeger @ 2026-07-13  8:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KVM, Christian Borntraeger, linux-s390, Janosch Frank,
	David Hildenbrand, Claudio Imbrenda, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Matthew Rosato, Farhan Ali,
	Eric Farman, Sven Schnelle

From: Christian Borntraeger <borntraeger@linux.ibm.com>

Paolo,

please pull the following changes since commit 8cdeaa50eae8dad34885515f62559ee83e7e8dda:

  Linux 7.2-rc2 (2026-07-05 14:44:06 -1000)

from the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-master-7.2-1

for you to fetch changes up to 3e3aa6da87d30a0064a17b836685cd43c90a3572:

  KVM: s390: pci: Fix handling of AIF enable without AISB (2026-07-09 16:23:10 +0200)

----------------------------------------------------------------
KVM: s390: Fixes for 7.2

- more gmap KVM memory management fixes
- PCI passthru fixes

----------------------------------------------------------------
Claudio Imbrenda (3):
      KVM: s390: vsie: Avoid potential deadlock with real spaces
      KVM: s390: Fix dat_crste_walk_range() early return
      KVM: s390: Improve kvm_s390_vm_stop_migration()

Haoxiang Li (1):
      KVM: s390: pci: Fix GISC refcount leak on AIF enable failure

Matthew Rosato (1):
      KVM: s390: pci: Fix handling of AIF enable without AISB

 arch/s390/kvm/dat.c      |  2 ++
 arch/s390/kvm/gmap.c     |  7 ++++++-
 arch/s390/kvm/kvm-s390.c |  6 ++++--
 arch/s390/kvm/pci.c      | 12 +++++++++---
 4 files changed, 21 insertions(+), 6 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure
  2026-07-13  8:30 [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
@ 2026-07-13  8:30 ` Christian Borntraeger
  2026-07-13  8:48   ` sashiko-bot
  2026-07-13  8:30 ` [GIT PULL 2/5] KVM: s390: vsie: Avoid potential deadlock with real spaces Christian Borntraeger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Christian Borntraeger @ 2026-07-13  8:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KVM, Christian Borntraeger, linux-s390, Janosch Frank,
	David Hildenbrand, Claudio Imbrenda, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Matthew Rosato, Farhan Ali,
	Eric Farman, Sven Schnelle

From: Haoxiang Li <haoxiang_li2024@163.com>

kvm_s390_gisc_register() registers the guest ISC before pinning
the guest interrupt forwarding pages and allocating the AISB bit.
If any of the later setup steps fails, the function unwinds the
pinned pages and other local state, but does not unregister the
GISC reference. Add the missing kvm_s390_gisc_unregister() to the
error unwind path.

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Tested-by: Matthew Rosato <mjrosato@linux.ibm.com>
Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260624061910.2794734-1-haoxiang_li2024@163.com>
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 5b075c38998e3..686113be05309 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -328,6 +328,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 unpin1:
 	unpin_user_page(aibv_page);
 out:
+	kvm_s390_gisc_unregister(kvm, fib->fmt0.isc);
 	return rc;
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [GIT PULL 2/5] KVM: s390: vsie: Avoid potential deadlock with real spaces
  2026-07-13  8:30 [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
  2026-07-13  8:30 ` [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure Christian Borntraeger
@ 2026-07-13  8:30 ` Christian Borntraeger
  2026-07-13  8:30 ` [GIT PULL 3/5] KVM: s390: Fix dat_crste_walk_range() early return Christian Borntraeger
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Christian Borntraeger @ 2026-07-13  8:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KVM, Christian Borntraeger, linux-s390, Janosch Frank,
	David Hildenbrand, Claudio Imbrenda, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Matthew Rosato, Farhan Ali,
	Eric Farman, Sven Schnelle

From: Claudio Imbrenda <imbrenda@linux.ibm.com>

The natural lock ordering is mmu_lock -> children_lock, but in
gmap_create_shadow() the reverse order is used when handling shadowing
of real address spaces.

Convert the inner locking of kvm->mmu_lock to a trylock; return -EAGAIN
if the lock is busy, and let the caller try again.

This path is not expected to happen in real-life scenarios, so its
performance is not important.

Fixes: a2c17f9270cc ("KVM: s390: New gmap code")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/gmap.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
index 298fbaecec28d..8abb4f55b306b 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -1374,8 +1374,13 @@ struct gmap *gmap_create_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *pare
 			/* Only allow one real-space gmap shadow. */
 			list_for_each_entry(sg, &parent->children, list) {
 				if (sg->guest_asce.r) {
-					scoped_guard(write_lock, &parent->kvm->mmu_lock)
+					if (write_trylock(&parent->kvm->mmu_lock)) {
 						gmap_unshadow(sg);
+						write_unlock(&parent->kvm->mmu_lock);
+					} else {
+						gmap_put(new);
+						return ERR_PTR(-EAGAIN);
+					}
 					break;
 				}
 			}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [GIT PULL 3/5] KVM: s390: Fix dat_crste_walk_range() early return
  2026-07-13  8:30 [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
  2026-07-13  8:30 ` [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure Christian Borntraeger
  2026-07-13  8:30 ` [GIT PULL 2/5] KVM: s390: vsie: Avoid potential deadlock with real spaces Christian Borntraeger
@ 2026-07-13  8:30 ` Christian Borntraeger
  2026-07-13  8:30 ` [GIT PULL 4/5] KVM: s390: Improve kvm_s390_vm_stop_migration() Christian Borntraeger
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Christian Borntraeger @ 2026-07-13  8:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KVM, Christian Borntraeger, linux-s390, Janosch Frank,
	David Hildenbrand, Claudio Imbrenda, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Matthew Rosato, Farhan Ali,
	Eric Farman, Sven Schnelle

From: Claudio Imbrenda <imbrenda@linux.ibm.com>

If a walk entry handler for a lower level returns a value,
dat_crste_walk_range() will not return immediately, but instead loop
again and move to the next entry.

This means that some entries are potentially skipped, and early return
is ignored. Skipped entries might lead to all kinds of issues, given
that the caller expects them to not be skipped. Early return is often
used to interrupt a walk when a rescheduling is needed; if it is
ignored it can lead to stalls.

Fix by breaking from the loop immediately if the walk to a lower level
returned non-zero.

Fixes: 2db149a0a6c5 ("KVM: s390: KVM page table management functions: walks")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/dat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index 5f1960ec982d0..ed4259d176295 100644
--- a/arch/s390/kvm/dat.c
+++ b/arch/s390/kvm/dat.c
@@ -570,6 +570,8 @@ static long dat_crste_walk_range(gfn_t start, gfn_t end, struct crst_table *tabl
 			else if (walk->ops->pte_entry)
 				rc = dat_pte_walk_range(max(start, cur), min(end, next),
 							dereference_pmd(crste.pmd), walk);
+			if (rc)
+				break;
 		}
 	}
 	return rc;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [GIT PULL 4/5] KVM: s390: Improve kvm_s390_vm_stop_migration()
  2026-07-13  8:30 [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
                   ` (2 preceding siblings ...)
  2026-07-13  8:30 ` [GIT PULL 3/5] KVM: s390: Fix dat_crste_walk_range() early return Christian Borntraeger
@ 2026-07-13  8:30 ` Christian Borntraeger
  2026-07-13  8:30 ` [GIT PULL 5/5] KVM: s390: pci: Fix handling of AIF enable without AISB Christian Borntraeger
  2026-07-13 11:34 ` [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
  5 siblings, 0 replies; 9+ messages in thread
From: Christian Borntraeger @ 2026-07-13  8:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KVM, Christian Borntraeger, linux-s390, Janosch Frank,
	David Hildenbrand, Claudio Imbrenda, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Matthew Rosato, Farhan Ali,
	Eric Farman, Sven Schnelle

From: Claudio Imbrenda <imbrenda@linux.ibm.com>

There is no need to clear cmma-dirty state if the VM is not using CMMA.

Skip the CMMA-related code if CMMA is not in use.

Fixes: 6cfd47f91f6a ("KVM: s390: Fix cmma dirty tracking")
Fixes: 190df4a212a7 ("KVM: s390: CMMA tracking, ESSA emulation, migration mode")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 23c817595e28d..150b5dd2170e2 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1280,8 +1280,10 @@ static int kvm_s390_vm_stop_migration(struct kvm *kvm)
 	 * PGSTEs might have cmma_d set.
 	 */
 	WRITE_ONCE(kvm->arch.migration_mode, 0);
-	if (kvm->arch.use_cmma)
-		kvm_s390_sync_request_broadcast(kvm, KVM_REQ_STOP_MIGRATION);
+	if (!kvm->arch.use_cmma)
+		return 0;
+
+	kvm_s390_sync_request_broadcast(kvm, KVM_REQ_STOP_MIGRATION);
 	/* Clear cmma_d on all existing PGSTEs and set cmma_dirty_pages to 0. */
 	gmap_set_cmma_all_clean(kvm->arch.gmap);
 	atomic64_set(&kvm->arch.cmma_dirty_pages, 0);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [GIT PULL 5/5] KVM: s390: pci: Fix handling of AIF enable without AISB
  2026-07-13  8:30 [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
                   ` (3 preceding siblings ...)
  2026-07-13  8:30 ` [GIT PULL 4/5] KVM: s390: Improve kvm_s390_vm_stop_migration() Christian Borntraeger
@ 2026-07-13  8:30 ` Christian Borntraeger
  2026-07-13  8:47   ` sashiko-bot
  2026-07-13 11:34 ` [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
  5 siblings, 1 reply; 9+ messages in thread
From: Christian Borntraeger @ 2026-07-13  8:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KVM, Christian Borntraeger, linux-s390, Janosch Frank,
	David Hildenbrand, Claudio Imbrenda, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Matthew Rosato, Farhan Ali,
	Eric Farman, Sven Schnelle

From: Matthew Rosato <mjrosato@linux.ibm.com>

When a guest seeks to register IRQs without a summary bit specified,
ensure that the associated GAITE then stores 0 for the guest AISB
location instead of virt_to_phys(page_address(NULL)).

Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
Cc: stable@vger.kernel.org
Reviewed-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/pci.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 686113be05309..720bb58cabe2e 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -300,9 +300,14 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 
 	gaite->gisc = fib->fmt0.isc;
 	gaite->count++;
-	gaite->aisbo = fib->fmt0.aisbo;
-	gaite->aisb = virt_to_phys(page_address(aisb_page) + (fib->fmt0.aisb &
-							      ~PAGE_MASK));
+	if (fib->fmt0.sum == 1) {
+		gaite->aisbo = fib->fmt0.aisbo;
+		gaite->aisb = virt_to_phys(page_address(aisb_page) +
+					   (fib->fmt0.aisb & ~PAGE_MASK));
+	} else {
+		gaite->aisbo = 0;
+		gaite->aisb = 0;
+	}
 	aift->kzdev[zdev->aisb] = zdev->kzdev;
 	spin_unlock_irq(&aift->gait_lock);
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [GIT PULL 5/5] KVM: s390: pci: Fix handling of AIF enable without AISB
  2026-07-13  8:30 ` [GIT PULL 5/5] KVM: s390: pci: Fix handling of AIF enable without AISB Christian Borntraeger
@ 2026-07-13  8:47   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-07-13  8:47 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: kvm, Vasily Gorbik, linux-s390, Heiko Carstens, Alexander Gordeev

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

Pre-existing issues:
- [Critical] Unchecked return value from `airq_iv_create()` causes a host kernel panic in `kvm_zpci_set_airq()`.
- [Critical] Insufficient page pinning for the Adapter Interrupt Bit Vector (AIBV) allows hardware DMA to write beyond the pinned page into arbitrary host memory.
- [Critical] The `set_bit_inv()` call in the interrupt forwarding path can write outside the bounds of the single pinned AISB page due to an unvalidated page offset and bit index.
- [High] Repeated KVM_S390_ZPCIOP_REG_AEN ioctl calls unconditionally overwrite existing AIF allocations, causing unbounded memory and resource exhaustion.
- [High] Error paths in `kvm_s390_pci_aif_enable()` fail to update `rc`, erroneously returning success to userspace when enabling fails.
- [High] Early error paths in `kvm_s390_pci_aif_enable()` fail to rollback successfully allocated resources like GISC registrations and memory quotas.
- [High] `account_mem()` and `unaccount_mem()` perpetually leak `struct user_struct` reference counts.
- [High] `unaccount_mem()` blindly uses `current_user()` and `current->mm`, risking quota corruption when invoked asynchronously or from a different context.
--

--- Patch [1]: [GIT PULL 5/5] KVM: s390: pci: Fix handling of AIF enable without AISB ---
commit e5fbf43dd647bd07ede442137b445e6821685004
Author: Matthew Rosato <mjrosato@linux.ibm.com>
KVM: s390: pci: Fix handling of AIF enable without AISB

When a guest seeks to register IRQs without a summary bit specified, ensure
that the associated GAITE stores 0 for the guest AISB location instead of an
invalid physical address.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 5b075c38998e3..dc748cde4703b 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -300,9 +300,14 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  
>  	gaite->gisc = fib->fmt0.isc;
>  	gaite->count++;
> -	gaite->aisbo = fib->fmt0.aisbo;
> -	gaite->aisb = virt_to_phys(page_address(aisb_page) + (fib->fmt0.aisb &
> -							      ~PAGE_MASK));
> +	if (fib->fmt0.sum == 1) {
> +		gaite->aisbo = fib->fmt0.aisbo;
> +		gaite->aisb = virt_to_phys(page_address(aisb_page) +
> +					   (fib->fmt0.aisb & ~PAGE_MASK));

[Severity: Critical]
This is a pre-existing issue, but does this setup allow for an out-of-bounds
write during interrupt forwarding?

If a guest provides a summary bit address near a page boundary and a large
summary bit offset (up to 255), we might exceed the bounds of the single
pinned page. When a host interrupt arrives, aen_host_forward() executes:

    set_bit_inv(gaite->aisbo, phys_to_virt(gaite->aisb));

Since set_bit_inv() modifies a 64-bit word, could this read and write up to
31 bytes into the adjacent, unpinned virtual page in the host linear mapping?

> +	} else {
> +		gaite->aisbo = 0;
> +		gaite->aisb = 0;
> +	}
>  	aift->kzdev[zdev->aisb] = zdev->kzdev;
>  	spin_unlock_irq(&aift->gait_lock);

[Severity: High]
This is a pre-existing issue, but does repeated invocation of the
KVM_S390_ZPCIOP_REG_AEN ioctl leak AIF allocations?

Looking at kvm_s390_pci_aif_enable(), we allocate and assign new resources
to zdev without checking for or freeing prior state:

    zdev->aisb = bit; /* store the summary bit number */
    zdev->aibv = airq_iv_create(...);

Could a guest or malicious userspace repeatedly invoke the ioctl to cause
unbounded memory exhaustion of pinned pages and airq_iv structures?

[Severity: High]
This is a pre-existing issue, but do the error paths in
kvm_s390_pci_aif_enable() accidentally return success?

If account_mem() or airq_iv_alloc_bit() fail, the code jumps to cleanup labels:

    if (account_mem(pcount))
        goto unpin2;
    ...
    if (bit == -1UL)
        goto unlock;

Since rc remains initialized to 0, does this erroneously return success to
userspace when enabling actually failed?

[Severity: High]
This is a pre-existing issue, but do the early error paths in
kvm_s390_pci_aif_enable() leak resources?

If pin_user_pages_fast(), account_mem(), or airq_iv_alloc_bit() fail, we
jump to the cleanup labels:

    unlock:
        mutex_unlock(&aift->aift_lock);
    unpin2:
        if (fib->fmt0.sum == 1)
            unpin_user_page(aisb_page);
    unpin1:
        unpin_user_page(aibv_page);
    out:
        return rc;

Does this miss calling kvm_s390_gisc_unregister() and unaccount_mem(),
leaving GISC registrations and locked_vm quotas permanently leaked?

[Severity: High]
This is a pre-existing issue, but do account_mem() and unaccount_mem() leak
struct user_struct reference counts?

Both functions increment the user refcount via get_uid():

    struct user_struct *user = get_uid(current_user());

However, free_uid() is never called in these functions. Could this lead to a
persistent memory leak and potential refcount overflow?

[Severity: High]
This is a pre-existing issue, but does unaccount_mem() risk quota corruption
by blindly using current_user() and current->mm?

When unaccount_mem() is called:

    struct user_struct *user = get_uid(current_user());
    if (user)
        atomic_long_sub(nr_pages, &user->locked_vm);

If this is invoked asynchronously during VFIO device closure (e.g., via fput
in a system workqueue), the current user will resolve to the worker thread.
Does this mean the original user's quota is never restored, and an unrelated
system worker's quota is incorrectly reduced?

[Severity: Critical]
This is a pre-existing issue, but does an unchecked return value from
airq_iv_create() risk a host kernel panic?

In kvm_s390_pci_aif_enable():

    zdev->aibv = airq_iv_create(msi_vecs, ...);

If airq_iv_create() returns NULL due to memory pressure, this is assigned to
zdev->aibv. Immediately after, kvm_zpci_set_airq() is called, which executes:

    fib.fmt0.noi = airq_iv_end(zdev->aibv);

Does airq_iv_end() unconditionally dereference the potentially NULL pointer,
allowing unprivileged userspace or a guest to trigger a panic?

[Severity: Critical]
This is a pre-existing issue, but is the page pinning for the Adapter
Interrupt Bit Vector (AIBV) insufficient?

In kvm_s390_pci_aif_enable(), only a single page is pinned:

    npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);

The physical address passed to hardware contains the unvalidated offset:

    gaddr = page_to_phys(aibv_page) + (fib->fmt0.aibv & ~PAGE_MASK);

If a guest specifies an AIBV offset close to the end of the page, could the
hardware DMA engine write beyond the pinned page boundary and corrupt
adjacent host memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713083007.65863-6-borntraeger@de.ibm.com?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure
  2026-07-13  8:30 ` [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure Christian Borntraeger
@ 2026-07-13  8:48   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-07-13  8:48 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Vasily Gorbik, kvm, Alexander Gordeev, Heiko Carstens, linux-s390

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

Pre-existing issues:
- [Critical] Lack of guest-provided AIBV/AISB address boundary validation allows arbitrary host memory corruption.
- [High] Missing cleanup on KVM_S390_ZPCIOP_REG_AEN overwrite and failure paths causes resource leaks and silent failures.
- [High] Missing NULL pointer check for airq_iv_create() return value causes a host kernel panic.
- [High] Reference count leak of user_struct and unsafe memory un-accounting context corrupts memory quotas.
--

--- Patch [1]: [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure ---
commit 3845d6e353bdaf75c36ec128cd79d6dddde46a80
Author: Haoxiang Li <haoxiang_li2024@163.com>

KVM: s390: pci: Fix GISC refcount leak on AIF enable failure
    
kvm_s390_gisc_register() registers the guest ISC before pinning
the guest interrupt forwarding pages and allocating the AISB bit.
If any of the later setup steps fails, the function unwinds the
pinned pages and other local state, but does not unregister the
GISC reference. Add the missing kvm_s390_gisc_unregister() to the
error unwind path.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 5b075c38998e3..686113be05309 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -328,6 +328,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>  unpin1:
>  	unpin_user_page(aibv_page);
>  out:
> +	kvm_s390_gisc_unregister(kvm, fib->fmt0.isc);

[Severity: High]
This isn't a bug introduced by this patch, but does this function properly
handle failure paths and repeated calls?

If a guest repeatedly issues the KVM_S390_ZPCIOP_REG_AEN ioctl, is there a
check to prevent blindly overwriting existing allocations?

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

Also, if airq_iv_alloc_bit() fails, it appears we jump to unlock without
setting a negative rc or calling unaccount_mem(pcount):

    bit = airq_iv_alloc_bit(aift->sbv);
    if (bit == -1UL)
        goto unlock;

Additionally, if kvm_zpci_set_airq() fails, it looks like we return the error
without rolling back the pinned pages, adapter event bits, or memory
accounting:

    rc = kvm_zpci_set_airq(zdev);
    return rc;

Could these paths lead to resource leaks or silent failures if the hardware
rejects the registration?

[Severity: High]
This is a pre-existing issue, but is it possible for airq_iv_create() to
return NULL here?

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 the host is under memory pressure and airq_iv_create() fails, we
immediately assign NULL to zdev->aibv. Then in kvm_zpci_set_airq(), it looks
like this is unconditionally dereferenced:

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

Can this lead to a host kernel panic if guest userspace triggers the
registration ioctl?

[Severity: Critical]
This is a pre-existing issue, but are the bounds of the guest-provided
addresses adequately validated here?

arch/s390/kvm/pci.c:kvm_s390_pci_aif_enable() {
    ...
    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;
    ...
}

The code pins exactly 1 page. However, could msi_vecs require more space than
what remains in the page from the provided offset? 

If a guest provides an unaligned physical address near the end of a page, and
the interrupt vectors require more space (up to 256 bytes), might the
hardware's DMA write cross the page boundary into unpinned host physical
memory?

[Severity: High]
This isn't a bug introduced by this patch, but how do the memory accounting
functions handle the user_struct reference count and quotas?

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

It seems we increment the user_struct refcount with get_uid() but never free
it, nor do we store the user pointer for accurate future teardown. 

Then in unaccount_mem():

arch/s390/kvm/pci.c:unaccount_mem() {
    ...
    struct user_struct *user = get_uid(current_user());

    if (user)
        atomic_long_sub(nr_pages, &user->locked_vm);
    ...
}

Does this leak a user_struct reference on each call? Furthermore, if teardown
happens in a different context (such as a kernel thread during hot-unplug),
could it subtract quotas from an unrelated process instead of the process that
originally registered the AEN?

>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713083007.65863-2-borntraeger@de.ibm.com?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [GIT PULL 0/5] KVM: s390: Fixes for 7.2
  2026-07-13  8:30 [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
                   ` (4 preceding siblings ...)
  2026-07-13  8:30 ` [GIT PULL 5/5] KVM: s390: pci: Fix handling of AIF enable without AISB Christian Borntraeger
@ 2026-07-13 11:34 ` Christian Borntraeger
  5 siblings, 0 replies; 9+ messages in thread
From: Christian Borntraeger @ 2026-07-13 11:34 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: KVM, linux-s390, Janosch Frank, David Hildenbrand,
	Claudio Imbrenda, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Matthew Rosato, Farhan Ali, Eric Farman,
	Sven Schnelle

Am 13.07.26 um 10:30 schrieb Christian Borntraeger:
> From: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> Paolo,
> 
> please pull the following changes since commit 8cdeaa50eae8dad34885515f62559ee83e7e8dda:
> 
>    Linux 7.2-rc2 (2026-07-05 14:44:06 -1000)
> 
> from the Git repository at:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-master-7.2-1
> 
> for you to fetch changes up to 3e3aa6da87d30a0064a17b836685cd43c90a3572:
> 
>    KVM: s390: pci: Fix handling of AIF enable without AISB (2026-07-09 16:23:10 +0200)
> 
> ----------------------------------------------------------------
> KVM: s390: Fixes for 7.2
> 
> - more gmap KVM memory management fixes
> - PCI passthru fixes
> 
Paolo, FYI the PCI sashiko findings are independent from these patches and Farhan and Matt are
already working on additional fixes.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-07-13 11:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13  8:30 [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger
2026-07-13  8:30 ` [GIT PULL 1/5] KVM: s390: pci: Fix GISC refcount leak on AIF enable failure Christian Borntraeger
2026-07-13  8:48   ` sashiko-bot
2026-07-13  8:30 ` [GIT PULL 2/5] KVM: s390: vsie: Avoid potential deadlock with real spaces Christian Borntraeger
2026-07-13  8:30 ` [GIT PULL 3/5] KVM: s390: Fix dat_crste_walk_range() early return Christian Borntraeger
2026-07-13  8:30 ` [GIT PULL 4/5] KVM: s390: Improve kvm_s390_vm_stop_migration() Christian Borntraeger
2026-07-13  8:30 ` [GIT PULL 5/5] KVM: s390: pci: Fix handling of AIF enable without AISB Christian Borntraeger
2026-07-13  8:47   ` sashiko-bot
2026-07-13 11:34 ` [GIT PULL 0/5] KVM: s390: Fixes for 7.2 Christian Borntraeger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox