linux-kernel - Re: [PATCH v2 3/4] KVM: Implement barriers before accessing kvm->buses[] on SRCU read paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <yr5vh2yn25fxa7xeja6gxix6nlqvx7jdwfuurwvf26523vnbiz@5ppp455mjuuc>
Date: Sat, 19 Jul 2025 10:37:12 +0800
From: Yao Yuan <yaoyuan0329os@...il.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Yao Yuan <yaoyuan@...ux.alibaba.com>, Keir Fraser <keirf@...gle.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org, 
	Eric Auger <eric.auger@...hat.com>, Oliver Upton <oliver.upton@...ux.dev>, 
	Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>, 
	Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v2 3/4] KVM: Implement barriers before accessing
 kvm->buses[] on SRCU read paths

On Fri, Jul 18, 2025 at 07:54:38AM -0700, Sean Christopherson wrote:
> On Thu, Jul 17, 2025, Yao Yuan wrote:
> > On Wed, Jul 16, 2025 at 11:07:36AM +0800, Keir Fraser wrote:
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index 3bde4fb5c6aa..9132148fb467 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -965,11 +965,15 @@ static inline bool kvm_dirty_log_manual_protect_and_init_set(struct kvm *kvm)
> > >  	return !!(kvm->manual_dirty_log_protect & KVM_DIRTY_LOG_INITIALLY_SET);
> > >  }
> > >
> > > +/*
> > > + * Get a bus reference under the update-side lock. No long-term SRCU reader
> > > + * references are permitted, to avoid stale reads vs concurrent IO
> > > + * registrations.
> > > + */
> > >  static inline struct kvm_io_bus *kvm_get_bus(struct kvm *kvm, enum kvm_bus idx)
> > >  {
> > > -	return srcu_dereference_check(kvm->buses[idx], &kvm->srcu,
> > > -				      lockdep_is_held(&kvm->slots_lock) ||
> > > -				      !refcount_read(&kvm->users_count));
> > > +	return rcu_dereference_protected(kvm->buses[idx],
> > > +					 lockdep_is_held(&kvm->slots_lock));
> >
> > I want to consult the true reason for using protected version here,
> > save unnecessary READ_ONCE() ?
>
> Avoiding the READ_ONCE() is a happy bonus.  The main goal is to help document
> and enforce that kvm_get_bus() can only be used if slots_lock is held.  Keeping
> this as srcu_dereference_check() would result in PROVE_RCU getting a false negative
> if the caller held kvm->srcu but not slots_lock.

Ah, I noticed the srcu_read_lock_held(ssp) in srcu_dereference_check() this time !

>
> From a documentation perspective, rcu_dereference_protected() (hopefully) helps
> highlight that there's something "special" about this helper, e.g. gives the reader
> a hint that they probably shouldn't be using kvm_get_bus().

Yes, I got it. Thanks for your nice explanation!

>