Re: [PATCH v2 3/3] RISC-V: KVM: fix vcpu vector context handling for kernel-mode vector

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The `RISCV_V_VCPU_NEED_RESTORE` thread flag is leaked across KVM_RUN boundaries, leading to uninitialized kernel memory disclosure.
--

--- Patch [3]: [PATCH v2 3/3] RISC-V: KVM: fix vcpu vector context handling for kernel-mode vector ---
commit 48826c7676c07c389b4f37039e65e239445a85c8
Author: Andy Chiu <tchiu@tenstorrent.com>

RISC-V: KVM: fix vcpu vector context handling for kernel-mode vector

This patch registers an RCU-protected callback to flush the vector context
when the kernel-mode vector is serving a guest. This addresses vtype
corruption during vector workloads by correctly saving the guest's context
and sourcing the right status.VS.

> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index cf6e231e76e25..b7b68809a5817 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c

[ ... ]

> @@ -765,6 +769,12 @@ static void noinstr kvm_riscv_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>  	kvm_riscv_vcpu_swap_in_guest_state(vcpu);
>  	guest_state_enter_irqoff();
>  
> +	if (current->thread.riscv_v_flags & RISCV_V_VCPU_NEED_RESTORE) {
> +		current->thread.riscv_v_flags &= ~RISCV_V_VCPU_NEED_RESTORE;
> +		__kvm_riscv_vector_restore(gcntx);
> +		gcntx->sstatus = (gcntx->sstatus & ~SR_VS) | SR_VS_CLEAN;
> +	}
> +

[Severity: Critical]
Could this leak uninitialized kernel memory to the guest if the thread flag
carries over from a previous vCPU run?

If a softirq uses Kernel Mode Vector while a vCPU is loaded, it sets
RISCV_V_VCPU_NEED_RESTORE via kvm_riscv_vcpu_flush_vector(). If the thread
subsequently exits KVM to userspace, and then runs a different vCPU (for
example, one without the vector extension), this code will consume the
leaked flag.

Will this blindly restore the new vCPU's uninitialized gcntx->vector.datap
(which was allocated via kmalloc) into the physical vector registers, and
illegitimately force SR_VS_CLEAN in the guest's sstatus?

[ ... ]

> diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
> index 62d2fb77bb9b9..d25c10bd99a89 100644
> --- a/arch/riscv/kvm/vcpu_vector.c
> +++ b/arch/riscv/kvm/vcpu_vector.c

[ ... ]

> @@ -72,6 +73,7 @@ void kvm_riscv_vcpu_host_vector_restore(struct kvm_cpu_context *cntx)
>  {
>  	if (!kvm_riscv_isa_check_host(V))
>  		__kvm_riscv_vector_restore(cntx);
> +	riscv_v_flags_set(riscv_v_flags() & ~RISCV_V_VCPU_CTX);
>  }

[Severity: Critical]
Should RISCV_V_VCPU_NEED_RESTORE also be cleared here to prevent it from
leaking to the thread's future execution?

Since kvm_arch_vcpu_put() calls kvm_riscv_vcpu_host_vector_restore() during
KVM exit, clearing it here might prevent the cross-vCPU leakage described
above.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715051629.1169645-1-tchiu@tenstorrent.com?part=3