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

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

[Cen Zhang (Microsoft)]

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, recycling all staged buffers with
xsk_buff_free() if any allocation fails. Only after this stage
succeeds, copy the data, reserve the RX descriptors, and release the
buffers in an error-free loop.

Fixes: 804627751b42 ("xsk: add support for AF_XDP multi-buffer on Rx path")
Reported-by: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
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/20260724164719.99563-1-blbllhy@gmail.com

 net/xdp/xsk.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index f906d51b6699..383fc2b1de48 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -298,9 +298,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;
 	u32 from_len, meta_len, rem, num_desc;
-	struct xdp_buff_xsk *xskb;
+	struct xdp_buff_xsk *xskb, *tmp;
 	struct xdp_buff *xsk_xdp;
+	LIST_HEAD(xsk_buffs);
 	skb_frag_t *frag;
+	u32 i;
 
 	from_len = xdp->data_end - copy_from;
 	meta_len = xdp->data - copy_from;
@@ -343,23 +345,44 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 		frag =  &sinfo->frags[0];
 	}
 
+	for (i = 0; i < num_desc; i++) {
+		xsk_xdp = xsk_buff_alloc(xs->pool);
+		if (!xsk_xdp)
+			goto err_alloc;
+
+		xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
+		if (unlikely(!list_empty(&xskb->list_node)))
+			goto err_alloc;
+		list_add_tail(&xskb->list_node, &xsk_buffs);
+	}
+
 	do {
 		u32 to_len = frame_size + meta_len;
 		u32 copied;
 
-		xsk_xdp = xsk_buff_alloc(xs->pool);
+		xskb = list_first_entry(&xsk_buffs, struct xdp_buff_xsk,
+					list_node);
+		list_del_init(&xskb->list_node);
+		xsk_xdp = &xskb->xdp;
 		copy_to = xsk_xdp->data - meta_len;
 
 		copied = xsk_copy_xdp(copy_to, &copy_from, to_len, &from_len, &frag, rem);
 		rem -= copied;
 
-		xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
 		__xsk_rcv_zc_safe(xs, xskb, copied - meta_len,
 				  rem ? XDP_PKT_CONTD : 0);
 		meta_len = 0;
 	} while (rem);
 
 	return 0;
+
+err_alloc:
+	list_for_each_entry_safe(xskb, tmp, &xsk_buffs, list_node) {
+		list_del_init(&xskb->list_node);
+		xsk_buff_free(&xskb->xdp);
+	}
+	xs->rx_dropped++;
+	return -ENOMEM;
 }
 
 static bool xsk_tx_writeable(struct xdp_sock *xs)
-- 
2.53.0

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

[Jason Xing]

On Sat, Jul 25, 2026 at 11:43 AM Cen Zhang (Microsoft)
<blbllhy@gmail.com> wrote:
>
> 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, recycling all staged buffers with
> xsk_buff_free() if any allocation fails. Only after this stage
> succeeds, copy the data, reserve the RX descriptors, and release the
> buffers in an error-free loop.
>
> Fixes: 804627751b42 ("xsk: add support for AF_XDP multi-buffer on Rx path")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>

You sent two patches within 24 hour, which obeys the basic rule of the
netdev community.

Please wait.

Thanks,
Jason


