[PATCH] KVM: arm64: vgic: Hold config_lock while tearing down a CPU interface

[Zenghui Yu]

On 2024/8/19 18:53, Marc Zyngier wrote:
> On Mon, 19 Aug 2024 10:39:50 +0100,
> Zenghui Yu <yuzenghui at huawei.com> wrote:
> >
> > Hi Marc,
> >
> > On 2024/8/8 17:15, Marc Zyngier wrote:
> > > Tearing down a vcpu CPU interface involves freeing the private interrupt
> > > array. If we don't hold the lock, we may race against another thread
> > > trying to configure it. Yeah, fuzzers do wonderful things...
> > >
> > > Taking the lock early solves this particular problem.
> > >
> > > Fixes: 03b3d00a70b5 ("KVM: arm64: vgic: Allocate private interrupts on demand")
> > > Reported-by: Alexander Potapenko <glider at google.com>
> > > Tested-by: Alexander Potapenko <glider at google.com>
> > > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > > ---
> > >  arch/arm64/kvm/vgic/vgic-init.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> > > index 7f68cf58b978..41feb858ff9a 100644
> > > --- a/arch/arm64/kvm/vgic/vgic-init.c
> > > +++ b/arch/arm64/kvm/vgic/vgic-init.c
> > > @@ -438,14 +438,13 @@ void kvm_vgic_destroy(struct kvm *kvm)
> > >  	unsigned long i;
> > >  
> > >  	mutex_lock(&kvm->slots_lock);
> > > +	mutex_lock(&kvm->arch.config_lock);
> > >  
> > >  	vgic_debug_destroy(kvm);
> > >  
> > >  	kvm_for_each_vcpu(i, vcpu, kvm)
> > >  		__kvm_vgic_vcpu_destroy(vcpu);
> > >  
> > > -	mutex_lock(&kvm->arch.config_lock);
> > > -
> > >  	kvm_vgic_dist_destroy(kvm);
> > >  
> > >  	mutex_unlock(&kvm->arch.config_lock);
> >
> > The following splat was triggered when running the vgic_init selftest on
> > a lockdep kernel (I use rc4, with defconfig + PROVE_LOCKING).
> >
> > I'm not entirely sure that the splat is related to this change. Just
> > haven't got time to dig further so post it out early for record.
> 
> Arghh. Thanks for reporting this. Can you try the following patch? It
> does the trick for me, but I don't trust myself anymore...

I tested it with defconfig+PROVE_LOCKING and my own config, both worked
for me (the WARNING disappeared). I think the locking order should be
correct now.

Thanks,
Zenghui