Re: [PATCH v4 03/13] KVM: s390: Fix race in __do_essa()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful code, documentation, and commit message mismatch regarding which lock is used.
--

--- Patch [3]: [PATCH v4 03/13] KVM: s390: Fix race in __do_essa() ---
commit 44561620d155e5433777595f8bc3b2d06087934d
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix race in __do_essa()

This patch fixes a potential race condition between __do_essa() and the 
migration and CMMA state handling functions. It transitions the affected paths 
to use the architecture-specific slots lock via scope-based guards.

> KVM: s390: Fix race in __do_essa()
>
> An unlikely race between __do_essa() and kvm_s390_vm_start_migration(),
> kvm_s390_vm_stop_migration(), or dat_get_cmma() was possible.
>
> Fix by locking kvm->slots_lock. Since this is not a hot path, the
> overhead of an additional mutex is negligible.

[Severity: Medium]
This isn't a bug, but the commit message states the fix uses kvm->slots_lock,
while the code changes actually use kvm->slots_arch_lock. Should the commit
message be updated to reflect the use of the architecture-specific lock?

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 89de085f39670..1952f592a80a1 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1298,7 +1298,9 @@ static int kvm_s390_vm_set_migration(struct kvm *kvm,
>  {
>  	int res = -ENXIO;
>  
> -	mutex_lock(&kvm->slots_lock);
> +	guard(srcu)(&kvm->srcu);
> +	guard(mutex)(&kvm->slots_arch_lock);

[Severity: Medium]
This isn't a bug, but the function documentation for kvm_s390_vm_start_migration()
and kvm_s390_vm_stop_migration() (which are called from here) still states they
must be called with kvm->slots_lock held:

arch/s390/kvm/kvm-s390.c:kvm_s390_vm_start_migration():
/*
 * Must be called with kvm->srcu held to avoid races on memslots, and with
 * kvm->slots_lock to avoid races with ourselves, kvm_s390_vm_stop_migration(),
 * and kvm_s390_get_cmma_bits().
 */
static int kvm_s390_vm_start_migration(struct kvm *kvm)

Since these paths now use kvm->slots_arch_lock instead, would it be helpful
to update those comments to match the new locking behavior?

[ ... ]

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