> ---
> 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/20260724164719.99563-1-blbllhy@gmail.com
>
>  net/xdp/xsk.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f906d51b6699..383fc2b1de48 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -298,9 +298,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;
>         u32 from_len, meta_len, rem, num_desc;
> -       struct xdp_buff_xsk *xskb;
> +       struct xdp_buff_xsk *xskb, *tmp;
>         struct xdp_buff *xsk_xdp;
> +       LIST_HEAD(xsk_buffs);
>         skb_frag_t *frag;
> +       u32 i;
>
>         from_len = xdp->data_end - copy_from;
>         meta_len = xdp->data - copy_from;
> @@ -343,23 +345,44 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>                 frag =  &sinfo->frags[0];
>         }
>
> +       for (i = 0; i < num_desc; i++) {
> +               xsk_xdp = xsk_buff_alloc(xs->pool);
> +               if (!xsk_xdp)
> +                       goto err_alloc;
> +
> +               xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
> +               if (unlikely(!list_empty(&xskb->list_node)))
> +                       goto err_alloc;
> +               list_add_tail(&xskb->list_node, &xsk_buffs);
> +       }
> +
>         do {
>                 u32 to_len = frame_size + meta_len;
>                 u32 copied;
>
> -               xsk_xdp = xsk_buff_alloc(xs->pool);
> +               xskb = list_first_entry(&xsk_buffs, struct xdp_buff_xsk,
> +                                       list_node);
> +               list_del_init(&xskb->list_node);
> +               xsk_xdp = &xskb->xdp;
>                 copy_to = xsk_xdp->data - meta_len;
>
>                 copied = xsk_copy_xdp(copy_to, &copy_from, to_len, &from_len, &frag, rem);
>                 rem -= copied;
>
> -               xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
>                 __xsk_rcv_zc_safe(xs, xskb, copied - meta_len,
>                                   rem ? XDP_PKT_CONTD : 0);
>                 meta_len = 0;
>         } while (rem);
>
>         return 0;
> +
> +err_alloc:
> +       list_for_each_entry_safe(xskb, tmp, &xsk_buffs, list_node) {
> +               list_del_init(&xskb->list_node);
> +               xsk_buff_free(&xskb->xdp);
> +       }
> +       xs->rx_dropped++;
> +       return -ENOMEM;
>  }
>
>  static bool xsk_tx_writeable(struct xdp_sock *xs)
> --
> 2.53.0
>
>

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

[Maciej Fijalkowski]

On Fri, Jul 24, 2026 at 11:42:46PM -0400, Cen Zhang (Microsoft) wrote:
> 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, recycling all staged buffers with
> xsk_buff_free() if any allocation fails. Only after this stage
> succeeds, copy the data, reserve the RX descriptors, and release the
> buffers in an error-free loop.
> 
> Fixes: 804627751b42 ("xsk: add support for AF_XDP multi-buffer on Rx path")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> ---
> 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/20260724164719.99563-1-blbllhy@gmail.com
> 
>  net/xdp/xsk.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f906d51b6699..383fc2b1de48 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -298,9 +298,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;
>  	u32 from_len, meta_len, rem, num_desc;
> -	struct xdp_buff_xsk *xskb;
> +	struct xdp_buff_xsk *xskb, *tmp;
>  	struct xdp_buff *xsk_xdp;
> +	LIST_HEAD(xsk_buffs);
>  	skb_frag_t *frag;
> +	u32 i;
>  
>  	from_len = xdp->data_end - copy_from;
>  	meta_len = xdp->data - copy_from;
> @@ -343,23 +345,44 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>  		frag =  &sinfo->frags[0];
>  	}
>  
> +	for (i = 0; i < num_desc; i++) {
> +		xsk_xdp = xsk_buff_alloc(xs->pool);
> +		if (!xsk_xdp)
> +			goto err_alloc;
> +
> +		xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
> +		if (unlikely(!list_empty(&xskb->list_node)))
> +			goto err_alloc;
> +		list_add_tail(&xskb->list_node, &xsk_buffs);

could we use existing xsk_buff_add_frag() ?
then I presume xsk_buff_free() would understand list and walk through
xdp_buff's and free it ?

I believe we could reuse pool's xskb_list instead of fabricating the
on-stack variant here.

> +	}
> +
>  	do {
>  		u32 to_len = frame_size + meta_len;
>  		u32 copied;
>  
> -		xsk_xdp = xsk_buff_alloc(xs->pool);
> +		xskb = list_first_entry(&xsk_buffs, struct xdp_buff_xsk,
> +					list_node);
> +		list_del_init(&xskb->list_node);
> +		xsk_xdp = &xskb->xdp;
>  		copy_to = xsk_xdp->data - meta_len;
>  
>  		copied = xsk_copy_xdp(copy_to, &copy_from, to_len, &from_len, &frag, rem);
>  		rem -= copied;
>  
> -		xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
>  		__xsk_rcv_zc_safe(xs, xskb, copied - meta_len,
>  				  rem ? XDP_PKT_CONTD : 0);
>  		meta_len = 0;
>  	} while (rem);
>  
>  	return 0;
> +
> +err_alloc:
> +	list_for_each_entry_safe(xskb, tmp, &xsk_buffs, list_node) {
> +		list_del_init(&xskb->list_node);
> +		xsk_buff_free(&xskb->xdp);
> +	}
> +	xs->rx_dropped++;
> +	return -ENOMEM;
>  }
>  
>  static bool xsk_tx_writeable(struct xdp_sock *xs)
> -- 
> 2.53.0
> 

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

