[PATCH net v3] xsk: fix NULL pointer dereference in __xsk_rcv()

In the __xsk_rcv() multi-buffer path, xsk_buff_alloc() is called in a
loop without checking its return value. xsk_buff_can_alloc() only
counts fill queue entries without validating their addresses, so it
can succeed while xsk_buff_alloc() rejects all remaining entries and
returns NULL.

  Oops: general protection fault, probably for non-canonical address
   0xdffffc0000000000
  KASAN: null-ptr-deref in range
   [0x0000000000000000-0x0000000000000007]
  RIP: 0010:__xsk_rcv+0x426/0xc20 (net/xdp/xsk.c:350)
  Call Trace:
   xsk_generic_rcv+0x26d/0x5f0
   xdp_do_generic_redirect+0x3c5/0xcf0
   do_xdp_generic+0x92f/0xe70
   __netif_receive_skb_core.constprop.0+0xf7e/0x2b30

Fix this with a two-stage transaction. First allocate and stage all
buffers required for the packet using the pool's xskb_list, recycling
them with xsk_buff_free() if any allocation fails. Only after the
allocation stage succeeds, copy the data, reserve the RX descriptors,
and release the buffers in an error-free loop using
xsk_buff_get_frag().

Fixes: 804627751b42 ("xsk: add support for AF_XDP multi-buffer on Rx path")
Reported-by: [email protected]
Signed-off-by: Cen Zhang (Microsoft) <[email protected]>
---
v3:
 - Use pool's xskb_list and xsk_buff_get_frag() instead of a local
   staging list, per Maciej and Jason's review.
 - Set frags flag on head so xsk_buff_free() recycles the entire
   staged group in one call.
 - Guard against duplicate aligned Fill Ring addresses.
v2:
 - Allocate all packet buffers before reserving RX descriptors.
 - Recycle partially allocated buffers instead of only cancelling the
  RX producer reservations.
Link: https://lore.kernel.org/netdev/[email protected]

 net/xdp/xsk.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 7855ee09c4b6..02ff3ad97410 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -297,10 +297,11 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 {
 	u32 frame_size = __xsk_pool_get_rx_frame_size(xs->pool);
 	void *copy_from = xsk_copy_xdp_start(xdp), *copy_to;
+	struct xdp_buff *first, *next, *xsk_xdp;
 	u32 from_len, meta_len, rem, num_desc;
 	struct xdp_buff_xsk *xskb;
-	struct xdp_buff *xsk_xdp;
 	skb_frag_t *frag;
+	u32 i;
 
 	from_len = xdp->data_end - copy_from;
 	meta_len = xdp->data - copy_from;
@@ -343,11 +344,34 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 		frag =  &sinfo->frags[0];
 	}
 
+	if (WARN_ON_ONCE(!list_empty(&xs->pool->xskb_list)))
+		goto err_alloc;
+
+	first = xsk_buff_alloc(xs->pool);
+	if (!first)
+		goto err_alloc;
+
+	xdp_buff_set_frags_flag(first);
+	for (i = 1; i < num_desc; i++) {
+		xsk_xdp = xsk_buff_alloc(xs->pool);
+		if (!xsk_xdp)
+			goto err_free;
+
+		xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
+		if (unlikely(xsk_xdp == first ||
+			     !list_empty(&xskb->list_node)))
+			goto err_free;
+
+		list_add_tail(&xskb->list_node, &xs->pool->xskb_list);
+	}
+
+	xdp_buff_clear_frags_flag(first);
+	xsk_xdp = first;
 	do {
 		u32 to_len = frame_size + meta_len;
 		u32 copied;
 
-		xsk_xdp = xsk_buff_alloc(xs->pool);
+		next = xsk_buff_get_frag(xsk_xdp);
 		copy_to = xsk_xdp->data - meta_len;
 
 		copied = xsk_copy_xdp(copy_to, &copy_from, to_len, &from_len, &frag, rem);
@@ -356,10 +380,21 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 		xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
 		__xsk_rcv_zc_safe(xs, xskb, copied - meta_len,
 				  rem ? XDP_PKT_CONTD : 0);
+		xsk_xdp = next;
 		meta_len = 0;
 	} while (rem);
 
 	return 0;
+
+err_free:
+	/* Re-set frags flag; xsk_buff_alloc() may have cleared first->flags
+	 * if the same Fill Ring address aliased back to first.
+	 */
+	xdp_buff_set_frags_flag(first);
+	xsk_buff_free(first);
+err_alloc:
+	xs->rx_dropped++;
+	return -ENOMEM;
 }
 
 static bool xsk_tx_writeable(struct xdp_sock *xs)
-- 
2.53.0