Re: [PATCH v2 04/17] KVM: arm64: Skip fixed-feature state flush for protected vCPUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] State Leak and Host Integrity Bypass via Unswitched Debug Registers for Protected vCPUs
--

commit d142f7027c7a95ffce6ea110097ca91e27e8ccea
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Skip fixed-feature state flush for protected vCPUs

This commit modifies flush_hyp_vcpu() and related debug state flushing
functions to skip copying fixed-feature states, such as mdcr_el2, TWI/TWE,
and debug state, for protected vCPUs. The host's debug registers are
considered hypervisor-owned for protected guests, and the debug state
flush now returns early in this case.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index ca7122b0bccdd..aeff6ded40fd6 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -200,6 +200,9 @@ static void flush_debug_state(struct pkvm_hyp_vcpu *hyp_vcpu)
>  {
>  	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
>  
> +	if (pkvm_hyp_vcpu_is_protected(hyp_vcpu))
> +		return;
> +

[Severity: High]
Does this early return in flush_debug_state() in
arch/arm64/kvm/hyp/nvhe/hyp-main.c bypass the debug register context switch
for protected guests?

By returning early here, hyp_vcpu->vcpu.arch.debug_owner is never updated
from the host and remains at its default VCPU_DEBUG_FREE. During the world
switch, __debug_switch_to_guest_common() and
__debug_switch_to_host_common() return early if debug_owner == FREE.
Consequently, KVM bypasses saving the host's live hardware debug registers and
loading the guest's debug registers. The guest enters execution with the host's
actual hardware breakpoint registers still active in the pipeline.

Normally, this is mitigated because KVM sets MDCR_EL2.TDA to trap debug
accesses if the guest doesn't own the registers. However, for protected VMs,
pvm_init_traps_mdcr() only sets TDA if DebugVer is unimplemented. With
TDA unset, the protected guest can freely read the host's debug registers
(leaking host state) and overwrite them when it explicitly reads or writes
DBGBVRn_EL1, DBGBCRn_EL1, or MDSCR_EL1.

>  	hyp_vcpu->vcpu.arch.debug_owner = host_vcpu->arch.debug_owner;
>  
>  	if (kvm_guest_owns_debug_regs(&hyp_vcpu->vcpu)) {

[ ... ]

> @@ -218,6 +221,9 @@ static void sync_debug_state(struct pkvm_hyp_vcpu *hyp_vcpu)
>  {
>  	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
>  
> +	if (pkvm_hyp_vcpu_is_protected(hyp_vcpu))
> +		return;
> +

[Severity: High]
Does this early return in sync_debug_state() in
arch/arm64/kvm/hyp/nvhe/hyp-main.c skip restoring the host's debug registers
upon guest exit?

Because the context switch back to the host returns early due to the bypassed
flush, any guest modifications to the hardware debug registers persist into the
host execution context. When the guest exits, the corrupted debug registers
remain active when the host resumes execution, allowing an untrusted protected
guest to mount an attack against the host kernel.

>  	if (kvm_guest_owns_debug_regs(&hyp_vcpu->vcpu))
>  		host_vcpu->arch.vcpu_debug_state = hyp_vcpu->vcpu.arch.vcpu_debug_state;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907070002.3333525-1-fuad.tabba@linux.dev?part=4