Re: [PATCH v2] LoongArch: KVM: Fix TOCTOU race on pv_features

[Huacai Chen]

Hi, Tao,

On Fri, Aug 14, 2026 at 9:09 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
>
>
>
> On 2026/8/14 上午7:25, Tao Cui wrote:
> > From: Tao Cui <cuitao@xxxxxxxxxx>
> >
> > The check-then-set on kvm->arch.pv_features in
> > kvm_loongarch_cpucfg_set_attr() is lockless, so two vCPUs can race
> > past the validation and set different values.  Add a spinlock to
> > serialize it.
> >
> > Signed-off-by: Tao Cui <cuitao@xxxxxxxxxx>
> > ---
> >
> > Changes in v2: use a spinlock instead of a cmpxchg loop (Bibo Mao).
> > Link: https://lore.kernel.org/all/20260810081321.157258-1-cui.tao@xxxxxxxxx/
> >
> >   arch/loongarch/include/asm/kvm_host.h | 2 ++
> >   arch/loongarch/kvm/vcpu.c             | 6 +++++-
> >   arch/loongarch/kvm/vm.c               | 1 +
> >   3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
> > index 23cfbecebbd7..cdbbd90e0584 100644
> > --- a/arch/loongarch/include/asm/kvm_host.h
> > +++ b/arch/loongarch/include/asm/kvm_host.h
> > @@ -128,6 +128,8 @@ struct kvm_arch {
> >       struct kvm_phyid_map  *phyid_map;
> >       /* Enabled PV features */
> >       unsigned long pv_features;
> > +     /* Serializes pv_features updates */
> > +     spinlock_t pv_features_lock;
I think it is better to rename it to pv_setting_lock, and put it near
phyid_map_lock.

Huacai

> >       /* Supported KVM features */
> >       unsigned long kvm_features;
> >
> > diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> > index 20c207d80e31..551ed39b2d1f 100644
> > --- a/arch/loongarch/kvm/vcpu.c
> > +++ b/arch/loongarch/kvm/vcpu.c
> > @@ -1165,10 +1165,14 @@ static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu,
> >                       return -EINVAL;
> >
> >               /* All vCPUs need set the same PV features */
> > +             spin_lock(&kvm->arch.pv_features_lock);
> >               if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED)
> > -                             && ((kvm->arch.pv_features & valid) != val))
> > +                             && ((kvm->arch.pv_features & valid) != val)) {
> > +                     spin_unlock(&kvm->arch.pv_features_lock);
> >                       return -EINVAL;
> > +             }
> >               kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
> > +             spin_unlock(&kvm->arch.pv_features_lock);
> >               return 0;
> >       default:
> >               return -ENXIO;
> > diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> > index 1317c718f896..b984cc54f305 100644
> > --- a/arch/loongarch/kvm/vm.c
> > +++ b/arch/loongarch/kvm/vm.c
> > @@ -76,6 +76,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> >               return -ENOMEM;
> >       }
> >       spin_lock_init(&kvm->arch.phyid_map_lock);
> > +     spin_lock_init(&kvm->arch.pv_features_lock);
> >
> >       kvm_init_vmcs(kvm);
> >       kvm_vm_init_features(kvm);
> >
> Reviewed-by: Bibo Mao <maobibo@xxxxxxxxxxx>
>