From c7d8bc3dd511dac309754543d3063f3950b364c2 Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:00 +0000 Subject: [PATCH 1/8] net: skbuff: don't BUG() on bad arguments to pskb_expand_head() pskb_expand_head() BUG()s if it is handed a negative nhead or a shared skb. Both are bugs in the caller, and both keep getting hit: commit 2cbb259ec4f8 ("bpf: Reject negative head_room in __bpf_skb_change_head") and commit 64e6a754d33d ("llc: do not use skb_get() before dev_queue_xmit()") each fixed a crash here that took down the whole box. The function already returns an error that every caller has to handle, and at this point it hasn't touched the skb. Return -EINVAL instead of crashing. The warning is DEBUG_NET_WARN_ON_ONCE(), so debug kernels still point at the caller, while production kernels, including ones running with panic_on_warn, just fail the call. Signed-off-by: Josef Bacik Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-1-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index f0cf117f3566..089f76d96bdd 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -2303,9 +2303,15 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail, u8 *data; int i; - BUG_ON(nhead < 0); + if (unlikely(nhead < 0)) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EINVAL; + } - BUG_ON(skb_shared(skb)); + if (unlikely(skb_shared(skb))) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EINVAL; + } skb_zcopy_downgrade_managed(skb); From 7b32bee7db821e6b4d92d29708c24539e2b5464f Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:01 +0000 Subject: [PATCH 2/8] net: skbuff: don't BUG() on a bad frag_list layout in skb_segment() skb_segment()'s frag_list walk assumes the GRO-shaped layout it expects and BUG()s when the layout doesn't match. Anybody who can get a malformed GSO skb to a segmentation point gets to crash the box. Most recently commit d5dc1e69fd72 ("inet: frags: strip GSO state from fragments before reassembly") fixed one that an unprivileged user could trigger with two writes to a tap device in their own user namespace. commit 3382a1ed7f77 ("net: fix udp gso skb_segment after pull from frag_list") fixed another. skb_segment() already has an error path for a bad layout: the too-many-frags check sets -EINVAL and frees the partial segment list. Take that path for the four layout checks, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels. The packet gets dropped, which is what should happen to a packet we can't segment. The one check that runs after skb_clone() and before the clone is linked into the segment list frees the clone itself. Signed-off-by: Josef Bacik Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-2-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 089f76d96bdd..a577217513e8 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4924,7 +4924,11 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, if (hsize <= 0 && i >= nfrags && skb_headlen(list_skb) && (skb_headlen(list_skb) == len || sg)) { - BUG_ON(skb_headlen(list_skb) > len); + if (unlikely(skb_headlen(list_skb) > len)) { + DEBUG_NET_WARN_ON_ONCE(1); + err = -EINVAL; + goto err; + } nskb = skb_clone(list_skb, GFP_ATOMIC); if (unlikely(!nskb)) @@ -4937,7 +4941,12 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, pos += skb_headlen(list_skb); while (pos < offset + len) { - BUG_ON(i >= nfrags); + if (unlikely(i >= nfrags)) { + DEBUG_NET_WARN_ON_ONCE(1); + kfree_skb(nskb); + err = -EINVAL; + goto err; + } size = skb_frag_size(frag); if (pos + size > offset + len) @@ -5044,9 +5053,17 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, skb_shinfo(nskb)->flags |= skb_shinfo(frag_skb)->flags & SKBFL_SHARED_FRAG; if (!skb_headlen(list_skb)) { - BUG_ON(!nfrags); + if (unlikely(!nfrags)) { + DEBUG_NET_WARN_ON_ONCE(1); + err = -EINVAL; + goto err; + } } else { - BUG_ON(!list_skb->head_frag); + if (unlikely(!list_skb->head_frag)) { + DEBUG_NET_WARN_ON_ONCE(1); + err = -EINVAL; + goto err; + } /* to make room for head_frag. */ i--; From 279877d86c8294c7d2f7a51ca755de79659af215 Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:02 +0000 Subject: [PATCH 3/8] net: skbuff: don't BUG() when skb_copy_bits() fails in copy helpers skb_copy(), skb_copy_expand() and skb_try_coalesce() all BUG() if skb_copy_bits() fails. skb_copy_bits() only fails when the skb's lengths don't add up, which is a bug somewhere else, usually in a driver building the skb. Each of these functions already has a failure return its callers handle: - skb_copy() and skb_copy_expand() free the new skb and return NULL, as they do when the allocation fails. - skb_try_coalesce() returns false and the caller keeps the skbs separate. Copy into the tailroom before skb_put() so that @to is untouched on failure. Take those returns, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels. __pskb_pull_tail() has the same BUG_ON(), but several of its callers can't otherwise fail and don't check its return, so it's left for a separate change that fixes them first. Signed-off-by: Josef Bacik Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-3-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index a577217513e8..136d7269c1da 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -2201,7 +2201,12 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t gfp_mask) /* Set the tail pointer and length */ skb_put(n, skb->len); - BUG_ON(skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len)); + if (unlikely(skb_copy_bits(skb, -headerlen, n->head, + headerlen + skb->len))) { + DEBUG_NET_WARN_ON_ONCE(1); + kfree_skb(n); + return NULL; + } skb_copy_header(n, skb); return n; @@ -2545,8 +2550,13 @@ struct sk_buff *skb_copy_expand(const struct sk_buff *skb, head_copy_off = newheadroom - head_copy_len; /* Copy the linear header and data. */ - BUG_ON(skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off, - skb->len + head_copy_len)); + if (unlikely(skb_copy_bits(skb, -head_copy_len, + n->head + head_copy_off, + skb->len + head_copy_len))) { + DEBUG_NET_WARN_ON_ONCE(1); + kfree_skb(n); + return NULL; + } skb_copy_header(n, skb); @@ -6245,8 +6255,15 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, return false; if (len <= skb_tailroom(to) && skb_frags_readable(from)) { - if (len) - BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len)); + if (len) { + if (unlikely(skb_copy_bits(from, 0, + skb_tail_pointer(to), + len))) { + DEBUG_NET_WARN_ON_ONCE(1); + return false; + } + skb_put(to, len); + } *delta_truesize = 0; return true; } From 20ddd0fd2fcd1c7bb9bac9eb8d88e795a37de74c Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:03 +0000 Subject: [PATCH 4/8] net: skbuff: don't BUG() on leftover length in skb_checksum() and friends skb_checksum(), skb_crc32c() and __skb_to_sgvec() walk the head, frags and frag_list and BUG() if they run out of skb before they run out of @len. By then nothing has been read past the end of the skb. The walk stopped at the end of the data, and the check only tells us the caller asked for a range the skb doesn't have. commit 06a0afcfe2f5 ("xfrm: do pskb_pull properly in __xfrm_transport_prep") fixed one such caller that crashed in __skb_to_sgvec(). Return what each function already returns when it can't do the work, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels: - __skb_to_sgvec() returns -EINVAL. Every skb_to_sgvec() caller has checked for a negative return since it learned to return -EMSGSIZE. - skb_checksum() and skb_crc32c() return 0, as they already do for unreadable frags. The resulting checksum is wrong, so the packet fails verification on receive or goes out with a bad checksum on transmit, rather than taking the machine down. Signed-off-by: Josef Bacik Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-4-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 136d7269c1da..536e0bb934f2 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -3619,7 +3619,10 @@ __wsum skb_checksum(const struct sk_buff *skb, int offset, int len, __wsum csum) } start = end; } - BUG_ON(len); + if (unlikely(len)) { + DEBUG_NET_WARN_ON_ONCE(1); + return 0; + } return csum; } @@ -3784,7 +3787,10 @@ u32 skb_crc32c(const struct sk_buff *skb, int offset, int len, u32 crc) } start = end; } - BUG_ON(len); + if (unlikely(len)) { + DEBUG_NET_WARN_ON_ONCE(1); + return 0; + } return crc; } @@ -5347,7 +5353,10 @@ __skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len, } start = end; } - BUG_ON(len); + if (unlikely(len)) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EINVAL; + } return elt; } From 56453199d28f0072a8196fba14b2d702632e2219 Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:04 +0000 Subject: [PATCH 5/8] net: skbuff: don't BUG() on a bad csum_start in skb_copy_and_csum_dev() skb_copy_and_csum_dev() copies everything up to the checksum start out of the linear area, and BUG()s if the checksum start is past the end of the linear area. It misses the other direction: a CHECKSUM_PARTIAL skb that has been pulled past its csum_start gives a negative offset, which gets past the check and becomes a ~4GB copy. It also trusts csum_offset when it stores the folded checksum. So far skb_copy_and_csum_bits() BUG()ing on a short skb has covered for that, but once it returns instead, a bad csum_offset would write past the end of the caller's buffer. The function returns void and its callers are drivers copying a frame into a bounce buffer just before handing it to the hardware. There's nothing for them to back out of, so check both ends of csum_start and that the checksum field fits in the frame, and if not, copy the whole frame with skb_copy_bits() without filling in the checksum, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels. The frame goes out with a bad checksum and is dropped by the receiver. If even that copy fails, zero the buffer so the driver doesn't send stale bytes. Signed-off-by: Josef Bacik Cc: stable? The trailers only have Assisted-by: and Signed-off-by:. No Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-5-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 536e0bb934f2..7f5eecfeddd7 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -3981,7 +3981,18 @@ void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to) else csstart = skb_headlen(skb); - BUG_ON(csstart > skb_headlen(skb)); + if (unlikely(csstart < 0 || csstart > skb_headlen(skb) || + (skb->ip_summed == CHECKSUM_PARTIAL && + csstart + skb->csum_offset + sizeof(__sum16) > + skb->len))) { + DEBUG_NET_WARN_ON_ONCE(1); + /* Send the frame without the checksum filled in, or send + * zeroes if we can't even copy it. + */ + if (skb_copy_bits(skb, 0, to, skb->len)) + memset(to, 0, skb->len); + return; + } skb_copy_from_linear_data(skb, to, csstart); From 417261779eab5e2be1cfe60e1018ba9612a410e0 Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:05 +0000 Subject: [PATCH 6/8] net: skbuff: don't BUG() on leftover length in skb_copy_and_csum_bits() skb_copy_and_csum_bits() has the same check as skb_checksum(): it BUG()s if it runs out of skb before it runs out of @len. This one gets hit: commit 7d63b6712538 ("icmp: guard against too small mtu") and commit f99cd56230f5 ("net: Remove acked SYN flag from packet in the transmit queue correctly") each fixed a crash here from icmp_glue_bits(). It can't just return, though. Its callers copy into a buffer that is about to go out on the wire, an ICMP error quoting the offending packet for example, so bailing out early would send whatever was left in the rest of that buffer. Zero the part of the buffer we didn't fill and return 0, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels. As with skb_checksum(), the checksum usually won't match the data, so the receiver will usually drop the packet, but either way it carries nothing it shouldn't. @len is an int, so only zero when it is positive; a negative @len from a broken caller must not turn into a huge memset(). Signed-off-by: Josef Bacik Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-6-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 7f5eecfeddd7..6947649ae9fa 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -3717,7 +3717,13 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset, } start = end; } - BUG_ON(len); + if (unlikely(len)) { + DEBUG_NET_WARN_ON_ONCE(1); + /* Don't hand the caller a buffer with stale bytes in it. */ + if (len > 0) + memset(to, 0, len); + return 0; + } return csum; } EXPORT_SYMBOL(skb_copy_and_csum_bits); From 433380f5ab8f1b6bd6f10284c77c8a8031f68f6e Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:06 +0000 Subject: [PATCH 7/8] net: skbuff: don't BUG() on a missing head_frag in skb_zerocopy() skb_zerocopy() BUG()s if @from has no head_frag and the caller passed hlen == 0, meaning the caller didn't ask for the head to be copied and the head can't be referenced as a page either. The check runs before anything is touched, and skb_zerocopy() already documents -EFAULT for bad skb geometry. Return that, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels. Signed-off-by: Josef Bacik Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-7-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 6947649ae9fa..c69c3d4cc8f4 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -3917,7 +3917,10 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen) struct page *page; unsigned int offset; - BUG_ON(!from->head_frag && !hlen); + if (unlikely(!from->head_frag && !hlen)) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EFAULT; + } /* dont bother with small payloads */ if (len <= skb_tailroom(to)) From e6974aad4bd7561ed2f1ee56acaee38e3d3c5d01 Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Wed, 7 Oct 2026 17:36:07 +0000 Subject: [PATCH 8/8] net: skbuff: remove the BUG_ON()s from skb_shift() skb_shift() has two BUG_ON()s. The first fires if the caller asks to shift more than @skb holds. Nothing has been touched yet, and returning 0 already means "shifted nothing", which the TCP callers handle by falling back. Return 0, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels. The second fires if the frags run out before @shiftlen does, but it only checks after the shift has been committed to both skbs, when there's nothing left to back out to. The loop that builds the new frag layout only writes @tgt's frag slots past its nr_frags, and the one branch that modifies @skb's frags also finishes the shift. So if the loop ends with bytes still left to shift, nothing visible has changed yet. That's the same state the MAX_SKB_FRAGS bail-out inside the loop returns 0 from. Move the check up to just before the commit and return 0 there the same way. Signed-off-by: Josef Bacik Link: https://patch.msgid.link/20261007-b4-skbuff-bug-on-v2-8-b9a5f732895b@toxicpanda.com Signed-off-by: Jakub Kicinski --- net/core/skbuff.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index c69c3d4cc8f4..93c2f440b9ec 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4335,7 +4335,10 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen) int from, to, merge, todo; skb_frag_t *fragfrom, *fragto; - BUG_ON(shiftlen > skb->len); + if (unlikely(shiftlen > skb->len)) { + DEBUG_NET_WARN_ON_ONCE(1); + return 0; + } if (skb_headlen(skb)) return 0; @@ -4415,6 +4418,14 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen) } } + /* The frags ran out before shiftlen did. Nothing has been committed + * yet, so back out. + */ + if (unlikely(todo > 0)) { + DEBUG_NET_WARN_ON_ONCE(1); + return 0; + } + /* Ready to "commit" this state change to tgt */ skb_shinfo(tgt)->nr_frags = to; @@ -4432,8 +4443,6 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen) skb_shinfo(skb)->frags[to++] = skb_shinfo(skb)->frags[from++]; skb_shinfo(skb)->nr_frags = to; - BUG_ON(todo > 0 && !skb_shinfo(skb)->nr_frags); - onlymerged: /* Most likely the tgt won't ever need its checksum anymore, skb on * the other hand might need it if it needs to be resent