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

Michael Bommarito posted 1 patch 2 days, 18 hours ago
Failed in applying to current master (apply log)
There is a newer version of this series
drivers/xen/pvcalls-front.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH] xen/pvcalls: bound backend response req_id before indexing rsp[]
Posted by Michael Bommarito 2 days, 18 hours ago
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 is insufficient on its own: a backend
req_id of 0xffffffff becomes -1, passes the >= PVCALLS_NR_RSP_PER_RING
test, and indexes bedata->rsp[-1], an out-of-bounds write to the left of
the array. Declare req_id as u32 and add the range check so both ends of
the index are covered.

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).
Reject responses whose req_id is out of range.

Fixes: 235a71c53903 ("xen/pvcalls: implement release command")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/xen/pvcalls-front.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 50ce4820f7eeb..78bd4e894b32e 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -168,7 +168,8 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id)
 	struct pvcalls_bedata *bedata;
 	struct xen_pvcalls_response *rsp;
 	uint8_t *src, *dst;
-	int req_id = 0, more = 0, done = 0;
+	u32 req_id = 0;
+	int more = 0, done = 0;
 
 	if (dev == NULL)
 		return IRQ_HANDLED;
@@ -185,6 +186,12 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id)
 		rsp = RING_GET_RESPONSE(&bedata->ring, bedata->ring.rsp_cons);
 
 		req_id = rsp->req_id;
+		if (req_id >= PVCALLS_NR_RSP_PER_RING) {
+			/* Malicious or buggy backend: req_id out of range. */
+			bedata->ring.rsp_cons++;
+			done = 1;
+			continue;
+		}
 		if (rsp->cmd == PVCALLS_POLL) {
 			struct sock_mapping *map = (struct sock_mapping *)(uintptr_t)
 						   rsp->u.poll.id;
-- 
2.53.0
Re: [PATCH] xen/pvcalls: bound backend response req_id before indexing rsp[]
Posted by Juergen Gross 2 days, 18 hours ago
On 10.06.26 13: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 is insufficient on its own: a backend
> req_id of 0xffffffff becomes -1, passes the >= PVCALLS_NR_RSP_PER_RING
> test, and indexes bedata->rsp[-1], an out-of-bounds write to the left of
> the array. Declare req_id as u32 and add the range check so both ends of
> the index are covered.
> 
> 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).
> Reject responses whose req_id is out of range.
> 
> Fixes: 235a71c53903 ("xen/pvcalls: implement release command")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>   drivers/xen/pvcalls-front.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
> index 50ce4820f7eeb..78bd4e894b32e 100644
> --- a/drivers/xen/pvcalls-front.c
> +++ b/drivers/xen/pvcalls-front.c
> @@ -168,7 +168,8 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id)
>   	struct pvcalls_bedata *bedata;
>   	struct xen_pvcalls_response *rsp;
>   	uint8_t *src, *dst;
> -	int req_id = 0, more = 0, done = 0;
> +	u32 req_id = 0;
> +	int more = 0, done = 0;
>   
>   	if (dev == NULL)
>   		return IRQ_HANDLED;
> @@ -185,6 +186,12 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id)
>   		rsp = RING_GET_RESPONSE(&bedata->ring, bedata->ring.rsp_cons);
>   
>   		req_id = rsp->req_id;
> +		if (req_id >= PVCALLS_NR_RSP_PER_RING) {
> +			/* Malicious or buggy backend: req_id out of range. */

Please add an error message here, using pr_err_once().

Instead of just ignoring such responses, I'd consider stopping communication
with the backend for this device.


Juergen
Re: [PATCH] xen/pvcalls: bound backend response req_id before indexing rsp[]
Posted by Michael Bommarito 2 days, 18 hours ago
On Wed, Jun 10, 2026 at 7:48 AM Juergen Gross <jgross@suse.com> wrote:
> Please add an error message here, using pr_err_once().
>
> Instead of just ignoring such responses, I'd consider stopping communication
> with the backend for this device.

Good points.  Do you know if there is a canonical way to track
bad/buggy backends I should reference or reuse?
Re: [PATCH] xen/pvcalls: bound backend response req_id before indexing rsp[]
Posted by Jürgen Groß 2 days, 17 hours ago
On 10.06.26 13:50, Michael Bommarito wrote:
> On Wed, Jun 10, 2026 at 7:48 AM Juergen Gross <jgross@suse.com> wrote:
>> Please add an error message here, using pr_err_once().
>>
>> Instead of just ignoring such responses, I'd consider stopping communication
>> with the backend for this device.
> 
> Good points.  Do you know if there is a canonical way to track
> bad/buggy backends I should reference or reuse?

You could follow the xen-netback example (see xenvif_fatal_tx_err() and its
callers in drivers/net/xen-netback/netback.c).


Juergen