Re: [PATCH net-next] net: rename skb_unshare() to skb_unclone_full()
From: Nicolai Buchwitz
Date: Thu Oct 15 2026 - 05:56:30 EST
Patchwork claims that b4 aut missed Ido and Kuniyu in the list of recipients:
+TO both
On 15.10.2026 10:16, Nicolai Buchwitz wrote:
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 <kuba@xxxxxxxxxx>
Signed-off-by: Nicolai Buchwitz <nb@xxxxxxxxxxx>
---
Follow-up to Jakub's suggestion on the selftest shared-skb series:
https://lore.kernel.org/all/20261014130618.4c50f817@xxxxxxxxxx/
---
drivers/net/ethernet/amd/xgbe/xgbe-selftest.c | 2 +-
drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 4 ++--
drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 4 ++--
drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +-
drivers/net/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 409fd855235b..b714052e1a37 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -639,7 +639,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);
@@ -948,7 +948,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);
@@ -1154,7 +1154,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 27ec1e38c828..cd969dd73841 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2125,14 +2125,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
*
@@ -2142,10 +2142,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 30f4afa18bc8..a1c9c0d75da1 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;
---
base-commit: 0c20cb4cfb40cf270d6c4c2c2ff5e5d72322644a
change-id: 20261015-nb-skb-unclone-full-dafb147b8aa9
Best regards,
--
Nicolai Buchwitz <nb@xxxxxxxxxxx>