Re: [PATCH v3 03/18] KVM: arm64: Pin the host vCPU before adjusting its PC under pKVM

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Unmapped host `kvm_s2_mmu` pointer dereference in EL2 TLB flush handlers panics the hypervisor before the first KVM_RUN.
--

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

KVM: arm64: Pin the host vCPU before adjusting its PC under pKVM

This commit fixes a hypervisor panic in pKVM caused by dereferencing an
unmapped host vCPU before its first KVM_RUN. It introduces temporary pinning
of the host vCPU around __kvm_adjust_pc() when no hyp vCPU is loaded.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index b6bfe502bcd04..52a65b13b3810 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -546,8 +546,25 @@ static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt)
>  static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt)
>  {
>  	DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
> +	struct pkvm_hyp_vcpu *hyp_vcpu;
> +	struct kvm_vcpu *host_vcpu;
> +
> +	host_vcpu = __get_host_hyp_vcpus(vcpu, &hyp_vcpu);
> +	if (host_vcpu) {
> +		__kvm_adjust_pc(host_vcpu);
> +		return;
> +	}
> +
> +	/*
> +	 * With no hyp vCPU loaded for it, the host vCPU may be unpinned,
> +	 * and so unmapped at EL2: its first run pins it.
> +	 */
> +	host_vcpu = kern_hyp_va(vcpu);
> +	if (hyp_pin_shared_mem(host_vcpu, host_vcpu + 1))
> +		return;
>  
> -	__kvm_adjust_pc(kern_hyp_va(vcpu));
> +	__kvm_adjust_pc(host_vcpu);
> +	hyp_unpin_shared_mem(host_vcpu, host_vcpu + 1);
>  }

[Severity: Critical]
This is a pre-existing issue, but does this same vulnerability apply to the
TLB flush handlers in this file?

If a userspace process creates a VM and a memory slot, and then immediately
deletes it via KVM_SET_USER_MEMORY_REGION with a zero memory size before
the first KVM_RUN, this triggers kvm_arch_flush_shadow_memslot().

This unconditionally calls kvm_tlb_flush_vmid_range(), which issues an HVC
call that ends up in handle___kvm_tlb_flush_vmid_range():

arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___kvm_tlb_flush_vmid_range() {
	DECLARE_REG(struct kvm_s2_mmu *, mmu, host_ctxt, 1);
	DECLARE_REG(phys_addr_t, start, host_ctxt, 2);
	DECLARE_REG(unsigned long, pages, host_ctxt, 3);

	__kvm_tlb_flush_vmid_range(kern_hyp_va(mmu), start, pages);
}

Since the kvm struct (containing kvm_s2_mmu) is not pinned until
pkvm_create_hyp_vm() runs during the first KVM_RUN, passing kern_hyp_va(mmu)
without pinning it first might cause a data abort at EL2.

The unmapped pointer is eventually dereferenced in enter_vmid_context():

arch/arm64/kvm/hyp/nvhe/tlb.c:enter_vmid_context() {
	...
	__load_stage2(mmu);
	...
}

arch/arm64/kvm/hyp/nvhe/tlb.c:__load_stage2() {
	write_sysreg(mmu->vtcr, vtcr_el2);
	...
}

Since this is the exact same sequence described in the commit message (a host
pointer unmapped at EL2 being dereferenced before the first run), should we
also pin the mmu pointer around these TLB flush operations?

>  
>  static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt)

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