Re: [PATCH v6 06/10] KVM: s390: Fix locking in kvm_s390_set_mem_control()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lost early-exit return codes in `dat_crste_walk_range` leads to soft lockups and aging logic failures.
--

commit b7c3f62b3078686b2bddd109edeee90c43ecceb5
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix locking in kvm_s390_set_mem_control()

Add the missing locking around dat_reset_cmma().

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ffb20a64d328f..fe0ae15e5ad07 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -990,9 +990,11 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
>  		if (!kvm->arch.use_cmma)
>  			break;
>  
> +		guard(mutex)(&kvm->lock);
>  		VM_EVENT(kvm, 3, "%s", "RESET: CMMA states");
>  		do {
> -			start_gfn = dat_reset_cmma(kvm->arch.gmap->asce, start_gfn);
> +			scoped_guard(read_lock, &kvm->mmu_lock)
> +				start_gfn = dat_reset_cmma(kvm->arch.gmap->asce, start_gfn);

[Severity: High]
This is a pre-existing issue, but looking at how dat_reset_cmma() and other
DAT walk operations use _dat_walk_gfn_range() under the hood, is there a bug
in arch/s390/kvm/dat.c:dat_crste_walk_range() where we can silently lose
early-exit return codes?

When dat_crste_walk_range() performs lower-level walks, it assigns the result
to 'rc', but lacks an immediate check to break the loop:

arch/s390/kvm/dat.c:dat_crste_walk_range() {
	...
		if (!crste.h.i && !crste.h.fc) {
			if (!is_pmd(crste))
				rc = dat_crste_walk_range(max(start, cur), min(end, next),
							  _dereference_crste(crste), walk);
			else if (walk->ops->pte_entry)
				rc = dat_pte_walk_range(max(start, cur), min(end, next),
							dereference_pmd(crste.pmd), walk);
		}
	}
	return rc;
}

If a walk operation defines both crste_ops and pte_entry (such as
dat_reset_skeys or test_age_ops) and dat_pte_walk_range() returns a non-zero
code (like a gfn for a reschedule request or 1 for finding a young page),
the loop continues to the next iteration.

In the next iteration, if the upper-level crste_op evaluates to 0, could it
silently overwrite 'rc' and allow the walk to incorrectly continue?

Can this cause soft lockups on the host for guests with large memory due to
lost reschedule requests, or incorrect page aging data causing performance
degradation?

>  			cond_resched();
>  		} while (start_gfn);
>  		ret = 0;

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