All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Fuad Tabba" <fuad.tabba@linux.dev>
Cc: "Marc Zyngier" <maz@kernel.org>,
	"Oliver Upton" <oupton@kernel.org>,
	kvmarm@lists.linux.dev
Subject: Re: [PATCH 1/2] KVM: arm64: Flush external_mdscr_el1 to the pKVM hyp vCPU
Date: Sun, 26 Jul 2026 14:54:39 +0000	[thread overview]
Message-ID: <20260726145440.2D4CA1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260726143643.303456-2-fuad.tabba@linux.dev>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] The patch breaks pKVM guest isolation by allowing the host to arm hardware debug exceptions against a protected VM.

Pre-existing issues:
- [Critical] The hypervisor unconditionally copies trap configurations and debug states between the untrusted host and the protected VM on every entry and exit, violating pKVM state isolation and confidentiality.
--

--- Patch [1]: [PATCH 1/2] KVM: arm64: Flush external_mdscr_el1 to the pKVM hyp vCPU ---
commit 11c3469e00f567f8f8ae658ca753847976608a5d
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Flush external_mdscr_el1 to the pKVM hyp vCPU

This commit propagates the external_mdscr_el1 register state to the pKVM
hyp vCPU during flush_debug_state(). This ensures that a non-protected guest
runs with the correct hardware flags set by the host.

> 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
> @@ -108,10 +108,16 @@ static void flush_debug_state(struct pkvm_hyp_vcpu *hyp_vcpu)
>  
>  	hyp_vcpu->vcpu.arch.debug_owner = host_vcpu->arch.debug_owner;

[Severity: Critical]
This is a pre-existing issue, but does this allow the host to hijack debug
ownership for a protected VM?

The hypervisor unconditionally copies the untrusted host's debug_owner here
on every guest entry.

Additionally, similar unconditional copying occurs for trap configurations 
and debug states from the host structure:

arch/arm64/kvm/hyp/nvhe/hyp-main.c:flush_hyp_vcpu() {
    ...
    hyp_vcpu->vcpu.arch.mdcr_el2 = host_vcpu->arch.mdcr_el2;
    ...
}

And on guest exit, state is copied back out to the host:

arch/arm64/kvm/hyp/nvhe/hyp-main.c:sync_debug_state() {
    ...
    if (kvm_guest_owns_debug_regs(&hyp_vcpu->vcpu))
        host_vcpu->arch.vcpu_debug_state = hyp_vcpu->vcpu.arch.vcpu_debug_state;
    ...
}

Could this allow the host to reconfigure the protected guest's execution
environment via mdcr_el2, and leak private hardware debug register state 
back to the untrusted host?

> -	if (kvm_guest_owns_debug_regs(&hyp_vcpu->vcpu))
> +	if (kvm_guest_owns_debug_regs(&hyp_vcpu->vcpu)) {
>  		hyp_vcpu->vcpu.arch.vcpu_debug_state = host_vcpu->arch.vcpu_debug_state;
> -	else if (kvm_host_owns_debug_regs(&hyp_vcpu->vcpu))
> +	} else if (kvm_host_owns_debug_regs(&hyp_vcpu->vcpu)) {
>  		hyp_vcpu->vcpu.arch.external_debug_state = host_vcpu->arch.external_debug_state;
> +		/*
> +		 * The world switch loads MDSCR_EL1 from external_mdscr_el1
> +		 * (ctxt_mdscr_el1()).
> +		 */
> +		hyp_vcpu->vcpu.arch.external_mdscr_el1 = host_vcpu->arch.external_mdscr_el1;

[Severity: Critical]
Is this a regression that breaks pKVM guest isolation by allowing the host to 
arm hardware debug exceptions against a protected VM?

By copying external_mdscr_el1 from the untrusted host to the EL2 private
state unconditionally, a malicious host could populate external_mdscr_el1
with debug flags (e.g., MDE=1 and SS=1) prior to KVM_RUN.

The world switch would then load these host-controlled values directly into 
the hardware MDSCR_EL1 register. Under the pKVM threat model, could this 
allow the host to intercept execution, extract register state, and modify 
control flow of a protected VM?

> +	}
>  }

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

  reply	other threads:[~2026-07-26 14:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-26 14:36 [PATCH 0/2] KVM: arm64: Fix host-directed debug for non-protected pKVM guests Fuad Tabba
2026-07-26 14:36 ` [PATCH 1/2] KVM: arm64: Flush external_mdscr_el1 to the pKVM hyp vCPU Fuad Tabba
2026-07-26 14:54   ` sashiko-bot [this message]
2026-07-26 14:58     ` Fuad Tabba
2026-07-26 14:36 ` [PATCH 2/2] KVM: arm64: selftests: Add a userspace watchpoint test Fuad Tabba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260726145440.2D4CA1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=fuad.tabba@linux.dev \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.