Re: [PATCH] LoongArch: KVM: Allow to set pv_feature until vCPU run

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Per-vCPU execution state is used to protect VM-wide configuration, allowing malicious userspace to modify PV features while the guest is running.
- [High] The enforcement mechanism ensuring all vCPUs use identical PV features has been removed, silently allowing cross-vCPU mismatch.
--

--- Patch [1]: [PATCH] LoongArch: KVM: Allow to set pv_feature until vCPU run ---
commit bf9fe2ebf34418b4704a3fcafd58fb42df1697c5
Author: Bibo Mao <maobibo@loongson.cn>

LoongArch: KVM: Allow to set pv_feature until vCPU run

This patch modifies KVM on LoongArch to allow setting the pv_feature
multiple times during VM creation. The restriction is relaxed so that
features can be modified until the vCPU starts running, which aims to help
with VM migration.

> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 20c207d80e31a..ce6a1b06d50db 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c

[ ... ]

> @@ -1164,11 +1164,14 @@ static int kvm_loongarch_cpucfg_set_attr(struct kvm_vcpu *vcpu,
>  		if (val & ~valid)
>  			return -EINVAL;
>  
> -		/* All vCPUs need set the same PV features */
> -		if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED)
> -				&& ((kvm->arch.pv_features & valid) != val))
> +		if ((kvm->arch.pv_features & valid) == val)
> +			return 0;
> +
> +		if (vcpu->arch.ran_atleast_once)
>  			return -EINVAL;

[Severity: High]
Can this allow a user space process to modify PV features while the guest is
running?

Because the check in kvm_loongarch_cpucfg_set_attr() uses per-vCPU state
(vcpu->arch.ran_atleast_once) to gate the VM-wide configuration, what
happens if userspace runs vCPU 0 (setting its ran_atleast_once flag to
true), and then invokes KVM_SET_DEVICE_ATTR on vCPU 1 (whose
ran_atleast_once is still false)?

Wouldn't this bypass the check and mutate the VM-wide kvm->arch.pv_features
while vCPU 0 is actively executing?

> -		kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
> +
> +		/* All vCPUs need set the same PV features */
> +		kvm->arch.pv_features = val;

[Severity: High]
Does this silently mask misconfigurations if userspace sets different
features on different vCPUs?

Without the LOONGARCH_PV_FEAT_UPDATED latch, if userspace configures
different PV features on different unrun vCPUs, it appears KVM will accept
both operations and simply overwrite kvm->arch.pv_features with the latest
value rather than returning an error.

Could this fail to enforce the architectural invariant mentioned in the
comment that all vCPUs must have the same features?

>  		return 0;
>  	default:
>  		return -ENXIO;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716013823.3259816-1-maobibo@loongson.cn?part=1