[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH net-next v4 00/15] net: skbuff: follow-ups to the BUG_ON() removal



Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@xxxxxxxxxx>:

On Wed, 04 Nov 2026 20:47:12 +0000 you wrote:
> v1: 
> https://lore.kernel.org/all/20261019-b4-skbuff-bug-on-followups-v1-0-41dbef50b9f7@xxxxxxxxxxxxxx/
> v2: 
> https://lore.kernel.org/all/20261021-b4-skbuff-bug-on-followups-v2-0-2aa780a03524@xxxxxxxxxxxxxx/
> v3: 
> https://lore.kernel.org/all/20261103-b4-skbuff-bug-on-followups-v3-0-54ad0ab57614@xxxxxxxxxxxxxx/
> 
> v3->v4:
> - 1: the !sg && csum branch of skb_segment() returns -EFAULT instead of
>   -ENOMEM when skb_copy_bits() fails, like the branch next to it.  The
>   new cases in 12 made that stand out.  I kept Mina's Reviewed-by.
> - 12: run the nested and short skb cases without SG too (Willem).
> - 11: no code change.  The commit message was wrong that the path
>   without SG was fine before.  Three of the new cases in 12 hit the NULL
>   pointer dereference without SG if the list_skb check in the clone
>   branch condition is taken out, because the walk reads list_skb before
>   it looks at SG.
> - Picked up Mina's Reviewed-by on 1 and Acked-by on 4, Willem's
>   Reviewed-by on 11 and 12, and Juergen's Reviewed-by on 13.
> - No other changes, same base as v3.
> v2->v3:
> - 1, 2, 6: move the frag_list recursion into a noinline function, so
>   that taking the address of a local there doesn't give the helpers a
>   stack protector (Paolo).  On x86_64 defconfig plus CONFIG_IP_SCTP
>   none of skb_copy_and_csum_bits(), skb_checksum_full(),
>   skb_crc32c_full() or the two wrappers has one now, and
>   __ip_append_data(), __ip6_append_data() and skb_segment() are
>   unchanged.  The callers that pass a local do gain one.  Besides the
>   seven I listed on v2, that's __skb_gro_checksum_complete() and
>   skb_checksum_help(), which are converted in this version, and
>   skb_crc32c_csum_help().
> - 1: put the Fixes: tags back (Fernando, Sashiko).  csump in the
>   prototype, and drop the "Both of above in one bottle" comment
>   (Fernando).  skb_copy_and_csum_dev() zeroes the whole frame when the
>   copy fails, since leaving the checksum alone doesn't guarantee a bad
>   one (Sashiko).
> - 2, 6: the kernel-doc describes the unreadable check as it is, the
>   wrappers don't blame unreadable frags for -EINVAL, and the commit
>   messages say what changes for a failing frag_list member (Fernando,
>   Sashiko).
> - 2: convert skb_checksum_help() too, as 6 does for its crc32c twin, and
>   add Fixes: 20ddd0fd2fcd.
> - 3: convert __skb_gro_checksum_complete() too (Fernando, Sashiko).
>   Take the skb off CHECKSUM_COMPLETE on failure.  Otherwise conntrack,
>   which doesn't drop on a bad checksum, leaves the pseudo-header sum in
>   skb->csum and TCP's validation accepts the packet after all
>   (Sashiko).  Update the comments, and add Fixes: 65249feb6b3d.
> - 4: the commit message was wrong that devmem TX reaches this checksum.
>   Its skbs carry SKBFL_SHARED_FRAG, so the segment fails to linearize
>   first.  The patch is hardening, and the commit message says so now.
> - 5: when the tail can't be checksummed but what's left can be read,
>   set CHECKSUM_NONE instead of failing the trim, which dropped packets
>   that used to be accepted (Sashiko).  Retitled, and add Fixes: tags.
> - 7, 8: add the Fixes: tags for the original commits, and correct the
>   history in 8's commit message (Sashiko).  8 also says what it takes
>   to reach the memcpy().
> - 7: reject a negative @len or @hlen on entry.  The reorder left nothing
>   ahead of the skb_put(), which panics on one.
> - 8: the truesize check refuses the coalesce instead of only warning,
>   and runs before the head is stolen.  With len unsigned and nothing
>   else, an skb with a len above INT_MAX went from refused to coalesced.
> - 9: check data_len > len directly and zero the frame.  The old check
>   missed CHECKSUM_PARTIAL (Sashiko).  Retitled.
> - 10: drop both len > 0 tests, and correct what the commit message says
>   they covered (Sashiko).
> - 9, 10: drop the Fixes: tags.  Both problems are older than the commits
>   the tags named, and neither has a known trigger.
> - 11: Willem, this one changed again after you OK'd the walk up front.
>   A nested frag_list is only one way for the walk to run out of members.
>   A len that counts bytes the skb doesn't hold gets to the same NULL
>   pointer dereference, and the walk up front doesn't catch that.  So the
>   patch now checks for a NULL list_skb in the three places where the
>   walk moves on to it, and that's all it does: no pass up front, which
>   also takes care of the cost you pointed out for BIG TCP, and none of
>   v2's skb_has_frag_list() tests.  You asked on v2 whether the first two
>   checks were needed.  For a nested frag_list they weren't, for this
>   they are.  12 has a case for each, and taking any one of the three
>   checks out brings the oops back.  The path without SG is left alone
>   again.  The commit message says that IP reassembly does nest
>   frag_lists, just not on GSO skbs (Sashiko).  Retitled.
> - 12: add four cases for a len that counts bytes the skb doesn't hold.
>   Retitled.
> - 13: correct what the commit message says about when bounce_skb() runs
>   and when skb_copy_bits() fails.
> - 14: update the comment above skb_nsg() (Sashiko).
> - 15: one more stale comment, in net_test.c.
> - Picked up Fernando's Reviewed-by on 4.  Dropped Mina's on 1 and
>   Fernando's on 2, since the frag_list loop moved in both, and Willem's
>   on 12, since it grew.
> - Shortened the commit messages.
> - Rebased onto current net-next.
> v1->v2:
> - skb_segment(): check each frag_list member as the walk takes it,
>   instead of an extra pass over the frag_list up front (Willem).
> - KUnit test: braces around the suppressed skb_segment() call (Willem),
>   and the comment update for the non-uniform case moved into the same
>   patch (Mina).
> - Picked up Willem's Reviewed-by and Mina's Acked-bys.
> 
> [...]