[Jason Xing]

Hi Maciej,

On Mon, Jul 27, 2026 at 8:09 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Fri, Jul 24, 2026 at 11:42:46PM -0400, Cen Zhang (Microsoft) wrote:
> > 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, recycling all staged buffers with
> > xsk_buff_free() if any allocation fails. Only after this stage
> > succeeds, copy the data, reserve the RX descriptors, and release the
> > buffers in an error-free loop.
> >
> > Fixes: 804627751b42 ("xsk: add support for AF_XDP multi-buffer on Rx path")
> > Reported-by: AutonomousCodeSecurity@microsoft.com
> > Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>

Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

> > ---
> > 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/20260724164719.99563-1-blbllhy@gmail.com
> >
> >  net/xdp/xsk.c | 29 ++++++++++++++++++++++++++---
> >  1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index f906d51b6699..383fc2b1de48 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -298,9 +298,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;
> >       u32 from_len, meta_len, rem, num_desc;
> > -     struct xdp_buff_xsk *xskb;
> > +     struct xdp_buff_xsk *xskb, *tmp;
> >       struct xdp_buff *xsk_xdp;
> > +     LIST_HEAD(xsk_buffs);
> >       skb_frag_t *frag;
> > +     u32 i;
> >
> >       from_len = xdp->data_end - copy_from;
> >       meta_len = xdp->data - copy_from;
> > @@ -343,23 +345,44 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
> >               frag =  &sinfo->frags[0];
> >       }
> >
> > +     for (i = 0; i < num_desc; i++) {
> > +             xsk_xdp = xsk_buff_alloc(xs->pool);
> > +             if (!xsk_xdp)
> > +                     goto err_alloc;
> > +
> > +             xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
> > +             if (unlikely(!list_empty(&xskb->list_node)))
> > +                     goto err_alloc;
> > +             list_add_tail(&xskb->list_node, &xsk_buffs);
>
> could we use existing xsk_buff_add_frag() ?
> then I presume xsk_buff_free() would understand list and walk through
> xdp_buff's and free it ?

Good suggestion, but xsk_buff_add_frag() will add more irrelevant
stuff like nr_frags, xdp_frags_size, xdp_frags_truesize... They are
all happening in the softirq context. Refactoring the helper would
bring more work here.

Honestly, I like the local array which seems simpler/cleaner to understand :)

Thanks,
Jason

>
> I believe we could reuse pool's xskb_list instead of fabricating the
> on-stack variant here.
>
> > +     }
> > +
> >       do {
> >               u32 to_len = frame_size + meta_len;
> >               u32 copied;
> >
> > -             xsk_xdp = xsk_buff_alloc(xs->pool);
> > +             xskb = list_first_entry(&xsk_buffs, struct xdp_buff_xsk,
> > +                                     list_node);
> > +             list_del_init(&xskb->list_node);
> > +             xsk_xdp = &xskb->xdp;
> >               copy_to = xsk_xdp->data - meta_len;
> >
> >               copied = xsk_copy_xdp(copy_to, &copy_from, to_len, &from_len, &frag, rem);
> >               rem -= copied;
> >
> > -             xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
> >               __xsk_rcv_zc_safe(xs, xskb, copied - meta_len,
> >                                 rem ? XDP_PKT_CONTD : 0);
> >               meta_len = 0;
> >       } while (rem);
> >
> >       return 0;
> > +
> > +err_alloc:
> > +     list_for_each_entry_safe(xskb, tmp, &xsk_buffs, list_node) {
> > +             list_del_init(&xskb->list_node);
> > +             xsk_buff_free(&xskb->xdp);
> > +     }
> > +     xs->rx_dropped++;
> > +     return -ENOMEM;
> >  }
> >
> >  static bool xsk_tx_writeable(struct xdp_sock *xs)
> > --
> > 2.53.0
> >
>

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

