[PATCH v6 0/6] KVM s390x PCI fixes

[PATCH v6 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
---------
v5: https://lore.kernel.org/all/20260723183440.3271-1-alifm@linux.ibm.com/
v5 -> v6
   - Fix the AIBV calculation (patch 6).

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

-- 
2.43.0

[PATCH v6 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 v6 1/6] KVM: s390: pci: Reject adapter interrupt forwarding if already enabled

[sashiko-bot]

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

Pre-existing issues:
- [High] Memory leak of `user_struct` and incorrect memory quota accounting.
--

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

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

The MPCIFC instruction does not 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. This prevents
overwriting and leaking resources when the ioctl is called multiple times for
the same device.

> 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 isn't a bug introduced by this patch, but does the resource accounting
in account_mem() and unaccount_mem() leak the user_struct reference and
mis-account memory quotas during teardown?

In account_mem(), get_uid() increments the user_struct reference count, but
this reference does not appear to be freed via free_uid() on the success path:

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

	return 0;
	...
}

Additionally, does unaccount_mem() use the current task context for teardown
instead of the saved context?

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);
	if (current->mm)
		atomic64_sub(nr_pages, &current->mm->pinned_vm);
}

If teardown happens in a different context, such as a worker thread during
delayed_fput() (e.g., during kvm_s390_pci_aif_disable), could unaccount_mem()
subtract from the wrong task's locked_vm?

Because locked_vm is an atomic_long_t, subtracting unallocated pages could
cause a negative underflow. During subsequent account_mem() checks, this
negative value might be treated as a large unsigned integer, which would cause
new_pages > page_limit to return -ENOMEM, potentially preventing the affected
user from allocating locked memory in the future.

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

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

[PATCH v6 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>
Reviewed-by: Matthew Rosato <mjrosato@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 v6 2/6] KVM: s390: pci: Fix memory accounting for pinned/unpinned pages

[sashiko-bot]

> 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>
> 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/20260723221409.5874-1-alifm@linux.ibm.com?part=2

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

[Matthew Rosato]

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

FYI there is still a double space here, Christian maybe you can just fix when applying?

> +			goto out;
> +		}

[PATCH v6 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 v6 3/6] 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] Concurrent execution of KVM_S390_ZPCI_OP ioctl and PCI device hot-unplug leads to Use-After-Free of zdev.
--

--- Patch [3]: [PATCH v6 3/6] KVM: s390: pci: Fix missing error codes and memory unaccounting ---
commit 1ca75882cbc64513fea8b4f11f208e8914e5eb06
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 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.

> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 44c00e5e32c57..f58d7ec0bc7ec 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c

[ ... ]

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

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
involving the zdev pointer in this file.

When userspace calls the KVM_S390_ZPCI_OP ioctl, kvm_s390_pci_zpci_op()
fetches zdev from get_zdev_from_kvm_by_fh():

arch/s390/kvm/pci.c:get_zdev_from_kvm_by_fh() {
	list_for_each_entry(kzdev, &kvm->arch.kzdev_list, entry) {
		if (kzdev->zdev->fh == fh) {
			zdev = kzdev->zdev;
			break;
		}
	}
	spin_unlock(&kvm->arch.kzdev_list_lock);

	return zdev;
}

The zdev pointer is returned without taking a reference count. If the ioctl
thread is preempted, and the physical PCI device is removed concurrently
(hot-unplug), the PCI subsystem could drop the last reference to zdev and
free it via kfree_rcu().

After the RCU grace period, the ioctl thread would resume and attempt to
lock the freed memory:

arch/s390/kvm/pci.c:kvm_s390_pci_zpci_op() {
	zdev = get_zdev_from_kvm_by_fh(kvm, args->fh);
	if (!zdev)
		return -ENODEV;

	mutex_lock(&zdev->kzdev_lock);
	...
}

Does this sequence allow a use-after-free if the device is removed while
the ioctl is executing?

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

[PATCH v6 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 v6 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/20260723221409.5874-1-alifm@linux.ibm.com?part=4

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

[Farhan Ali]

Currently if kvm_zpci_set_airq() fails, kvm_s390_pci_aif_enable() returns
the error code but doesn't do any resource cleanup thus leaking resources.
Fix this by cleaning up all the resources such as the GAITE, AIBV, AISB and
unpinning any pinned pages. 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 a bit longer, as
we hold the 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 v6 5/6] KVM: s390: pci: Fix resource leak on IRQ registration failure

[sashiko-bot]

> Currently if kvm_zpci_set_airq() fails, kvm_s390_pci_aif_enable() returns
> the error code but doesn't do any resource cleanup thus leaking resources.
> Fix this by cleaning up all the resources such as the GAITE, AIBV, AISB and
> unpinning any pinned pages. 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 a bit longer, as
> we hold the 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/20260723221409.5874-1-alifm@linux.ibm.com?part=5

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

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index ca8ef76bcf6b..4394cefef242 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,14 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
 		return gisc;
 
 	/* Replace AIBV address */
+	size = BITS_TO_LONGS(msi_vecs + fib->fmt0.aibvo) * sizeof(unsigned long);
+	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 +294,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 v6 6/6] KVM: s390: pci: Validate AIBV and AISB before pinning guest pages

[sashiko-bot]

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

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

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

Re: [PATCH v6 0/6] KVM s390x PCI fixes

[Matthew Rosato]

On 7/23/26 6:14 PM, Farhan Ali wrote:
> 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
> 

Thanks for tackling these, Farhan.

Besides reviewing the code I've run a variety of tests on these last few
versions and things look good -- so if you'd like feel free to include:

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

> ChangeLog
> ---------
> v5: https://lore.kernel.org/all/20260723183440.3271-1-alifm@linux.ibm.com/
> v5 -> v6
>    - Fix the AIBV calculation (patch 6).
> 
> 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 | 107 +++++++++++++++++++++++++++++++++++---------
>  arch/s390/kvm/pci.h |   2 +
>  2 files changed, 88 insertions(+), 21 deletions(-)
> 

Re: [PATCH v6 0/6] KVM s390x PCI fixes

[Christian Borntraeger]

Am 24.07.26 um 00:14 schrieb 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
> ---------
> v5: https://lore.kernel.org/all/20260723183440.3271-1-alifm@linux.ibm.com/
> v5 -> v6
>     - Fix the AIBV calculation (patch 6).
> 
> 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 | 107 +++++++++++++++++++++++++++++++++++---------
>   arch/s390/kvm/pci.h |   2 +
>   2 files changed, 88 insertions(+), 21 deletions(-)
> 

thanks, all applied and queued for kvm/master.
https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git/log/