Here is the summary with links:
  - [net-next,v4,01/15] net: skbuff: make skb_copy_and_csum_bits() report 
unreadable frags
    https://git.kernel.org/netdev/net-next/c/23fe6ac2e97a
  - [net-next,v4,02/15] net: skbuff: add skb_checksum_full() that reports 
failure
    https://git.kernel.org/netdev/net-next/c/2a1e46c16105
  - [net-next,v4,03/15] net: skbuff: fail checksum validation when the skb 
can't be checksummed
    https://git.kernel.org/netdev/net-next/c/3fe3f8beef7e
  - [net-next,v4,04/15] net: skbuff: fail segmentation when a segment can't be 
checksummed
    https://git.kernel.org/netdev/net-next/c/7fec569df355
  - [net-next,v4,05/15] net: skbuff: drop hw csum if trim can't checksum tail
    https://git.kernel.org/netdev/net-next/c/05938d3edbdb
  - [net-next,v4,06/15] net: skbuff: add skb_crc32c_full() that reports failure
    https://git.kernel.org/netdev/net-next/c/b8f3a11ff453
  - [net-next,v4,07/15] net: skbuff: let skb_zerocopy() copy a short skb 
without head_frag
    https://git.kernel.org/netdev/net-next/c/86cc1a8a942b
  - [net-next,v4,08/15] net: skbuff: make skb_try_coalesce()'s len unsigned
    https://git.kernel.org/netdev/net-next/c/b242a00c2bc3
  - [net-next,v4,09/15] net: skbuff: reject data_len > len in copy_and_csum_dev
    https://git.kernel.org/netdev/net-next/c/3133a780ef89
  - [net-next,v4,10/15] net: skbuff: reject a negative length in 
skb_copy_and_csum_bits()
    https://git.kernel.org/netdev/net-next/c/2173a320e20d
  - [net-next,v4,11/15] net: skbuff: don't run off frag_list in skb_segment()
    https://git.kernel.org/netdev/net-next/c/e447e46ce976
  - [net-next,v4,12/15] net: test the skb layouts skb_segment() refuses
    https://git.kernel.org/netdev/net-next/c/7d02a2b23764
  - [net-next,v4,13/15] xen-netfront: don't BUG() when skb_copy_bits() fails in 
bounce_skb()
    https://git.kernel.org/netdev/net-next/c/ece597f62085
  - [net-next,v4,14/15] net/tls: don't BUG() on leftover length in __skb_nsg()
    https://git.kernel.org/netdev/net-next/c/e021a72ee9cb
  - [net-next,v4,15/15] net: update comments that still describe the skb 
BUG_ON()s
    https://git.kernel.org/netdev/net-next/c/c0073ae6e9d3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html





 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.