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. Assisted-by: LLM Signed-off-by: Josef Bacik --- 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 a6821ab13969..ffc78b1a8ab8 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); @@ -6233,8 +6243,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; } -- 2.55.0