From 258c3ae4a4af0c7948a3d99e7262701aaa350d31 Mon Sep 17 00:00:00 2001 From: Nicolai Buchwitz Date: Thu, 15 Oct 2026 10:16:38 +0200 Subject: [PATCH] net: rename skb_unshare() to skb_unclone_full() skb_unshare() only copies cloned skbs. A shared skb that is not a clone comes back unchanged, which the name doesn't suggest. The loopback selftest hooks got caught by this and hit BUG_ON(skb_shared()). Rename it to skb_unclone_full(). Like skb_unclone() it only acts on clones, but it copies the whole skb, not just the head. Suggested-by: Jakub Kicinski Signed-off-by: Nicolai Buchwitz Reviewed-by: Ioana Ciornei # dpaa2 Link: https://patch.msgid.link/20261015-nb-skb-unclone-full-v1-1-0dfbf01f60f2@tipi-net.de Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/amd/xgbe/xgbe-selftest.c | 2 +- drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 4 ++-- .../net/ethernet/freescale/dpaa2/dpaa2-switch.c | 4 ++-- .../ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +- .../ethernet/stmicro/stmmac/stmmac_selftests.c | 2 +- drivers/net/macsec.c | 6 +++--- include/linux/if_vlan.h | 16 ++++++++-------- include/linux/skbuff.h | 11 +++++++---- net/bluetooth/6lowpan.c | 2 +- net/dsa/tag.c | 2 +- net/ieee802154/6lowpan/rx.c | 2 +- net/ieee802154/6lowpan/tx.c | 2 +- net/mctp/route.c | 2 +- net/sched/sch_netem.c | 4 ++-- net/sctp/ulpqueue.c | 2 +- net/tipc/msg.c | 2 +- 16 files changed, 34 insertions(+), 31 deletions(-) diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c b/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c index cfdbd4639a1e..7ebae08ddf29 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c @@ -44,7 +44,7 @@ static int xgbe_test_loopback_validate(struct sk_buff *skb, struct iphdr *ih; int eat; - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) goto out; diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c index def2d5c0e7f7..7227a35dfaf2 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c @@ -1418,9 +1418,9 @@ static netdev_tx_t __dpaa2_eth_tx(struct sk_buff *skb, /* We'll be holding a back-reference to the skb until Tx Confirmation; * we don't want that overwritten by a concurrent Tx with a cloned skb. */ - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (unlikely(!skb)) { - /* skb_unshare() has already freed the skb */ + /* skb_unclone_full() has already freed the skb */ percpu_stats->tx_dropped++; return NETDEV_TX_OK; } diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c index 8cabce0eb2ab..fe8ac5e85259 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c @@ -1276,9 +1276,9 @@ static netdev_tx_t dpaa2_switch_port_tx(struct sk_buff *skb, } /* We'll be holding a back-reference to the skb until Tx confirmation */ - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (unlikely(!skb)) { - /* skb_unshare() has already freed the skb */ + /* skb_unclone_full() has already freed the skb */ net_err_ratelimited("%s: Error copying the socket buffer\n", net_dev->name); goto err_exit; } diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c index 77c5cfbe4ce4..4e418c7456b7 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c @@ -2117,7 +2117,7 @@ dma_addr_t otx2_dma_map_skb_frag(struct otx2_nic *pfvf, /* Crypto hardware need write permission for ipsec crypto offload */ if (unlikely(xfrm_offload(skb))) { dir = DMA_BIDIRECTIONAL; - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); } /* First segment is always skb->data */ diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c index dbe8339947be..40811df8f2b7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c @@ -920,7 +920,7 @@ static int stmmac_test_vlan_validate(struct sk_buff *skb, proto = tpriv->svlan ? ETH_P_8021AD : ETH_P_8021Q; - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) goto out; diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 4296884d5b4d..87f3f4bdb026 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -644,7 +644,7 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb, return ERR_PTR(-ENOMEM); } } else { - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) { macsec_txsa_put(tx_sa); return ERR_PTR(-ENOMEM); @@ -953,7 +953,7 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb, aead_request_set_crypt(req, sg, sg, len, iv); aead_request_set_ad(req, macsec_hdr_len(macsec_skb_cb(skb)->has_sci)); - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) { aead_request_free(req); return ERR_PTR(-ENOMEM); @@ -1159,7 +1159,7 @@ static rx_handler_result_t macsec_handle_frame(struct sk_buff **pskb) if (hdr->eth.h_proto != htons(ETH_P_MACSEC)) return handle_not_macsec(skb); - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); *pskb = skb; if (!skb) return RX_HANDLER_CONSUMED; diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h index 975892367136..c863cdf60f01 100644 --- a/include/linux/if_vlan.h +++ b/include/linux/if_vlan.h @@ -427,8 +427,8 @@ static inline int __vlan_insert_tag(struct sk_buff *skb, * Inserts the VLAN tag into @skb as part of the payload at offset mac_len * Returns a VLAN tagged skb. This might change skb->head. * - * Following the skb_unshare() example, in case of error, the calling function - * doesn't have to worry about freeing the original skb. + * Following the skb_unclone_full() example, in case of error, the calling + * function doesn't have to worry about freeing the original skb. * * Does not change skb->protocol so this function can be used during receive. * @@ -458,8 +458,8 @@ static inline struct sk_buff *vlan_insert_inner_tag(struct sk_buff *skb, * Inserts the VLAN tag into @skb as part of the payload * Returns a VLAN tagged skb. This might change skb->head. * - * Following the skb_unshare() example, in case of error, the calling function - * doesn't have to worry about freeing the original skb. + * Following the skb_unclone_full() example, in case of error, the calling + * function doesn't have to worry about freeing the original skb. * * Does not change skb->protocol so this function can be used during receive. * @@ -480,8 +480,8 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, * Inserts the VLAN tag into @skb as part of the payload * Returns a VLAN tagged skb. This might change skb->head. * - * Following the skb_unshare() example, in case of error, the calling function - * doesn't have to worry about freeing the original skb. + * Following the skb_unclone_full() example, in case of error, the calling + * function doesn't have to worry about freeing the original skb. * * Return: modified @skb on success, NULL on error (@skb is freed). */ @@ -524,8 +524,8 @@ static inline void __vlan_hwaccel_copy_tag(struct sk_buff *dst, const struct sk_ * * Pushes the VLAN tag from @skb->vlan_tci inside to the payload. * - * Following the skb_unshare() example, in case of error, the calling function - * doesn't have to worry about freeing the original skb. + * Following the skb_unclone_full() example, in case of error, the calling + * function doesn't have to worry about freeing the original skb. */ static inline struct sk_buff *__vlan_hwaccel_push_inside(struct sk_buff *skb) { diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index af1cbc2799b7..5079335f7aae 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -2131,14 +2131,14 @@ static inline struct sk_buff *skb_share_check(struct sk_buff *skb, gfp_t pri) } /* - * Copy shared buffers into a new sk_buff. We effectively do COW on + * Copy cloned buffers into a new sk_buff. We effectively do COW on * packets to handle cases where we have a local reader and forward * and a couple of other messy ones. The normal one is tcpdumping * a packet that's being forwarded. */ /** - * skb_unshare - make a copy of a shared buffer + * skb_unclone_full - make a private copy of a cloned buffer * @skb: buffer to check * @pri: priority for memory allocation * @@ -2148,10 +2148,13 @@ static inline struct sk_buff *skb_share_check(struct sk_buff *skb, gfp_t pri) * the original buffer is returned. When called with a spinlock held or * from interrupt state @pri must be %GFP_ATOMIC * + * Unlike skb_unclone(), the paged data is copied as well. A buffer with + * skb->users > 1 that is not a clone is returned as is, use + * skb_share_check() for that case. + * * %NULL is returned on a memory allocation failure. */ -static inline struct sk_buff *skb_unshare(struct sk_buff *skb, - gfp_t pri) +static inline struct sk_buff *skb_unclone_full(struct sk_buff *skb, gfp_t pri) { might_sleep_if(gfpflags_allow_blocking(pri)); if (skb_cloned(skb)) { diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index 9a736cefac18..8c51c20f6c4b 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -515,7 +515,7 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev) /* We must take a copy of the skb before we modify/replace the ipv6 * header as the header could be used elsewhere */ - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) return NET_XMIT_DROP; diff --git a/net/dsa/tag.c b/net/dsa/tag.c index 991732d6eae2..6a5aa7fc331c 100644 --- a/net/dsa/tag.c +++ b/net/dsa/tag.c @@ -64,7 +64,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev, return 0; } - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) return 0; diff --git a/net/ieee802154/6lowpan/rx.c b/net/ieee802154/6lowpan/rx.c index 517e6493f5d1..9148f1be7cba 100644 --- a/net/ieee802154/6lowpan/rx.c +++ b/net/ieee802154/6lowpan/rx.c @@ -294,7 +294,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *wdev, */ if (lowpan_is_frag1(*skb_network_header(skb)) || lowpan_is_iphc(*skb_network_header(skb))) { - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) goto out; } diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c index b261daedc290..d346e906e41e 100644 --- a/net/ieee802154/6lowpan/tx.c +++ b/net/ieee802154/6lowpan/tx.c @@ -279,7 +279,7 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev) return NET_XMIT_DROP; } } else { - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (!skb) return NET_XMIT_DROP; } diff --git a/net/mctp/route.c b/net/mctp/route.c index e7c95eeacb48..d18ed9d0db1d 100644 --- a/net/mctp/route.c +++ b/net/mctp/route.c @@ -396,7 +396,7 @@ static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb) * this will free the skb; callers can no longer access it * safely. */ - key->reasm_head = skb_unshare(skb, GFP_ATOMIC); + key->reasm_head = skb_unclone_full(skb, GFP_ATOMIC); if (!key->reasm_head) return -ENOMEM; diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c index 8a1b89a289d8..1dc2924890eb 100644 --- a/net/sched/sch_netem.c +++ b/net/sched/sch_netem.c @@ -460,7 +460,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) { struct netem_sched_data *q = qdisc_priv(sch); - /* We don't fill cb now as skb_unshare() may invalidate it */ + /* We don't fill cb now as skb_unclone_full() may invalidate it */ struct netem_skb_cb *cb; struct sk_buff *skb2 = NULL; struct sk_buff *segs = NULL; @@ -528,7 +528,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, qdisc_skb_cb(skb)->pkt_len = skb->len; } - skb = skb_unshare(skb, GFP_ATOMIC); + skb = skb_unclone_full(skb, GFP_ATOMIC); if (unlikely(!skb)) { WRITE_ONCE(q->allocation_errors, q->allocation_errors + 1); qdisc_qstats_drop(sch); diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c index b05daafd369a..93823eebd727 100644 --- a/net/sctp/ulpqueue.c +++ b/net/sctp/ulpqueue.c @@ -339,7 +339,7 @@ struct sctp_ulpevent *sctp_make_reassembled_event(struct net *net, if (skb_cloned(f_frag)) { /* This is a cloned skb, we can't just modify * the frag_list. We need a new skb to do that. - * Instead of calling skb_unshare(), we'll do it + * Instead of calling skb_unclone_full(), we'll do it * ourselves since we need to delay the free. */ new = skb_copy(f_frag, GFP_ATOMIC); diff --git a/net/tipc/msg.c b/net/tipc/msg.c index b0bba0feef56..39dfd33c8eec 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -145,7 +145,7 @@ int tipc_buf_append(struct sk_buff **headbuf, struct sk_buff **buf) if (skb_has_frag_list(frag) && __skb_linearize(frag)) goto err; *buf = NULL; - frag = skb_unshare(frag, GFP_ATOMIC); + frag = skb_unclone_full(frag, GFP_ATOMIC); if (unlikely(!frag)) goto err; head = *headbuf = frag;