[Jason Xing]

On Sat, Jul 25, 2026 at 11:43 AM Cen Zhang (Microsoft)
<blbllhy@gmail.com> wrote:
>
> 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, recycling all staged buffers with
> xsk_buff_free() if any allocation fails. Only after this stage
> succeeds, copy the data, reserve the RX descriptors, and release the
> buffers in an error-free loop.
>
> Fixes: 804627751b42 ("xsk: add support for AF_XDP multi-buffer on Rx path")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> ---
> 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/20260724164719.99563-1-blbllhy@gmail.com
>
>  net/xdp/xsk.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f906d51b6699..383fc2b1de48 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -298,9 +298,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;
>         u32 from_len, meta_len, rem, num_desc;
> -       struct xdp_buff_xsk *xskb;
> +       struct xdp_buff_xsk *xskb, *tmp;
>         struct xdp_buff *xsk_xdp;
> +       LIST_HEAD(xsk_buffs);
>         skb_frag_t *frag;
> +       u32 i;
>
>         from_len = xdp->data_end - copy_from;
>         meta_len = xdp->data - copy_from;
> @@ -343,23 +345,44 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>                 frag =  &sinfo->frags[0];
>         }
>
> +       for (i = 0; i < num_desc; i++) {
> +               xsk_xdp = xsk_buff_alloc(xs->pool);
> +               if (!xsk_xdp)
> +                       goto err_alloc;
> +
> +               xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
> +               if (unlikely(!list_empty(&xskb->list_node)))
> +                       goto err_alloc;

It will cause a memory leak because the current xsk_xdp that is not
added to the local list will miss the chance to get freed? And the
empty list_node cannot be easily freed by xp_free()...

Thanks,
Jason

> +               list_add_tail(&xskb->list_node, &xsk_buffs);
> +       }
> +
>         do {
>                 u32 to_len = frame_size + meta_len;
>                 u32 copied;
>
> -               xsk_xdp = xsk_buff_alloc(xs->pool);
> +               xskb = list_first_entry(&xsk_buffs, struct xdp_buff_xsk,
> +                                       list_node);
> +               list_del_init(&xskb->list_node);
> +               xsk_xdp = &xskb->xdp;
>                 copy_to = xsk_xdp->data - meta_len;
>
>                 copied = xsk_copy_xdp(copy_to, &copy_from, to_len, &from_len, &frag, rem);
>                 rem -= copied;
>
> -               xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
>                 __xsk_rcv_zc_safe(xs, xskb, copied - meta_len,
>                                   rem ? XDP_PKT_CONTD : 0);
>                 meta_len = 0;
>         } while (rem);
>
>         return 0;
> +
> +err_alloc:
> +       list_for_each_entry_safe(xskb, tmp, &xsk_buffs, list_node) {
> +               list_del_init(&xskb->list_node);
> +               xsk_buff_free(&xskb->xdp);
> +       }
> +       xs->rx_dropped++;
> +       return -ENOMEM;
>  }
>
>  static bool xsk_tx_writeable(struct xdp_sock *xs)
> --
> 2.53.0
>
>

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

[Maciej Fijalkowski]

