Re: [PATCH net] net: fix use-after-free in pskb_expand_head()

pskb_expand_head() and pskb_carve_inside_header() decide whether to drop
the old head gracefully via skb_release_data() or directly free it via
skb_free_head().

Blamed commit replaced the check against dataref == delta with skb_cloned(skb).

However, skb_cloned(skb) masks out the upper 16 bits (SKB_DATAREF_SHIFT)
of dataref using SKB_DATAREF_MASK. For an skb with nohdr == 1 or a clone
referencing a shared buffer where only one regular reference exists,
skb_cloned(skb) evaluates to false even when dataref is shared with other
skbs.

When pskb_expand_head() is called on such a buffer (e.g. during bridge
flooding, generic XDP, or VLAN tag insertion), it falsely assumes the head
is unshared and calls skb_free_head(skb) directly. This drops the underlying
head/frag reference immediately, while other clones or references still
point to it, causing a use-after-free in subsequent packet processing.

Introduce skb_dataref_bias() and skb_data_is_shared() to accurately test
if dataref is shared by checking atomic_read(&skb_shinfo(skb)->dataref) != bias.

Use skb_dataref_bias() in skb_data_unref() and skb_data_is_shared() in
pskb_expand_head() and pskb_carve_inside_header().

Fixes: 3e24591a19bb ("skb: Drop "fastpath" variable for skb_cloned check in pskb_expand_head")
Reported-by: syzbot+22c4f9a7026c86bcc3b8@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6a954e27.4d659fcc.734b4.0046.GAE@google.com/ (local)
Reported-by: syzbot+128e9f5a0f85a51215b1@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=128e9f5a0f85a51215b1
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/skbuff.h | 13 ++++++++++++-
 net/core/skbuff.c      |  4 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 421f6fc454511ea6c7b83d5227481840a7c4bcc0..efeef148e1ba50cbb0d480975dc6c3c59acb98c7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h

@@ -1295,6 +1295,11 @@ static inline bool skb_unref(struct sk_buff *skb)
 	return true;
 }
 
+static inline int skb_dataref_bias(const struct sk_buff *skb)
+{
+	return skb->nohdr ? (1 << SKB_DATAREF_SHIFT) + 1 : 1;
+}
+
 static inline bool skb_data_unref(const struct sk_buff *skb,
 				  struct skb_shared_info *shinfo)
 {

@@ -1303,7 +1308,7 @@ static inline bool skb_data_unref(const struct sk_buff *skb,
 	if (!skb->cloned)
 		return true;
 
-	bias = skb->nohdr ? (1 << SKB_DATAREF_SHIFT) + 1 : 1;
+	bias = skb_dataref_bias(skb);
 
 	if (atomic_read(&shinfo->dataref) == bias)
 		smp_rmb();

@@ -2019,6 +2024,12 @@ static inline struct sk_buff *skb_get(struct sk_buff *skb)
  * If users == 1, we are the only owner and can avoid redundant atomic changes.
  */
 
+static inline bool skb_data_is_shared(const struct sk_buff *skb)
+{
+	return skb->cloned &&
+	       atomic_read(&skb_shinfo(skb)->dataref) != skb_dataref_bias(skb);
+}
+
 /**
  *	skb_cloned - is the buffer a clone
  *	@skb: buffer to check

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 966af3beed94d0a9f0387cec70b5297eccfcf19f..f4c0645f98d3f2f0c8765403e0184769e928d7b0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c

@@ -2331,7 +2331,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 	 * is not we can just drop the old head and let the existing refcount
 	 * be since all we did is relocate the values
 	 */
-	if (skb_cloned(skb)) {
+	if (skb_data_is_shared(skb)) {