Merge branch 'packet-fix-packet_tx_ring-data-corruption-on-skb_orphan'

Willem de Bruijn says:

====================
packet: fix PACKET_TX_RING data corruption on skb_orphan

When transmitting packets via PACKET_TX_RING, tpacket_snd links user
ring buffer pages as skb frags and releases the slot on skb->destructor
(tpacket_destruct_skb).

skb_orphan() invokes the destructor while the skb is still alive.
This marks the slot as TP_STATUS_AVAILABLE prematurely, allowing
userspace to overwrite the slot and causing data corruption.

This series fixes the issue by switching PACKET_TX_RING to standard
ubuf_info zerocopy completion, ensuring ring slots are released only
after all payload references are freed or copied.

Virtio-net needs a separate solution, because deferring the release
can cause deadlock in its !use_napi mode.

- Patch 1 addresses the virtio-net special case.
- Patch 2 converts tpacket_snd to standard ubuf_info completion

Patch 1 must be applied, and backported, before patch 2. Both carry
the same Fixes tag for that reason.

v1: https://lore.kernel.org/netdev/20260914214229.1674102-1-willemdebruijn.kernel@gmail.com/
====================

Link: https://patch.msgid.link/20260919004748.1463985-1-willemdebruijn.kernel@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2026-09-22 18:33:03 -07:00
commit 6836807c5f
3 changed files with 74 additions and 56 deletions

View File

@ -3349,6 +3349,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
else
virtqueue_disable_cb(sq->vq);
if (!use_napi &&
unlikely(skb_orphan_frags(skb, GFP_ATOMIC))) {
DEV_STATS_INC(dev, tx_dropped);
dev_kfree_skb_any(skb);
kick = !xmit_more || netif_xmit_stopped(txq);
goto kick_vq;
}
/* timestamp packet in software */
skb_tx_timestamp(skb);
@ -3381,6 +3389,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
kick = use_napi ? __netdev_tx_sent_queue(txq, skb->len, xmit_more) :
!xmit_more || netif_xmit_stopped(txq);
kick_vq:
if (kick) {
if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) {
u64_stats_update_begin(&sq->stats.syncp);

View File

@ -1834,22 +1834,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)
@ -1872,8 +1856,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;
}

View File

@ -2530,26 +2530,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) &&
@ -2589,27 +2569,56 @@ static int packet_snd_vnet_parse(struct msghdr *msg, size_t *len,
return 0;
}
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 = sockc->priority;
skb->mark = sockc->mark;
skb_set_delivery_type_by_clockid(skb, sockc->transmit_time, po->sk.sk_clockid);
skb_setup_tx_timestamp(skb, sockc);
skb_zcopy_set_nouarg(skb, ph.raw);
skb_reserve(skb, hlen);
skb_reset_network_header(skb);
@ -2749,6 +2758,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;
@ -2876,7 +2886,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) &&
@ -2908,7 +2918,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);
@ -4486,21 +4513,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: