From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C24DC4F55DB for ; Wed, 30 Sep 2026 15:09:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790780998; cv=none; b=rOxKbbv3SkDP+hPYZocl0OK40avD2hKGV3e2T1c9xgFd82SRM8AC7/gcnsxjbdGZDIxvEx87Hf1EOyKP8tUwBp3mlAvf0kCQWl9+mjbptwQT17+PH4a7y3hEr9/CfgrLn0fg2YL+OL2DdDu4hxm0xRxmaLOrfms5WTUjjqPN6qo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790780998; c=relaxed/simple; bh=ES2+Afyteb5RyAXC0lUEN8w6RDLL3SSifc1YEgBhgBM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=lvoyZnMPAr3aNW9O4BAacEqe8h+0/K/K7BTnTtWV8brtGMMXwQzpHsL88lRbOGzhub18AphPqygMB3KvmAqyt8oyN7rxS11qn36tq3BNln0ydAmGaMZBJLaw4UnCG3EijdAF9YScBiHty+SqFRn+Sr4bx3d7PLpEigjy2AmL4CA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=CVdVg4xN; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="CVdVg4xN" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4CF101F0089A; Wed, 30 Sep 2026 15:09:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790780991; bh=7tbZXRPF1SCWGOOoyhdUb79fWjo0uS2PPGlqOIc2eD0=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=CVdVg4xNIq4mrCf5aTfHivZXv8Vp7QOH1vviwB50C4I78G6a2jPLThSdLBriLeQYm SUAySXaektLYMzDkCjismPRLLqg0iutB5NZksiGcHotmYZenxvJHb2GpfIDfJCpZS7 FzKICHGmxqaB5pTtLsrdwTI9B3hwt+Qgj+KKFxqCmuYJbCt89UsUk3ykpe/GyjPzAd /QFnqZFay3uRFV7KOPHbJ7IuWRPnXTWPpbKHOoyHlVPAsCdhs4q4adk3CqmtFCEuZi 6aGQTQ4YG0BYHp7fU3J6XCBDMxQ98tAua3CEyzIdGmCs/gkoV+GUjUcQ2Eb26JteYJ JkGWbfDxGQc2A== From: Sasha Levin To: stable@vger.kernel.org Cc: Willem de Bruijn , Katherine Leaver , Bjoern Doebel , Jakub Kicinski , Sasha Levin Subject: [PATCH 5.15.y 2/2] packet: use ubuf_info completion for TX_RING packets Date: Wed, 30 Sep 2026 11:09:47 -0400 Message-ID: <20260930150947.2353841-2-sashal@kernel.org> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260930150947.2353841-1-sashal@kernel.org> References: <2026092952-contents-unworn-2736@gregkh> <20260930150947.2353841-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: Willem de Bruijn [ Upstream commit 9518405613863d0bf0700927a21367f5942cf058 ] tpacket_snd sends skbs with frags pointing into its ring slots. Slots are released when skb->destructor is called. A call to skb_orphan calls skb->destructor before the skb is freed. This can cause the slot to be reused while still linked into the skb. Switch to standard zerocopy completion (ubuf_info) so the slot is only released once all references to the payload are freed or copied. Restore skb->destructor to standard sock_wfree. The ubuf_info completion callback can be called with a NULL skb, but only from net_zcopy_put and related API, used by zerocopy implementations that hold their own reference on the uarg, such as MSG_ZEROCOPY. This uarg is only ever completed from skb_zcopy_clear, so skb is always set. To prevent userspace from aliasing in-flight state on shared ring slots, allocate tpacket_uarg per packet, rather than per slot. This adds a small allocation to the transmit path. Use standard kmalloc to allow backporting to stable kernels. The uarg holds an sk_wmem_alloc reference, rather than an sk_refcnt reference. packet_free_tx_ring waits on sk_wmem_alloc before freeing the ring pages. Always allocate vec->deferred for tx_ring so page-backed rings also wait on sk_wmem_alloc when skb_copy_ubufs drops page refs before calling tpacket_ubuf_complete. Drop the tx_ring.pg_vec test that tpacket_destruct_skb performed before accessing the slot. The sk_wmem_alloc reference now guarantees that the slot is valid. The test is also not sufficient by itself, as it reads pg_vec without pg_vec_lock, so it can race with packet_set_ring. As a result a slot is released when its payload is copied, which can be before transmission (e.g., in skb_orphan_frags_rx). If copied before skb_tx_timestamp() is called, no slot timestamp is recorded, similar to when skb_orphan() was called early in the datapath before this patch. Revert the now unused previous skb_zcopy_.._nouarg infra. Depends on commit 992cc9f94ca9 ("net/packet: defer vmalloc TX_RING free until skbs finish"). Reported-by: Katherine Leaver Reported-by: Bjoern Doebel Closes: https://lore.kernel.org/netdev/20260909085542.3370986-1-doebel@amazon.de/ Fixes: 5cd8d46ea156 ("packet: copy user buffers before orphan or clone") Cc: stable@vger.kernel.org Signed-off-by: Willem de Bruijn Link: https://patch.msgid.link/20260919004748.1463985-3-willemdebruijn.kernel@gmail.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- include/linux/skbuff.h | 19 +------- net/packet/af_packet.c | 102 ++++++++++++++++++++++++++--------------- 2 files changed, 65 insertions(+), 56 deletions(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index f712cff74b7a1..6f1d76d26c122 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1549,22 +1549,6 @@ static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg, } } -static inline void skb_zcopy_set_nouarg(struct sk_buff *skb, void *val) -{ - skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t) val | 0x1UL); - skb_shinfo(skb)->flags |= SKBFL_ZEROCOPY_FRAG; -} - -static inline bool skb_zcopy_is_nouarg(struct sk_buff *skb) -{ - return (uintptr_t) skb_shinfo(skb)->destructor_arg & 0x1UL; -} - -static inline void *skb_zcopy_get_nouarg(struct sk_buff *skb) -{ - return (void *)((uintptr_t) skb_shinfo(skb)->destructor_arg & ~0x1UL); -} - static inline void net_zcopy_put(struct ubuf_info *uarg) { if (uarg) @@ -1587,8 +1571,7 @@ static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy_success) struct ubuf_info *uarg = skb_zcopy(skb); if (uarg) { - if (!skb_zcopy_is_nouarg(skb)) - uarg->ops->complete(skb, uarg, zerocopy_success); + uarg->ops->complete(skb, uarg, zerocopy_success); skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY; } diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index a39f2e64e554c..85b0191a6f0f6 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2549,26 +2549,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, goto drop_n_restore; } -static void tpacket_destruct_skb(struct sk_buff *skb) -{ - struct packet_sock *po = pkt_sk(skb->sk); - - if (likely(po->tx_ring.pg_vec)) { - void *ph; - __u32 ts; - - ph = skb_zcopy_get_nouarg(skb); - - ts = __packet_set_timestamp(po, ph, skb); - __packet_set_status(po, ph, TP_STATUS_AVAILABLE | ts); - - packet_dec_pending(&po->tx_ring); - complete(&po->skb_completion); - } - - sock_wfree(skb); -} - static int __packet_snd_vnet_parse(struct virtio_net_hdr *vnet_hdr, size_t len) { if ((vnet_hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && @@ -2598,27 +2578,56 @@ static int packet_snd_vnet_parse(struct msghdr *msg, size_t *len, return __packet_snd_vnet_parse(vnet_hdr, *len); } +struct tpacket_uarg { + struct ubuf_info ubuf; + struct packet_sock *po; + void *ph; +}; + +static void tpacket_ubuf_complete(struct sk_buff *skb, struct ubuf_info *uarg, + bool success) +{ + struct tpacket_uarg *tu = container_of(uarg, struct tpacket_uarg, ubuf); + struct packet_sock *po = tu->po; + void *ph = tu->ph; + __u32 ts; + + DEBUG_NET_WARN_ON_ONCE(!skb); + + if (!refcount_dec_and_test(&uarg->refcnt)) + return; + + ts = __packet_set_timestamp(po, ph, skb); + __packet_set_status(po, ph, TP_STATUS_AVAILABLE | ts); + + packet_dec_pending(&po->tx_ring); + complete(&po->skb_completion); + + kfree(tu); + sk_free(&po->sk); +} + +static const struct ubuf_info_ops tpacket_ubuf_ops = { + .complete = tpacket_ubuf_complete, +}; + static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, - void *frame, struct net_device *dev, void *data, int tp_len, + struct net_device *dev, void *data, int tp_len, __be16 proto, unsigned char *addr, int hlen, int copylen, int hard_header_len, const struct sockcm_cookie *sockc) { - union tpacket_uhdr ph; int to_write, offset, len, nr_frags, len_max; struct socket *sock = po->sk.sk_socket; struct page *page; int err; - ph.raw = frame; - skb->protocol = proto; skb->dev = dev; skb->priority = po->sk.sk_priority; skb->mark = po->sk.sk_mark; skb->tstamp = sockc->transmit_time; skb_setup_tx_timestamp(skb, sockc); - skb_zcopy_set_nouarg(skb, ph.raw); skb_reserve(skb, hlen); skb_reset_network_header(skb); @@ -2757,6 +2766,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) struct virtio_net_hdr vnet_hdr; bool has_vnet_hdr = false; struct sockcm_cookie sockc; + struct tpacket_uarg *uarg; __be16 proto; int err, reserve = 0; void *ph; @@ -2883,7 +2893,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) err = len_sum; goto out_status; } - tp_len = tpacket_fill_skb(po, skb, ph, dev, data, tp_len, proto, + tp_len = tpacket_fill_skb(po, skb, dev, data, tp_len, proto, addr, hlen, copylen, hard_header_len, &sockc); if (likely(tp_len >= 0) && @@ -2915,7 +2925,24 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) virtio_net_hdr_set_proto(skb, &vnet_hdr); } - skb->destructor = tpacket_destruct_skb; + uarg = kmalloc(sizeof(*uarg), GFP_KERNEL); + if (unlikely(!uarg)) { + if (likely(len_sum > 0)) + err = len_sum; + else + err = -ENOMEM; + goto out_status; + } + uarg->po = po; + uarg->ph = ph; + uarg->ubuf.ops = &tpacket_ubuf_ops; + uarg->ubuf.flags = SKBFL_ZEROCOPY_FRAG; + refcount_set(&uarg->ubuf.refcnt, 1); + + /* Hold a sk_wmem_alloc reference until completion */ + refcount_inc(&po->sk.sk_wmem_alloc); + skb_zcopy_init(skb, &uarg->ubuf); + __packet_set_status(po, ph, TP_STATUS_SENDING); packet_inc_pending(&po->tx_ring); @@ -4471,21 +4498,20 @@ static struct pgv *alloc_pg_vec(struct tpacket_req *req, int order, bool tx_ring vec->len = block_nr; pg_vec = vec->pg_vec; + if (tx_ring) { + vec->deferred = kzalloc_obj(*vec->deferred, + GFP_KERNEL | __GFP_NOWARN); + if (!vec->deferred) + goto out_free_pgvec; + vec->deferred->vec = vec; + INIT_DELAYED_WORK(&vec->deferred->work, + packet_free_pg_vec_work); + } + for (i = 0; i < block_nr; i++) { pg_vec[i].buffer = alloc_one_pg_vec_page(order); if (unlikely(!pg_vec[i].buffer)) goto out_free_pgvec; - - if (tx_ring && !vec->deferred && - is_vmalloc_addr(pg_vec[i].buffer)) { - vec->deferred = kzalloc_obj(*vec->deferred, - GFP_KERNEL | __GFP_NOWARN); - if (!vec->deferred) - goto out_free_pgvec; - vec->deferred->vec = vec; - INIT_DELAYED_WORK(&vec->deferred->work, - packet_free_pg_vec_work); - } } out: -- 2.53.0