On Tue, Jul 28, 2026 at 08:07:48AM +0800, Jason Xing wrote:
> Hi Maciej,
> 
> On Mon, Jul 27, 2026 at 8:09 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Fri, Jul 24, 2026 at 11:42:46PM -0400, Cen Zhang (Microsoft) wrote:
> > > 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, recycling all staged buffers with
> > > xsk_buff_free() if any allocation fails. Only after this stage
> > > succeeds, copy the data, reserve the RX descriptors, and release the
> > > buffers in an error-free loop.
> > >
> > > Fixes: 804627751b42 ("xsk: add support for AF_XDP multi-buffer on Rx path")
> > > Reported-by: AutonomousCodeSecurity@microsoft.com
> > > Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> 
> Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
> 
> > > ---
> > > 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/20260724164719.99563-1-blbllhy@gmail.com
> > >
> > >  net/xdp/xsk.c | 29 ++++++++++++++++++++++++++---
> > >  1 file changed, 26 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > > index f906d51b6699..383fc2b1de48 100644
> > > --- a/net/xdp/xsk.c
> > > +++ b/net/xdp/xsk.c
> > > @@ -298,9 +298,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;
> > >       u32 from_len, meta_len, rem, num_desc;
> > > -     struct xdp_buff_xsk *xskb;
> > > +     struct xdp_buff_xsk *xskb, *tmp;
> > >       struct xdp_buff *xsk_xdp;
> > > +     LIST_HEAD(xsk_buffs);
> > >       skb_frag_t *frag;
> > > +     u32 i;
> > >
> > >       from_len = xdp->data_end - copy_from;
> > >       meta_len = xdp->data - copy_from;
> > > @@ -343,23 +345,44 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
> > >               frag =  &sinfo->frags[0];
> > >       }
> > >
> > > +     for (i = 0; i < num_desc; i++) {
> > > +             xsk_xdp = xsk_buff_alloc(xs->pool);
> > > +             if (!xsk_xdp)
> > > +                     goto err_alloc;
> > > +
> > > +             xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
> > > +             if (unlikely(!list_empty(&xskb->list_node)))
> > > +                     goto err_alloc;
> > > +             list_add_tail(&xskb->list_node, &xsk_buffs);
> >
> > could we use existing xsk_buff_add_frag() ?
> > then I presume xsk_buff_free() would understand list and walk through
> > xdp_buff's and free it ?
> 
> Good suggestion, but xsk_buff_add_frag() will add more irrelevant
> stuff like nr_frags, xdp_frags_size, xdp_frags_truesize... They are
> all happening in the softirq context. Refactoring the helper would
> bring more work here.

Fair enough, how about we meet in the halfway?

Don't use xxx_add_frag but reuse pool's list. Reason I'm pushing for it is
xsk_buff_free() has been thought to consume multi-buffer xskb's, so all
the list-walking would be hidden and error path would only consist of a
single free() call.

then the do/while loop while iterating could be using xsk_buff_add_frag()

> 
> Honestly, I like the local array which seems simpler/cleaner to understand :)
> 
> Thanks,
> Jason
> 
> >
> > I believe we could reuse pool's xskb_list instead of fabricating the
> > on-stack variant here.
> >
> > > +     }
> > > +
> > >       do {
> > >               u32 to_len = frame_size + meta_len;
> > >               u32 copied;
> > >
> > > -             xsk_xdp = xsk_buff_alloc(xs->pool);
> > > +             xskb = list_first_entry(&xsk_buffs, struct xdp_buff_xsk,
> > > +                                     list_node);
> > > +             list_del_init(&xskb->list_node);
> > > +             xsk_xdp = &xskb->xdp;
> > >               copy_to = xsk_xdp->data - meta_len;
> > >
> > >               copied = xsk_copy_xdp(copy_to, &copy_from, to_len, &from_len, &frag, rem);
> > >               rem -= copied;
> > >
> > > -             xskb = container_of(xsk_xdp, struct xdp_buff_xsk, xdp);
> > >               __xsk_rcv_zc_safe(xs, xskb, copied - meta_len,
> > >                                 rem ? XDP_PKT_CONTD : 0);
> > >               meta_len = 0;
> > >       } while (rem);
> > >
> > >       return 0;
> > > +
> > > +err_alloc:
> > > +     list_for_each_entry_safe(xskb, tmp, &xsk_buffs, list_node) {
> > > +             list_del_init(&xskb->list_node);
> > > +             xsk_buff_free(&xskb->xdp);
> > > +     }
> > > +     xs->rx_dropped++;
> > > +     return -ENOMEM;
> > >  }
> > >
> > >  static bool xsk_tx_writeable(struct xdp_sock *xs)
> > > --
> > > 2.53.0
> > >
> >