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

Tao Cui posted 1 patch 1 month, 2 weeks ago
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(-)
[PATCH v3] LoongArch: KVM: Fix TOCTOU race on pv_features
Posted by Tao Cui 1 month, 2 weeks ago
From: Tao Cui <cuitao@kylinos.cn>

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@loongson.cn>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---

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@linux.dev/
v2: https://lore.kernel.org/all/20260813232542.2098930-1-cui.tao@linux.dev/

 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
Re: [PATCH v3] LoongArch: KVM: Fix TOCTOU race on pv_features
Posted by Huacai Chen 3 weeks, 2 days ago
Applied, thanks.

Huacai

On Fri, Aug 14, 2026 at 5:47 PM Tao Cui <cui.tao@linux.dev> wrote:
>
> From: Tao Cui <cuitao@kylinos.cn>
>
> 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@loongson.cn>
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>
> 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@linux.dev/
> v2: https://lore.kernel.org/all/20260813232542.2098930-1-cui.tao@linux.dev/
>
>  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
>
Re: [PATCH v3] LoongArch: KVM: Fix TOCTOU race on pv_features
Posted by Huacai Chen 4 weeks ago
Hi, Bibo,

What do you think about this?


Huacai

On Fri, Aug 14, 2026 at 5:47 PM Tao Cui <cui.tao@linux.dev> wrote:
>
> From: Tao Cui <cuitao@kylinos.cn>
>
> 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@loongson.cn>
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>
> 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@linux.dev/
> v2: https://lore.kernel.org/all/20260813232542.2098930-1-cui.tao@linux.dev/
>
>  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
>
Re: [PATCH v3] LoongArch: KVM: Fix TOCTOU race on pv_features
Posted by Bibo Mao 3 weeks, 6 days ago

On 2026/8/31 下午10:07, Huacai Chen wrote:
> Hi, Bibo,
> 
> What do you think about this?
Thanks for your reminder, I forget this in flooding mailing list :(
It looks good to me.

Reviewed-by: Bibo Mao <maobibo@loongson.cn>
> 
> 
> Huacai
> 
> On Fri, Aug 14, 2026 at 5:47 PM Tao Cui <cui.tao@linux.dev> wrote:
>>
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> 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@loongson.cn>
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>> ---
>>
>> 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@linux.dev/
>> v2: https://lore.kernel.org/all/20260813232542.2098930-1-cui.tao@linux.dev/
>>
>>   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
>>