Re: [RFC PATCH v7 17/28] KVM: arm64: Apply a RES0 mask to PMBLIMITR_EL1 writes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] get_user_spe_sysreg() and set_user_spe_sysreg() return -EBUSY before SPE initialization, violating the KVM_GET_REG_LIST contract.
- [High] Userspace can bypass the PMBLIMITR_EL1 RES0 mask by writing to the register before the first KVM_RUN.
- [Low] Missing READ_ONCE() when accessing pmblimitr_el1_res0_mask causes a data race.
--

commit 1270412c2611a30b1f16a0aa9858eb290c7f9768
Author: Alexandru Elisei <alexandru.elisei@arm.com>

KVM: arm64: Apply a RES0 mask to PMBLIMITR_EL1 writes

This commit introduces a RES0 mask for PMBLIMITR_EL1 to hide unsupported
granule sizes from the guest. It computes this mask during the vCPU's
first run and applies it when userspace or the guest writes to the register.

> diff --git a/arch/arm64/kvm/spe.c b/arch/arm64/kvm/spe.c
> index 6a00a44c73edf..8c173a1f48bd4 100644
> --- a/arch/arm64/kvm/spe.c
> +++ b/arch/arm64/kvm/spe.c
[ ... ]
> @@ -60,13 +60,43 @@ bool kvm_supports_spe(void)
[ ... ]
>  int kvm_spe_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>  {
> +	struct kvm *kvm = vcpu->kvm;
> +	struct kvm_spe *kvm_spe = &kvm->arch.kvm_spe;
>  
> -	if (!vcpu_spe->initialized)
> +	if (!kvm_vcpu_spe_initialized(vcpu))
>  		return -EINVAL;
>  
> +	if (!kvm_spe->pmblimitr_el1_res0_mask)

[Severity: Low]
Does this plain read of pmblimitr_el1_res0_mask need a READ_ONCE() to
prevent compiler-induced load tearing?

The mask is updated locklessly using WRITE_ONCE() inside
kvm_spe_compute_pmblimitr_el1_res0_mask(). Without a READ_ONCE() here,
concurrent vCPU executions could trigger a data race.

> +		kvm_spe_compute_pmblimitr_el1_res0_mask(kvm);
>  
>  	return 0;
>  }
> @@ -85,8 +115,12 @@ void kvm_spe_destroy_vm(struct kvm *kvm)
>  
>  bool kvm_spe_write_sysreg(struct kvm_vcpu *vcpu, int reg, u64 val)
>  {
> +	struct kvm_spe *kvm_spe = &vcpu->kvm->arch.kvm_spe;
> +
>  	switch (reg) {
>  	case PMBLIMITR_EL1:
> +		val &= ~kvm_spe->pmblimitr_el1_res0_mask;

[Severity: High]
Can userspace bypass this RES0 sanitization?

If the VMM calls KVM_SET_ONE_REG for PMBLIMITR_EL1 before running the vCPU
for the first time, pmblimitr_el1_res0_mask will still be 0.

Because the mask isn't computed until the first run, the bitwise NOT evaluates
to all 1s (val &= ~0), which leaves the user-provided value unmodified and
potentially exposes the host hardware to unsupported granule configurations.

[Severity: Low]
Does this mask read also need a READ_ONCE() to safely pair with the
WRITE_ONCE() update, similar to the check in kvm_spe_vcpu_first_run_init()?

> +		fallthrough;
>  	case PMBSR_EL1:
>  	case PMBPTR_EL1:
>  		__vcpu_assign_sys_reg(vcpu, reg, val);

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 89b01b6ac7830..4c4bf88c9aa53 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
[ ... ]
> @@ -1563,6 +1563,38 @@ static unsigned int spe_visibility(const struct kvm_vcpu *vcpu,
>  	return REG_HIDDEN;
>  }
>  
> +static int set_user_spe_sysreg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
> +			       u64 val)
> +{
> +	int reg = r->reg;
> +
> +	if (!kvm_vcpu_spe_initialized(vcpu))
> +		return -EBUSY;

[Severity: Critical]
Does returning -EBUSY here break the KVM_GET_REG_LIST ABI contract?

spe_visibility() currently exposes SPE registers based purely on
vcpu_has_spe(). Because of this, KVM_GET_REG_LIST will advertise the SPE
registers immediately after a vCPU is created with the SPE feature.

If a VMM queries the register list and then calls KVM_GET_ONE_REG or
KVM_SET_ONE_REG before KVM_ARM_VCPU_SPE_INIT is called, it will unexpectedly
hit this -EBUSY path and fail. The KVM API requires that any register
returned by KVM_GET_REG_LIST must be successfully readable via
KVM_GET_ONE_REG.

> +
> +	if (reg == PMBLIMITR_EL1 || reg == PMBPTR_EL1 || reg == PMBSR_EL1)
> +		kvm_spe_write_sysreg(vcpu, reg, val);
> +	else
> +		__vcpu_assign_sys_reg(vcpu, reg, val);
> +
> +	return 0;
> +}
> +
> +static int get_user_spe_sysreg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
> +			       u64 *val)
> +{
> +	int reg = r->reg;
> +
> +	if (!kvm_vcpu_spe_initialized(vcpu))
> +		return -EBUSY;
> +
> +	if (reg == PMBLIMITR_EL1 || reg == PMBPTR_EL1 || reg == PMBSR_EL1)
> +		*val = kvm_spe_read_sysreg(vcpu, reg);
> +	else
> +		*val = __vcpu_sys_reg(vcpu, reg);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903160623.315525-1-alexandru.elisei@arm.com?part=17