Re: [PATCH v4 06/17] KVM: arm64: Add pkvm_hyp_req infrastructure

On Fri, 31 Jul 2026 at 16:06, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Phantom hypervisor requests leading to infinite loop and memory leak on hypercall errors.
> --
>
> --- Patch [6]: [PATCH v4 06/17] KVM: arm64: Add pkvm_hyp_req infrastructure ---
> commit ec846697735578e1b5a4e25b88e07602c29ae6a2
> Author: Vincent Donnefort <[email protected]>
>
> KVM: arm64: Add pkvm_hyp_req infrastructure
>
> Introduce a struct pkvm_hyp_req to enable the pKVM hypervisor to request
> resources from the host. Provide serialisation helpers to transport these
> requests via SMCCC registers and a macro to automate HVC retries.
>
> > diff --git a/arch/arm64/include/asm/kvm_pkvm.h b/arch/arm64/include/asm/kvm_pkvm.h
> > index 74fedd9c5ff02..6c2497047f8d8 100644
> > --- a/arch/arm64/include/asm/kvm_pkvm.h
> > +++ b/arch/arm64/include/asm/kvm_pkvm.h
>
> [ ... ]
>
> > +/* Return true if a pkvm_hyp_req has been decoded from the SMCCC args */
> > +static inline bool smccc_to_pkvm_hyp_req(struct pkvm_hyp_req *req, struct arm_smccc_res *res)
> > +{
> > +     u8 *src = (u8 *)res + offsetof(struct arm_smccc_res, a2);
> > +     u8 type = *src;
> > +
> > +     if (type == PKVM_HYP_NO_REQ || type >= __PKVM_HYP_REQ_TYPE_MAX)
> > +             return false;
> > +
> > +     req->type = type;
> > +     memcpy(&req->args, src + 1, pkvm_hyp_req_arg_size(type));
> > +
> > +     return true;
> > +}
> > +
> > +int __pkvm_handle_smccc_req(struct arm_smccc_res *res);
> > +
> > +#define pkvm_call_hyp_req(f, ...)                                                            \
> > +({                                                                                           \
> > +     struct arm_smccc_res __res;                                                             \
> > +     int __ret;                                                                              \
> > +     do {                                                                                    \
> > +             __ret = -1;                                                                     \
> > +             arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f), ##__VA_ARGS__, &__res);               \
> > +             if (WARN_ON(__res.a0 != SMCCC_RET_SUCCESS))                                     \
> > +                     break;                                                                  \
> > +             __ret = __res.a1;                                                               \
> > +             if (!__ret)                                                                     \
> > +                     break;                                                                  \
> > +             __ret = __pkvm_handle_smccc_req(&__res);                                        \
> > +     } while (!__ret);                                                                       \
> > +     __ret;                                                                                  \
> > +})
>
> [Severity: High]
> Can this sequence lead to an infinite loop and memory leak on hypercall errors?

This does not happen. Every handler reachable through
pkvm_call_hyp_req() goes via errno_to_smccc(), which always calls
pkvm_hyp_req_to_smccc(), and that writes 0 to x2 when there is no
request to pass back. So the decode never sees a stale argument.

/fuad

>
> When the hypercall fails and returns a non-zero error in __res.a1, the macro
> passes the result to __pkvm_handle_smccc_req() to check for a piggybacked
> request in __res.a2.
>
> Standard EL2 handlers typically do not clear the x2 register (which maps to a2)
> on error; they set x1 to the error code and return. This means a2 retains the
> original second hypercall argument passed by the host.
>
> If the least significant byte of that original host argument happens to match
> a valid request type, smccc_to_pkvm_hyp_req() would mistakenly decode a phantom
> request. The host would handle this phantom request, return success, and the
> macro would then infinitely retry the failing hypercall.
>
> Is there a way to explicitly distinguish a hypercall error from a legitimate
> hypervisor request, perhaps using a specific return code in a1 to indicate when
> a2 actually contains a request?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
>