Re: [PATCH net v6] xsk: avoid data corruption on cq descriptor number: manual merge

[Matthieu Baerts <matttbe@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1813 bytes --]

Hello,

On 24/11/2025 18:14, Fernando Fernandez Mancera wrote:
> Since commit 30f241fcf52a ("xsk: Fix immature cq descriptor
> production"), the descriptor number is stored in skb control block and
> xsk_cq_submit_addr_locked() relies on it to put the umem addrs onto
> pool's completion queue.
> 
> skb control block shouldn't be used for this purpose as after transmit
> xsk doesn't have control over it and other subsystems could use it. This
> leads to the following kernel panic due to a NULL pointer dereference.

FYI, and as predicted by Jason, we got a small conflict when merging
'net' in 'net-next' in the MPTCP tree due to this patch applied in 'net':

  0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")

and this one from 'net-next':

  8da7bea7db69 ("xsk: add indirect call for xsk_destruct_skb")

----- Generic Message -----
The best is to avoid conflicts between 'net' and 'net-next' trees but if
they cannot be avoided when preparing patches, a note about how to fix
them is much appreciated.

The conflict has been resolved on our side [1] and the resolution we
suggest is attached to this email. Please report any issues linked to
this conflict resolution as it might be used by others. If you worked on
the mentioned patches, don't hesitate to ACK this conflict resolution.
---------------------------

Regarding this conflict, the patch from 'net' removed two functions
above one that has been applied in 'net-next'. I then combined the two
modifications by removing the two functions and keeping the new
attributes set to xsk_destruct_skb().

Rerere cache is available in [2].

Cheers,
Matt

1: https://github.com/multipath-tcp/mptcp_net-next/commit/1caa6b15d784
2: https://github.com/multipath-tcp/mptcp-upstream-rr-cache/commit/265e1
-- 
Sponsored by the NGI0 Core fund.

[-- Attachment #2: 1caa6b15d784769d23637d9c5dae18ddc4bd8b39.patch --]
[-- Type: text/x-patch, Size: 2275 bytes --]

diff --cc net/xdp/xsk.c
index bcfd400e9cf8,69bbcca8ac75..f093c3453f64
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@@ -560,50 -591,43 +590,42 @@@ static u32 xsk_get_num_desc(struct sk_b
  static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool,
  				      struct sk_buff *skb)
  {
- 	struct xsk_addr_node *pos, *tmp;
+ 	u32 num_descs = xsk_get_num_desc(skb);
+ 	struct xsk_addrs *xsk_addr;
  	u32 descs_processed = 0;
  	unsigned long flags;
- 	u32 idx;
+ 	u32 idx, i;
  
 -	spin_lock_irqsave(&pool->cq_lock, flags);
 +	spin_lock_irqsave(&pool->cq_prod_lock, flags);
  	idx = xskq_get_prod(pool->cq);
  
- 	xskq_prod_write_addr(pool->cq, idx,
- 			     (u64)(uintptr_t)skb_shinfo(skb)->destructor_arg);
- 	descs_processed++;
+ 	if (unlikely(num_descs > 1)) {
+ 		xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
  
- 	if (unlikely(XSKCB(skb)->num_descs > 1)) {
- 		list_for_each_entry_safe(pos, tmp, &XSKCB(skb)->addrs_list, addr_node) {
+ 		for (i = 0; i < num_descs; i++) {
  			xskq_prod_write_addr(pool->cq, idx + descs_processed,
- 					     pos->addr);
+ 					     xsk_addr->addrs[i]);
  			descs_processed++;
- 			list_del(&pos->addr_node);
- 			kmem_cache_free(xsk_tx_generic_cache, pos);
  		}
+ 		kmem_cache_free(xsk_tx_generic_cache, xsk_addr);
+ 	} else {
+ 		xskq_prod_write_addr(pool->cq, idx,
+ 				     xsk_skb_destructor_get_addr(skb));
+ 		descs_processed++;
  	}
  	xskq_prod_submit_n(pool->cq, descs_processed);
 -	spin_unlock_irqrestore(&pool->cq_lock, flags);
 +	spin_unlock_irqrestore(&pool->cq_prod_lock, flags);
  }
  
  static void xsk_cq_cancel_locked(struct xsk_buff_pool *pool, u32 n)
  {
 -	unsigned long flags;
 -
 -	spin_lock_irqsave(&pool->cq_lock, flags);
 +	spin_lock(&pool->cq_cached_prod_lock);
  	xskq_prod_cancel_n(pool->cq, n);
 -	spin_unlock_irqrestore(&pool->cq_lock, flags);
 +	spin_unlock(&pool->cq_cached_prod_lock);
  }
  
- static void xsk_inc_num_desc(struct sk_buff *skb)
- {
- 	XSKCB(skb)->num_descs++;
- }
- 
- static u32 xsk_get_num_desc(struct sk_buff *skb)
- {
- 	return XSKCB(skb)->num_descs;
- }
- 
 -static void xsk_destruct_skb(struct sk_buff *skb)
 +INDIRECT_CALLABLE_SCOPE
 +void xsk_destruct_skb(struct sk_buff *skb)
  {
  	struct xsk_tx_metadata_compl *compl = &skb_shinfo(skb)->xsk_meta;