Re: [PATCH v2] xen/pvcalls: bound backend response req_id before indexing rsp[]

[Juergen Gross]

On 17.06.26 03:41, Michael Bommarito wrote:
> pvcalls_front_event_handler() takes req_id directly from the
> backend-supplied ring response and uses it to index the fixed-size
> bedata->rsp[] array for a memcpy() and a store, with no range check. A
> malicious or buggy backend can set req_id past PVCALLS_NR_RSP_PER_RING
> and drive an out-of-bounds write past the bedata allocation.
>
> req_id was also declared int while the wire field rsp->req_id is u32, so
> a range check on the signed value alone is insufficient: a backend
> req_id of 0xffffffff becomes -1, passes a >= PVCALLS_NR_RSP_PER_RING
> test and indexes bedata->rsp[-1]. Declare req_id as u32 so a single
> bound covers both ends.
>
> A backend that sends an out-of-range req_id has violated the wire
> protocol, so rather than silently dropping the response, log once and
> stop trusting the backend: set bedata->disabled. The event handler then
> ignores further responses, and the request paths that wait for a
> response return -EIO instead of blocking forever. This mirrors the
> fatal-error handling xen-netback uses (xenvif_fatal_tx_err()).
>
> The pvcalls frontend currently trusts its backend, so this is not a
> classic-Xen security issue, but it matters for hardening PV frontends
> against malicious backends (confidential and disaggregated deployments).
>
> Fixes: 2195046bfd69 ("xen/pvcalls: implement socket command and handle events")
> Suggested-by: Juergen Gross <jgross@xxxxxxxx>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>

Thanks,


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature