Re: [PATCH v3 10/11] KVM: arm64: nVHE: Check hypercall handlers against the declared ABI

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

Pre-existing issues:
- [High] Missing context synchronization event (ISB) after writing to SYS_S=
CTLR.
- [High] Double-fetch of untrusted host memory in pkvm_refill_memcache() le=
ading to a TOCTOU vulnerability.
--

--- Patch [10]: [PATCH v3 10/11] KVM: arm64: nVHE: Check hypercall handlers=
 against the declared ABI ---
commit c02cd7851991d5d8c9aac5af1e86684fe8258ca4
Author: Fuad Tabba <[email protected]>
KVM: arm64: nVHE: Check hypercall handlers against the declared ABI

This patch replaces manually unmarshalled hypercall arguments using
DECLARE_REG() with generated handlers via DEFINE_KVM_HOST_HCALL() and
DEFINE_KVM_HOST_HCALL0(). This ensures that the parameter list is type-chec=
ked
against the signature declared in <asm/kvm_hcall.h> to prevent mismatches
between caller and handler.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe=
/hyp-main.c
> index 23cb4313c60a2..675d607727929 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c

[ ... ]

>  static int pkvm_refill_memcache(struct pkvm_hyp_vcpu *hyp_vcpu)
>  {
>  	struct kvm_vcpu *host_vcpu =3D hyp_vcpu->host_vcpu;
> =20
>  	return refill_memcache(&hyp_vcpu->vcpu.arch.pkvm_memcache,
>  			       host_vcpu->arch.pkvm_memcache.nr_pages,
>  			       &host_vcpu->arch.pkvm_memcache);
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This is a pre-existing issue, but could this double-fetch of untrusted host
memory lead to a TOCTOU race condition?

The host memory is dereferenced here to read nr_pages and then passed as a
pointer to refill_memcache(), which fetches from it again:

arch/arm64/kvm/hyp/nvhe/mm.c:refill_memcache() {
    ...
    struct kvm_hyp_memcache tmp =3D *host_mc;
    ...
}

If the host concurrently modifies the struct between the fetches, EL2 might
use a min_pages target that differs from the actual struct state. Should the
struct be copied to EL2 private memory first before acting on it?

>  }

[ ... ]

> -static void handle___kvm_enable_ssbs(struct kvm_cpu_context *host_ctxt)
> +DEFINE_KVM_HOST_HCALL0(void, __kvm_enable_ssbs)
>  {
>  	u64 tmp;
> =20
>  	tmp =3D read_sysreg_el2(SYS_SCTLR);
>  	tmp |=3D SCTLR_ELx_DSSBS;
>  	write_sysreg_el2(tmp, SYS_SCTLR);
>  }
   ^

[Severity: High]
This isn't a bug introduced by this patch, but should an isb() instruction =
be
issued after the write to SYS_SCTLR?

Updates to control-plane system registers like SYS_SCTLR require a Context
Synchronization Event (CSE) to be visible to subsequent instructions.
Returning without an immediate isb() might leave the processor operating wi=
th
an architecturally undefined pipeline state.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804112317.1937=
[email protected]?part=3D10