Re: [PATCH 09/17] KVM: arm64: Handle PSCI calls for protected VMs at EL2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Malicious host can hijack guest execution flow via CPU_ON TOCTOU race caused by stale reset_state.reset flag.
--

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

KVM: arm64: Handle PSCI calls for protected VMs at EL2

This patch implements PSCI 1.1 for protected VMs in EL2, handling power state
transitions (CPU_ON, CPU_OFF, AFFINITY_INFO) within the hypervisor, while
keeping non-protected VMs unchanged. It defines protected vCPU power states
and synchronizes racing transitions via cmpxchg and release/acquire barriers.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
[ ... ]
> @@ -37,27 +40,103 @@ typedef void (*hyp_entry_exit_handler_fn)(struct pkvm_hyp_vcpu *);
>  
>  static void __maybe_unused handle_pvm_entry_hvc64(struct pkvm_hyp_vcpu *hyp_vcpu)
>  {
[ ... ]
> +			prev = cmpxchg_relaxed(&target_vcpu->power_state,
> +					       PSCI_0_2_AFFINITY_LEVEL_ON_PENDING,
> +					       PSCI_0_2_AFFINITY_LEVEL_OFF);
> +			switch (prev) {
> +			case PSCI_0_2_AFFINITY_LEVEL_ON_PENDING:
> +				/*
> +				 * Leave reset_state.reset set: clearing it
> +				 * races a concurrent CPU_ON's re-publish and
> +				 * wedges the target at ON_PENDING. The stale
> +				 * pc/r0/be are the guest's own.
> +				 */
> +				ret = PSCI_RET_INTERNAL_FAILURE;
> +				break;

[Severity: High]
Does this rollback logic allow a malicious host to hijack guest execution flow
by exploiting a TOCTOU race?

If guest vCPU A calls CPU_ON for target T with PC1, the hypervisor sets
power_state = ON_PENDING. If the host then rejects the call, this rollback
logic resets power_state to OFF but intentionally leaves reset_state.reset =
true and pc = PC1.

If guest vCPU B then calls CPU_ON for target T with PC2, pvm_psci_vcpu_on()
sets power_state = ON_PENDING. Before the hypervisor writes PC2, a malicious
host could concurrently issue KVM_RUN for target T on another CPU.

Because the stale reset flag is still true from the previous failure, target T
will immediately boot using the stale PC1 instead of PC2, redirecting the
guest control flow.

Should this rollback clear the reset state, or is there another way to prevent
the host from forcing a boot with a stale PC?

[ ... ]
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
[ ... ]
> +int pkvm_reset_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
> +{
> +	struct vcpu_reset_state *reset_state = &hyp_vcpu->vcpu.arch.reset_state;
> +	int prev;
> +
> +	/*
> +	 * Pairs with smp_store_release(&reset_state->reset, true) in
> +	 * pvm_psci_vcpu_on(). The acquire must precede the cmpxchg: reversed, a
> +	 * winning cmpxchg with a false acquire would leave power_state == ON
> +	 * with the reset skipped.
> +	 */
> +	if (!smp_load_acquire(&reset_state->reset))
> +		return -ECANCELED;

[Severity: High]
Can the host force pkvm_reset_vcpu() to succeed prematurely here?

Since the rollback in handle_pvm_entry_hvc64() leaves reset_state.reset
set to true, if a new CPU_ON sets power_state to ON_PENDING, a host-controlled
KVM_RUN can enter this function and pass this smp_load_acquire() check before
the new PC is actually written by pvm_psci_vcpu_on().

[ ... ]
> +static bool pvm_psci_vcpu_on(struct pkvm_hyp_vcpu *hyp_vcpu)
> +{
[ ... ]
> +	/*
> +	 * vCPUs race to power on the same target. Relaxed: reset_state
> +	 * is published by the release on reset_state.reset below.
> +	 */
> +	power_state = cmpxchg_relaxed(&target->power_state,
> +				      PSCI_0_2_AFFINITY_LEVEL_OFF,
> +				      PSCI_0_2_AFFINITY_LEVEL_ON_PENDING);

[Severity: High]
Does setting power_state to ON_PENDING here open a TOCTOU window?

If the target already has a stale reset_state.reset == true from a previously
failed CPU_ON attempt, making the target ON_PENDING here allows the host to
schedule the target and boot it before the new PC/r0/be states are updated
below.

> +	switch (power_state) {
[ ... ]

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