netdev - Re: [PATCH 2/2 bpf v2] xsk: avoid data corruption on cq descriptor number

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoD4es0WLbZRF+OhrfrL3XrkDw1rnr80bBxFJn9zZBJWGg@mail.gmail.com>
Date: Thu, 30 Oct 2025 09:05:31 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Fernando Fernandez Mancera <fmancera@...e.de>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, magnus.karlsson@...el.com, 
	maciej.fijalkowski@...el.com, sdf@...ichev.me, fw@...len.de
Subject: Re: [PATCH 2/2 bpf v2] xsk: avoid data corruption on cq descriptor number

On Wed, Oct 29, 2025 at 2:30 AM Fernando Fernandez Mancera
<fmancera@...e.de> 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.
>
>  BUG: kernel NULL pointer dereference, address: 0000000000000000
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: Oops: 0000 [#1] SMP NOPTI
>  CPU: 2 UID: 1 PID: 927 Comm: p4xsk.bin Not tainted 6.16.12+deb14-cloud-amd64 #1 PREEMPT(lazy)  Debian 6.16.12-1
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
>  RIP: 0010:xsk_destruct_skb+0xd0/0x180
>  [...]
>  Call Trace:
>   <IRQ>
>   ? napi_complete_done+0x7a/0x1a0
>   ip_rcv_core+0x1bb/0x340
>   ip_rcv+0x30/0x1f0
>   __netif_receive_skb_one_core+0x85/0xa0
>   process_backlog+0x87/0x130
>   __napi_poll+0x28/0x180
>   net_rx_action+0x339/0x420
>   handle_softirqs+0xdc/0x320
>   ? handle_edge_irq+0x90/0x1e0
>   do_softirq.part.0+0x3b/0x60
>   </IRQ>
>   <TASK>
>   __local_bh_enable_ip+0x60/0x70
>   __dev_direct_xmit+0x14e/0x1f0
>   __xsk_generic_xmit+0x482/0xb70
>   ? __remove_hrtimer+0x41/0xa0
>   ? __xsk_generic_xmit+0x51/0xb70
>   ? _raw_spin_unlock_irqrestore+0xe/0x40
>   xsk_sendmsg+0xda/0x1c0
>   __sys_sendto+0x1ee/0x200
>   __x64_sys_sendto+0x24/0x30
>   do_syscall_64+0x84/0x2f0
>   ? __pfx_pollwake+0x10/0x10
>   ? __rseq_handle_notify_resume+0xad/0x4c0
>   ? restore_fpregs_from_fpstate+0x3c/0x90
>   ? switch_fpu_return+0x5b/0xe0
>   ? do_syscall_64+0x204/0x2f0
>   ? do_syscall_64+0x204/0x2f0
>   ? do_syscall_64+0x204/0x2f0
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>   </TASK>
>  [...]
>  Kernel panic - not syncing: Fatal exception in interrupt
>  Kernel Offset: 0x1c000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
>
> Use the skb XDP extension to store the number of cq descriptors along
> with a list of umem addresses.
>
> Fixes: 30f241fcf52a ("xsk: Fix immature cq descriptor production")

It might be off topic a bit. I keep wondering if we can find another
solution to fix the issue that the above patches tries to fix: how to
avoid the wrong fallback in that corner case. It's unlikely to happen
but the cost of correcting it is high because of adding memory
allocation in the hot path and freeing memory when the irq is disabled
in the irq context... Sure, for the latter, we can implement a
workqueue to handle this to mitigate the impact. Here I have no
intention of expanding another different topic.

Adding additional memory allocation or something like that is really
not a promising direction we should take. In high performance features
like copy mode in xsk, the biggest challenge is memory operations and
the second one is various locks.

Thanks,
Jason