mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-07-22 02:17:36 -04:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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: 2195046bfd ("xen/pvcalls: implement socket command and handle events")
Suggested-by: Juergen Gross <jgross@suse.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Message-ID: <20260617014149.2647404-1-michael.bommarito@gmail.com>
This commit is contained in:
Michael Bommarito
2026-06-16 21:41:49 -04:00
committed by Juergen Gross
parent dc59e4fea9
commit d33846c8dc
1 changed files with 76 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
drivers/xen/pvcalls-front.c
View File

@@ -32,6 +32,7 @@ struct pvcalls_bedata {
struct xen_pvcalls_front_ring ring;
grant_ref_t ref;
int irq;
bool disabled;
struct list_head socket_mappings;
spinlock_t socket_lock;
@@ -131,6 +132,20 @@ static inline int get_request(struct pvcalls_bedata *bedata, int *req_id)
return 0;
}
/*
* Wait for the backend's response to req_id, or for the frontend to be
* disabled because the backend violated the wire protocol. Returns 0 once
* the response has arrived, or -EIO if the frontend was disabled.
*/
static int pvcalls_front_wait_rsp(struct pvcalls_bedata *bedata, u32 req_id)
{
wait_event(bedata->inflight_req,
READ_ONCE(bedata->rsp[req_id].req_id) == req_id ||
READ_ONCE(bedata->disabled));
return READ_ONCE(bedata->disabled) ? -EIO : 0;
}
static bool pvcalls_front_write_todo(struct sock_mapping *map)
{
struct pvcalls_data_intf *intf = map->active.ring;
@@ -168,7 +183,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;
@@ -179,12 +195,31 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id)
pvcalls_exit();
return IRQ_HANDLED;
}
if (READ_ONCE(bedata->disabled)) {
pvcalls_exit();
return IRQ_HANDLED;
}
again:
while (RING_HAS_UNCONSUMED_RESPONSES(&bedata->ring)) {
rsp = RING_GET_RESPONSE(&bedata->ring, bedata->ring.rsp_cons);
req_id = rsp->req_id;
if (req_id >= PVCALLS_NR_RSP_PER_RING) {
/*
* The backend supplied a req_id that would index
* bedata->rsp[] out of bounds: a protocol violation
* from a malicious or buggy backend. Log once, stop
* trusting this backend and disable the frontend rather
* than silently dropping the response and continuing.
*/
pr_err_once("pvcalls: backend sent out-of-range req_id %u, disabling frontend\n",
req_id);
WRITE_ONCE(bedata->disabled, true);
bedata->ring.rsp_cons++;
done = 1;
break;
}
if (rsp->cmd == PVCALLS_POLL) {
struct sock_mapping *map = (struct sock_mapping *)(uintptr_t)
rsp->u.poll.id;
@@ -217,7 +252,7 @@ static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id)
}
RING_FINAL_CHECK_FOR_RESPONSES(&bedata->ring, more);
if (more)
if (more && !READ_ONCE(bedata->disabled))
goto again;
if (done)
wake_up(&bedata->inflight_req);
@@ -330,8 +365,11 @@ int pvcalls_front_socket(struct socket *sock)
if (notify)
notify_remote_via_irq(bedata->irq);
wait_event(bedata->inflight_req,
READ_ONCE(bedata->rsp[req_id].req_id) == req_id);
ret = pvcalls_front_wait_rsp(bedata, req_id);
if (ret) {
pvcalls_exit();
return ret;
}
/* read req_id, then the content */
smp_rmb();
@@ -477,8 +515,11 @@ int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr,
if (notify)
notify_remote_via_irq(bedata->irq);
wait_event(bedata->inflight_req,
READ_ONCE(bedata->rsp[req_id].req_id) == req_id);
ret = pvcalls_front_wait_rsp(bedata, req_id);
if (ret) {
pvcalls_exit_sock(sock);
return ret;
}
/* read req_id, then the content */
smp_rmb();
@@ -711,8 +752,11 @@ int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
if (notify)
notify_remote_via_irq(bedata->irq);
wait_event(bedata->inflight_req,
READ_ONCE(bedata->rsp[req_id].req_id) == req_id);
ret = pvcalls_front_wait_rsp(bedata, req_id);
if (ret) {
pvcalls_exit_sock(sock);
return ret;
}
/* read req_id, then the content */
smp_rmb();
@@ -761,8 +805,11 @@ int pvcalls_front_listen(struct socket *sock, int backlog)
if (notify)
notify_remote_via_irq(bedata->irq);
wait_event(bedata->inflight_req,
READ_ONCE(bedata->rsp[req_id].req_id) == req_id);
ret = pvcalls_front_wait_rsp(bedata, req_id);
if (ret) {
pvcalls_exit_sock(sock);
return ret;
}
/* read req_id, then the content */
smp_rmb();
@@ -820,6 +867,14 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock,
}
}
if (READ_ONCE(bedata->disabled)) {
clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT,
(void *)&map->passive.flags);
wake_up(&map->passive.inflight_accept_req);
pvcalls_exit_sock(sock);
return -EIO;
}
map2 = kzalloc_obj(*map2);
if (map2 == NULL) {
clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT,
@@ -880,10 +935,18 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock,
}
if (wait_event_interruptible(bedata->inflight_req,
READ_ONCE(bedata->rsp[req_id].req_id) == req_id)) {
READ_ONCE(bedata->rsp[req_id].req_id) == req_id ||
READ_ONCE(bedata->disabled))) {
pvcalls_exit_sock(sock);
return -EINTR;
}
if (READ_ONCE(bedata->disabled)) {
clear_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT,
(void *)&map->passive.flags);
wake_up(&map->passive.inflight_accept_req);
pvcalls_exit_sock(sock);
return -EIO;
}
/* read req_id, then the content */
smp_rmb();
@@ -1054,7 +1117,8 @@ int pvcalls_front_release(struct socket *sock)
notify_remote_via_irq(bedata->irq);
wait_event(bedata->inflight_req,
READ_ONCE(bedata->rsp[req_id].req_id) == req_id);
READ_ONCE(bedata->rsp[req_id].req_id) == req_id ||
READ_ONCE(bedata->disabled));
if (map->active_socket) {
/*