Re: [PATCH RFC 02/10] KVM: Allow reading memslots while holding slots_arch_lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The lockdep condition incorrectly allows reading memslots with only slots_arch_lock, bypassing SRCU protection and exposing a data race on slots->generation.
--

commit b005b5fdc7c30e96c15ac0330a0485909dc26ee9
Author: Marco Elver <elver@google.com>

KVM: Allow reading memslots while holding slots_arch_lock

The commit adds a lockdep exception to allow reading memslots with only
slots_arch_lock held. It states that holding either slots_lock or
slots_arch_lock guarantees memslots cannot be modified concurrently.

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 03bfc92864b6e..5ed8260ef01f3 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1089,6 +1089,7 @@ static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id)
>  	as_id = array_index_nospec(as_id, KVM_MAX_NR_ADDRESS_SPACES);
>  	return srcu_dereference_check(kvm->memslots[as_id], &kvm->srcu,
>  			lockdep_is_held(&kvm->slots_lock) ||
> +			lockdep_is_held(&kvm->slots_arch_lock) ||
                        ^^^^^^^^
[Severity: Medium]
Does holding slots_arch_lock safely guarantee the memslots cannot be
modified concurrently?

The commit message states "Holding either lock guarantees that memslots
cannot be concurrently modified," however in kvm_swap_active_memslots() in
virt/kvm/kvm_main.c, slots_arch_lock is explicitly released before
slots->generation is mutated:

virt/kvm/kvm_main.c:kvm_swap_active_memslots() {
    ...
	rcu_assign_pointer(kvm->memslots[as_id], slots);
	spin_unlock(&kvm->mn_invalidate_lock);

	/*
	 * Acquired in kvm_set_memslot. Must be released before synchronize
	 * SRCU below in order to avoid deadlock with another thread
	 * acquiring the slots_arch_lock in an srcu critical section.
	 */
	mutex_unlock(&kvm->slots_arch_lock);

	synchronize_srcu_expedited(&kvm->srcu);
    ...
	slots->generation = gen;
}

If a reader uses this new lockdep exception in __kvm_memslots() to read the
memslots structure (and specifically slots->generation) relying solely on
slots_arch_lock, won't that reader race with this plain write, bypassing
SRCU protection entirely?

>  			!refcount_read(&kvm->users_count));
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910162343.4092060-1-elver@google.com?part=2