Re: [PATCH v2 5/7] RISC-V: KVM: Handle software-check exits for VCPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] KVM fails to context-switch the Shadow Stack Pointer (CSR_SSP), leading to cross-VCPU leaks and guest shadow stack corruption.
- [High] KVM fails to propagate the SPELP (Supervisor Previous Expected Landing Pad) bit when redirecting traps to the guest, breaking guest CFI tracking.
- [Medium] KVM unconditionally redirects EXC_SOFTWARE_CHECK to the guest upon VM exit, which will inject an un-enumerated exception into guests that have not negotiated the Zicfilp/Zicfiss extensions.
--

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

RISC-V: KVM: Handle software-check exits for VCPU

This commit enables the software-check exception for RISC-V KVM, delegating
it to VS mode so CFI violations (Zicfiss/Zicfilp) can be reported to the
guest.

> diff --git a/arch/riscv/kvm/vcpu_config.c b/arch/riscv/kvm/vcpu_config.c
> index b2a1028106d5d..dbe4167ac4f16 100644
> --- a/arch/riscv/kvm/vcpu_config.c
> +++ b/arch/riscv/kvm/vcpu_config.c

[ ... ]

> @@ -65,6 +65,10 @@ void kvm_riscv_vcpu_config_ran_once(struct kvm_vcpu *vcpu)
>  
>  	if (vcpu->guest_debug)
>  		cfg->hedeleg &= ~BIT(EXC_BREAKPOINT);
> +
> +	if (riscv_isa_extension_available(isa, ZICFILP) ||
> +	    riscv_isa_extension_available(isa, ZICFISS))
> +		cfg->hedeleg |= BIT(EXC_SOFTWARE_CHECK);

[Severity: High]
The Zicfiss extension introduces the CSR_SSP (Shadow Stack Pointer) register,
which is a user-mode CSR and isn't automatically saved or restored by the
hardware on VM exits.

If KVM does not manually context-switch this register in 
arch/riscv/kvm/vcpu.c:kvm_riscv_vcpu_context_switch() or during VCPU load/put
paths, could a new VCPU inherit the previous VCPU's CSR_SSP and leak state
across VMs?

Additionally, when returning to host userspace, won't the host kernel's
exception return code overwrite CSR_SSP with the host process's shadow stack
pointer, leading to an immediate crash upon re-entry to the guest?

>  }

> diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
> index 0bb0c51e3c890..5ab8e87ed248b 100644
> --- a/arch/riscv/kvm/vcpu_exit.c
> +++ b/arch/riscv/kvm/vcpu_exit.c

[ ... ]

> @@ -243,6 +243,9 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		run->exit_reason = KVM_EXIT_DEBUG;
>  		ret = 0;
>  		break;
> +	case EXC_SOFTWARE_CHECK:
> +		ret = vcpu_redirect(vcpu, trap);

[Severity: Medium]
This unconditionally redirects EXC_SOFTWARE_CHECK back to the guest.

If a guest has not negotiated the Zicfilp/Zicfiss extensions, KVM won't
delegate EXC_SOFTWARE_CHECK via hedeleg.

If an unexpected CPU state or hardware issue triggers this exception in
VS-mode and traps to HS-mode, this redirect will inject an un-enumerated
exception into the guest.

Does this code need to verify riscv_isa_extension_available() before
redirecting the trap?

[Severity: High]
When a trap occurs in VS/VU-mode and is taken to HS-mode, the hardware
records the previous Zicfilp ELP state in the host sstatus.SPELP bit, which
KVM subsequently saves into vcpu->arch.guest_context.sstatus.

When vcpu_redirect() is called here, it eventually invokes
kvm_riscv_vcpu_trap_redirect(), which updates the guest's VSSTATUS register:

arch/riscv/kvm/vcpu_exit.c:kvm_riscv_vcpu_trap_redirect() {
    ...
    unsigned long vsstatus = ncsr_read(CSR_VSSTATUS);

    vsstatus &= ~SR_SPP;
    if (vcpu->arch.guest_context.sstatus & SR_SPP)
        vsstatus |= SR_SPP;
    
    ncsr_write(CSR_VSSTATUS, vsstatus);
    ...
}

Since kvm_riscv_vcpu_trap_redirect() explicitly copies SPP, SPIE, and SIE
but does not copy the SPELP bit (SR_SPELP), does this cause the guest OS to
lose its Zicfilp CFI tracking state across KVM-injected traps?

> +		break;
>  	default:
>  		break;
>  	}

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