Re: [PATCH v5 7/8] RISC-V: KVM: Add support for control-flow integrity FWFT features

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Evaluating `this_cpu_ptr()` in a preemptible context via the `ncsr_write()` macro causes potential nested hypervisor state corruption.
--

commit 3bf03d5d2c60c30185d31cbc3e653f9c3b28377b
Author: Inochi Amaoto <inochiama@gmail.com>

RISC-V: KVM: Add support for control-flow integrity FWFT features

This commit adds support for exposing Control-Flow Integrity (CFI) FWFT
features (Zicfilp/Zicfiss) to VS-mode. It allows the VMM to control access
to these features by modifying the guest's henvcfg and hedeleg registers.

> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> index 01db40b532951..bc514ae6521d7 100644
> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c

[ ... ]

> +static long kvm_sbi_fwft_set_cfi(struct kvm_vcpu *vcpu,
> +				 struct kvm_sbi_fwft_config *conf,
> +				 bool one_reg_access, unsigned long value,
> +				 u64 flag)
> +{
> +	struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
> +
> +	if (value == 0)
> +		cfg->henvcfg &= ~flag;
> +	else if (value == 1)
> +		cfg->henvcfg |= flag;
> +	else
> +		return SBI_ERR_INVALID_PARAM;
> +
> +	if (cfg->henvcfg & (ENVCFG_LPE | ENVCFG_SSE))
> +		cfg->hedeleg |= BIT(EXC_SOFTWARE_CHECK);
> +	else
> +		cfg->hedeleg &= ~BIT(EXC_SOFTWARE_CHECK);
> +
> +	if (!one_reg_access) {
> +		ncsr_write(CSR_HEDELEG, cfg->hedeleg);

[Severity: High]
Is it safe to call ncsr_write() here with preemption enabled?

When a guest makes an SBI_EXT_FWFT_SET call, the KVM exit handler
(kvm_riscv_vcpu_exit) explicitly enables preemption before processing
the exit.

Looking at the ncsr_write() macro, it uses nacl_shmem() which evaluates
this_cpu_ptr():

#define ncsr_write(__csr, __val)
    ...
    nacl_csr_write(nacl_shmem(), __csr, __val);

If the thread is preempted and migrated to another physical CPU between
evaluating the CPU pointer and performing the write, could this write to
the new CPU's view of the old CPU's memory?

This seems like it might corrupt the nested hypervisor state (like HEDELEG or
HENVCFG) of another running vCPU, as well as trigger CONFIG_DEBUG_PREEMPT
warnings.

> +		/*
> +		 * Both Bit LPE and SSE are in the lower part, so it is safe
> +		 * to only write the henvcfg
> +		 */
> +		ncsr_write(CSR_HENVCFG, vcpu->arch.cfg.henvcfg);
> +	}
> +
> +	return SBI_SUCCESS;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706022046.214956-1-inochiama@gmail.com?part=7