Re: [PATCH net] xsk: reset the mac header in the generic Tx path
Eric Dumazet <[email protected]>
| Newsgroups | org.kernel.vger.bpf,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CANn89i+aacS2PfaMXnimzGgbCLz-JSnqo6QyoS122Gf+kxya3w@mail.gmail.com> |
On Mon, Aug 24, 2026 at 2:47 PM Jason Xing <[email protected]> wrote: > > On Fri, Aug 21, 2026 at 1:07 AM Eric Dumazet <[email protected]> wrote: > > > > On Mon, Aug 17, 2026 at 5:40 PM Petr Oros <[email protected]> wrote: > > > > > > __dev_queue_xmit() resets skb->mac_header, __dev_direct_xmit() does not, > > > so a frame taking the qdisc bypass path reaches ndo_start_xmit() with > > > the sentinel left by __finalize_skb_around() and skb_mac_header() points > > > 65535 bytes past skb->head. > > > > > > The packet socket reaches it through packet_direct_xmit(), and > > > packet_parse_headers() was taught to anchor the header by > > > commit c2707480cfbf ("net/packet: reset the MAC header on the > > > packet-socket transmit path"). AF_XDP reaches it through > > > __xsk_generic_xmit(), and net/xdp/xsk.c never sets the mac header. > > > > > > ice reads eth->h_proto through skb_mac_header() on its ordinary Tx path. > > > On an E810 every one of 2817224 AF_XDP frames reached the driver with the > > > sentinel still in place, so the ethertype never came from the frame. With > > > this patch all 5757920 frames of the same test carried the correct 0x88b5. > > > An AF_PACKET sender on the same port read 0x88b5 in both runs. > > > > > > On a KFENCE kernel that read lands inside the pool and gets reported as a > > > use after free of an unrelated object, 387 times in a 180 s run: > > > > > > BUG: KFENCE: use-after-free read in ice_xmit_frame_ring+0xddb/0x1650 [ice] > > > ice_xmit_frame_ring+0xddb/0x1650 [ice] > > > __dev_direct_xmit+0x347/0x4d0 > > > __xsk_generic_xmit+0xc13/0x1e70 > > > __xsk_sendmsg.constprop.0.isra.0+0x519/0x640 > > > xsk_sendmsg+0x6c/0x90 > > > > > > Rerunning the same reproducer on the same E810 with this patch applied > > > produced no reports and no bad reads. > > > > > > Anchor the header in xsk_skb_init_misc(), which runs once per skb for > > > both build paths. With IFF_TX_SKB_NO_LINEAR the offset still is not a > > > real header, but it stays in bounds and no such driver reads it. > > > > > > Fixes: 35fcde7f8deb ("xsk: support for Tx") > > > Signed-off-by: Petr Oros <[email protected]> > > > --- > > > net/xdp/xsk.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c > > > index 7855ee09c4b640..885427392cb6b3 100644 > > > --- a/net/xdp/xsk.c > > > +++ b/net/xdp/xsk.c > > > @@ -931,6 +931,7 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs, > > > skb->priority = READ_ONCE(xs->sk.sk_priority); > > > skb->mark = READ_ONCE(xs->sk.sk_mark); > > > skb->destructor = xsk_destruct_skb; > > > + skb_reset_mac_header(skb); > > > > I don't know, I had at some point the goal of removing our dependency > > on mac header in ndo_start_xmit(), > > as this must be skb->data. > > > > ice could use skb_eth_hdr() instead of eth_hdr() or skb_mac_header() > > and save few cycles. > > > > Some drivers call skb_reset_mac_header(skb), this is really a mess. > > > > With AI these days, it should not be too hard to completely remove our > > dependency on mac_header. > > > > I saw the status has been changed to 'rejected'. Why is that, I > wonder? I think it's just a fix that can be done in xsk like > skb_reset_network_header() that I'm going to add. > skb_reset_network_header() is orthogonal. Patching net/xdp/xsk.c (and follow-up network header resets) only covers one caller while leaving the core issue open. The discrepancy is that __dev_queue_xmit() unconditionally executes skb_reset_mac_header(skb), whereas __dev_direct_xmit() does not. As a result, other paths bypassing qdisc hit the exact same out-of-bounds read in ice. For example, AF_PACKET with PACKET_QDISC_BYPASS only resets mac_header in packet_parse_headers() under: if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) && sock->type == SOCK_RAW) { skb_reset_mac_header(skb); ... } Any PACKET_QDISC_BYPASS socket using SOCK_DGRAM or SOCK_RAW with an explicit protocol (e.g. ETH_P_IP) sends an skb with mac_header unset (0xffff sentinel) directly into ndo_start_xmit(). If the core stack was expected to guarantee mac_header for direct xmit, that reset belongs in __dev_direct_xmit(), not duplicated in xsk, packet, selftests, etc. Work in net-next would presumably audit / change all drivers to not rely on mac_header being set (This I can do when net-next reopens) Then we can remove skb_reset_mac_header(skb) from __dev_queue_xmit() and __dev_direct_xmit(), and from dpaa_tx() / dpaa_tx_extended()