Commit 534fc31d authored by Ross Lagerwall's avatar Ross Lagerwall Committed by Juergen Gross

xen/netback: Fix buffer overrun triggered by unusual packet

It is possible that a guest can send a packet that contains a head + 18
slots and yet has a len <= XEN_NETBACK_TX_COPY_LEN. This causes nr_slots
to underflow in xenvif_get_requests() which then causes the subsequent
loop's termination condition to be wrong, causing a buffer overrun of
queue->tx_map_ops.

Rework the code to account for the extra frag_overflow slots.

This is CVE-2023-34319 / XSA-432.

Fixes: ad7f402a ("xen/netback: Ensure protocol headers don't fall in the non-linear area")
Signed-off-by: default avatarRoss Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: default avatarPaul Durrant <paul@xen.org>
Reviewed-by: default avatarWei Liu <wei.liu@kernel.org>
Signed-off-by: default avatarJuergen Gross <jgross@suse.com>
parent ec351c8f
...@@ -396,7 +396,7 @@ static void xenvif_get_requests(struct xenvif_queue *queue, ...@@ -396,7 +396,7 @@ static void xenvif_get_requests(struct xenvif_queue *queue,
struct gnttab_map_grant_ref *gop = queue->tx_map_ops + *map_ops; struct gnttab_map_grant_ref *gop = queue->tx_map_ops + *map_ops;
struct xen_netif_tx_request *txp = first; struct xen_netif_tx_request *txp = first;
nr_slots = shinfo->nr_frags + 1; nr_slots = shinfo->nr_frags + frag_overflow + 1;
copy_count(skb) = 0; copy_count(skb) = 0;
XENVIF_TX_CB(skb)->split_mask = 0; XENVIF_TX_CB(skb)->split_mask = 0;
...@@ -462,8 +462,8 @@ static void xenvif_get_requests(struct xenvif_queue *queue, ...@@ -462,8 +462,8 @@ static void xenvif_get_requests(struct xenvif_queue *queue,
} }
} }
for (shinfo->nr_frags = 0; shinfo->nr_frags < nr_slots; for (shinfo->nr_frags = 0; nr_slots > 0 && shinfo->nr_frags < MAX_SKB_FRAGS;
shinfo->nr_frags++, gop++) { shinfo->nr_frags++, gop++, nr_slots--) {
index = pending_index(queue->pending_cons++); index = pending_index(queue->pending_cons++);
pending_idx = queue->pending_ring[index]; pending_idx = queue->pending_ring[index];
xenvif_tx_create_map_op(queue, pending_idx, txp, xenvif_tx_create_map_op(queue, pending_idx, txp,
...@@ -476,12 +476,12 @@ static void xenvif_get_requests(struct xenvif_queue *queue, ...@@ -476,12 +476,12 @@ static void xenvif_get_requests(struct xenvif_queue *queue,
txp++; txp++;
} }
if (frag_overflow) { if (nr_slots > 0) {
shinfo = skb_shinfo(nskb); shinfo = skb_shinfo(nskb);
frags = shinfo->frags; frags = shinfo->frags;
for (shinfo->nr_frags = 0; shinfo->nr_frags < frag_overflow; for (shinfo->nr_frags = 0; shinfo->nr_frags < nr_slots;
shinfo->nr_frags++, txp++, gop++) { shinfo->nr_frags++, txp++, gop++) {
index = pending_index(queue->pending_cons++); index = pending_index(queue->pending_cons++);
pending_idx = queue->pending_ring[index]; pending_idx = queue->pending_ring[index];
...@@ -492,6 +492,11 @@ static void xenvif_get_requests(struct xenvif_queue *queue, ...@@ -492,6 +492,11 @@ static void xenvif_get_requests(struct xenvif_queue *queue,
} }
skb_shinfo(skb)->frag_list = nskb; skb_shinfo(skb)->frag_list = nskb;
} else if (nskb) {
/* A frag_list skb was allocated but it is no longer needed
* because enough slots were converted to copy ops above.
*/
kfree_skb(nskb);
} }
(*copy_ops) = cop - queue->tx_copy_ops; (*copy_ops) = cop - queue->tx_copy_ops;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment