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

[Huacai Chen]

Hi, Bibo,

What do you think about this?


Huacai

On Fri, Aug 14, 2026 at 5:47 PM Tao Cui <cui.tao@xxxxxxxxx> 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.
>
> Reviewed-by: Bibo Mao <maobibo@xxxxxxxxxxx>
> Signed-off-by: Tao Cui <cuitao@xxxxxxxxxx>
> ---
>
> Changes in v2: use a spinlock instead of a cmpxchg loop (Bibo Mao).
> Changes in v3: rename the lock to pv_setting_lock and place it next to
> phyid_map_lock (Huacai Chen).
>
> v1: https://lore.kernel.org/all/20260810081321.157258-1-cui.tao@xxxxxxxxx/
> v2: https://lore.kernel.org/all/20260813232542.2098930-1-cui.tao@xxxxxxxxx/
>
>  arch/loongarch/include/asm/kvm_host.h | 1 +
>  arch/loongarch/kvm/vcpu.c             | 6 +++++-
>  arch/loongarch/kvm/vm.c               | 1 +
>  3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
> index 23cfbecebbd7..3538a199aac6 100644
> --- a/arch/loongarch/include/asm/kvm_host.h
> +++ b/arch/loongarch/include/asm/kvm_host.h
> @@ -125,6 +125,7 @@ struct kvm_arch {
>         unsigned int  pte_shifts[MAX_PGTABLE_LEVELS];
>         unsigned int  root_level;
>         spinlock_t    phyid_map_lock;
> +       spinlock_t    pv_setting_lock;
>         struct kvm_phyid_map  *phyid_map;
>         /* Enabled PV features */
>         unsigned long pv_features;
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 20c207d80e31..e47fa463a8be 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_setting_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_setting_lock);
>                         return -EINVAL;
> +               }
>                 kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
> +               spin_unlock(&kvm->arch.pv_setting_lock);
>                 return 0;
>         default:
>                 return -ENXIO;
> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> index 1317c718f896..86ba7d54e81f 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_setting_lock);
>
>         kvm_init_vmcs(kvm);
>         kvm_vm_init_features(kvm);
> --
> 2.43.0
>