Re: kvm_read_guest_page() missing kvm->srcu read lock?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/05/2018 19:41, Andre Przywara wrote:
> Hi,
> 
> Jan posted an lockdep splat complaining about a suspicious
> rcu_dereference_check:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2018-May/031116.html
> 
> The gist of that is:
> ...
> [ 1025.695517]  dump_stack+0x9c/0xd4
> [ 1025.695524]  lockdep_rcu_suspicious+0xcc/0x118
> [ 1025.695537]  gfn_to_memslot+0x174/0x190
> [ 1025.695546]  kvm_read_guest+0x50/0xb0
> [ 1025.695553]  vgic_its_check_id.isra.0+0x114/0x148
> ...
> I chased that down and wonder if kvm_read_guest{,_page} is supposed to
> be called inside a kvm->srcu critical section?
> 
> We have a check that suggests that eventually someone needs to enter the
> SRCU criticial section:
> static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm,
> 						  int as_id)
> {
>         return srcu_dereference_check(kvm->memslots[as_id], &kvm->srcu,
>                         lockdep_is_held(&kvm->slots_lock) ||
>                         !refcount_read(&kvm->users_count));
> }
> 
> If I get this correctly this mean for accessing kvm->memslots we either
> need to be inside an srcu critical section or hold the kvm->slots_lock
> (for updates only).
> 
> If I am not mistaken, it is not necessary for *callers* of
> kvm_read_guest_page() to do this, as this could be entirely contained
> inside this function - since we only use the reference to the memslot
> entry while doing the copy_from_user(), and the data is safe afterwards
> from an RCU point of view because it has been *copied*.

Yes, it's the caller's responsibility.  srcu_read_lock/unlock is pretty
expensive so KVM assumes that the topmost callers do it.

Paolo



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux