Re: [PATCH v2 11/11] KVM: arm64: Tag host-VA hypercall parameters __kern

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The `kern_hyp_va_host()` macro strips the `__kern` tag using `(__force void *)` and returns a plain pointer, undermining the static analysis guarantees for both misapplied unwraps and subsequent TOCTOU bugs.

Pre-existing issues:
- [Critical] Always-on hypercalls (e.g., `__vgic_v3_save_aprs`, `__kvm_adjust_pc`) accept host VAs, translate them via `kern_hyp_va_host()`, and read/write to them without validating physical page ownership, enabling a Confused Deputy arbitrary memory corruption attack.
--

--- Patch [11]: [PATCH v2 11/11] KVM: arm64: Tag host-VA hypercall parameters __kern ---
commit 0acf480afb506d5122464f1024574511fad649b3
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Tag host-VA hypercall parameters __kern

The nVHE hypervisor takes host virtual addresses as hypercall arguments
and translates each with kern_hyp_va() before use. This patch adds a __kern 
sparse address space for EL2 code to tag host-VA parameters and introduces 
kern_hyp_va_host() to unwrap them.

> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 6eae7e7e2a684..57f65257bb56d 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
[ ... ]
> @@ -140,6 +142,14 @@ static __always_inline unsigned long __kern_hyp_va(unsigned long v)
>  
>  #define kern_hyp_va(v) 	((typeof(v))(__kern_hyp_va((unsigned long)(v))))
>  
> +/*
> + * Translate a __kern-tagged host VA, dropping the tag: the only sanctioned
> + * unwrap. Translation only, no ownership or bounds validation; the result
> + * carries the pointee type stripped of the tag and of any cv-qualifiers.
> + */
> +#define kern_hyp_va_host(v)						\
> +	((TYPEOF_UNQUAL(*(v)) *)__kern_hyp_va((unsigned long)(__force void *)(v)))

[Severity: Medium]
Since the __force void * cast unconditionally strips all sparse address space
information, does this suppress warnings if the macro is accidentally applied
to an untagged EL2 private-VA-range pointer?

Additionally, because the returned pointer has no address space tag, doesn't
this prevent sparse from warning if a developer directly double-fetches or acts
on the translated untrusted host memory instead of copying it first?

For example, in __pkvm_init_vm(), host_kvm is passed as a plain pointer and
loses its sparse static analysis protection despite still aliasing untrusted
memory.

[ ... ]
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 6986ce55fd9ef..f8ea1661b3166 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
[ ... ]
> @@ -500,15 +500,15 @@ DEFINE_KVM_HOST_HCALL0_VOID(__vgic_v3_init_lrs)
>  }
>  
>  DEFINE_KVM_HOST_HCALL_VOID(__vgic_v3_save_aprs,
> -	struct vgic_v3_cpu_if *, cpu_if)
> +	struct vgic_v3_cpu_if __kern *, cpu_if)
>  {
> -	__vgic_v3_save_aprs(kern_hyp_va(cpu_if));
> +	__vgic_v3_save_aprs(kern_hyp_va_host(cpu_if));
>  }

[Severity: Critical]
This is a pre-existing issue, but by retaining the lack of bounds validation
through kern_hyp_va_host(), can the host EL1 trigger an arbitrary memory
corruption in the hypervisor?

Since kern_hyp_va_host() explicitly performs no ownership or bounds validation,
it appears the host kernel could provide a forged pointer that translates to a
physical page owned by EL2.

The hypervisor then passes this pointer directly to __vgic_v3_save_aprs()
after unvalidated translation:

arch/arm64/kvm/hyp/vgic-v3-sr.c:__vgic_v3_save_aprs() {
    ...
    cpu_if->vgic_ap0r[3] = __vgic_v3_read_ap0rn(3);
    ...
}

which directly writes to the untrusted pointer, potentially allowing the host
to corrupt EL2 private memory and bypass isolation.

[ ... ]

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