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

On Tue, 4 Aug 2026 at 12:41, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issu=
e(s) to consider:
>
> Pre-existing issues:
> - [High] Missing context synchronization event (ISB) after writing to SYS=
_SCTLR.

I thought we'd covered this [1]. The ERET back to the host is itself a
CSE, and SCTLR_EL2.DSSBS only sets the default PSTATE.SSBS on the
*next* exception entry to EL2, nothing executing at EL2 consumes it
before the return, so an ISB is redundant.

> - [High] Double-fetch of untrusted host memory in pkvm_refill_memcache() =
leading to a TOCTOU vulnerability.

Not a TOCTOU: __pkvm_host_donate_hyp() does a host+hyp-locked
ownership transition on each page before EL2 dereferences it, and
nr_pages is only the loop's target count. Worst case the host
under-donates its own pages and starves its own vCPU.

/fuad


[1] https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/ar=
m64.md


> --
>
> --- Patch [10]: [PATCH v3 10/11] KVM: arm64: nVHE: Check hypercall handle=
rs 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-ch=
ecked
> 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/nv=
he/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;
> >
> >       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 ho=
st
> 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 mig=
ht
> 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;
> >
> >       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() instructio=
n be
> issued after the write to SYS_SCTLR?
>
> Updates to control-plane system registers like SYS_SCTLR require a Contex=
t
> Synchronization Event (CSE) to be visible to subsequent instructions.
> Returning without an immediate isb() might leave the processor operating =
with
> an architecturally undefined pipeline state.
>
> --
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804112317.19=
[email protected]?part=3D10