Re: [PATCH v3 02/10] KVM: s390: Fix leaking of PGM_ADDRESSING to userspace

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `kvm_s390_set_skeys` ioctl handler contains an O(N^2) retry loop that executes with preemption disabled, which can trigger a hard lockup or RCU stall.
--

--- Patch [2]: [PATCH v3 02/10] KVM: s390: Fix leaking of PGM_ADDRESSING to userspace ---
commit 175d33190557729485ae32f46f8a61bb29680d52
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix leaking of PGM_ADDRESSING to userspace

This commit fixes an issue where kvm_s390_set_cmma_bits() and several
storage key operations return positive error values (like PGM_ADDRESSING)
to userspace instead of standard negative error codes. It updates the
return logic to convert these positive values to -EFAULT.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index eff0644b8517d..8b7e7da354a25 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c

[ ... ]

> @@ -2274,7 +2274,7 @@ static int kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)

[Severity: High]
This isn't a bug introduced by this patch, but looking at the loop just
above this return statement in kvm_s390_set_skeys():

	do {
		r = kvm_s390_mmu_cache_topup(mc);
		if (r == -ENOMEM)
			break;
		scoped_guard(read_lock, &kvm->mmu_lock) {
			for (i = 0 ; i < args->count; i++) {
				r = dat_set_storage_key(mc, kvm->arch.gmap->asce,
							args->start_gfn + i, keys[i], 0);
				if (r)
					break;
			}
		}
	} while (r == -ENOMEM);

Can this loop trigger a hard lockup or RCU stall?

When dat_set_storage_key() depletes the MMU cache and returns -ENOMEM, the
inner loop breaks. The outer loop will then refill the cache and restart
the inner loop completely from i = 0.

For large unmapped memory ranges, does this result in O(N^2) behavior where
the kernel re-traverses previously mapped pages over and over?

Since this entire process happens under scoped_guard(read_lock, &kvm->mmu_lock)
which disables preemption, an unprivileged guest calling KVM_S390_SET_SKEYS
with a large count could potentially cause a local denial of service.

>  	kvm_s390_free_mmu_cache(mc);
>  out:
>  	kvfree(keys);
> -	return r;
> +	return r <= 0 ? r : -EFAULT;
>  }

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