Netdev List
 help / color / mirror / Atom feed
From: patchwork-bot+netdevbpf@kernel.org
To: Josef Bacik <josef@toxicpanda.com>
Cc: davem@davemloft.net, edumazet@kernel.org, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, andrew+netdev@lunn.ch,
	jgross@suse.com, sstabellini@kernel.org,
	willemdebruijn.kernel@gmail.com, fmancera@suse.de,
	almasrymina@google.com, david.laight.linux@gmail.com,
	netdev@vger.kernel.org, xen-devel@lists.xenproject.org,
	willemb@google.com
Subject: Re: [PATCH net-next v4 00/15] net: skbuff: follow-ups to the BUG_ON() removal
Date: Wed, 11 Nov 2026 22:00:14 +0000	[thread overview]
Message-ID: <179443441492.1663612.10817269880170067726.git-patchwork-notify@kernel.org> (raw)
In-Reply-To: <20261104-b4-skbuff-bug-on-followups-v4-0-b7ffeb364f5d@toxicpanda.com>

Hello:

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

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@toxicpanda.com/
> v2: https://lore.kernel.org/all/20261021-b4-skbuff-bug-on-followups-v2-0-2aa780a03524@toxicpanda.com/
> v3: https://lore.kernel.org/all/20261103-b4-skbuff-bug-on-followups-v3-0-54ad0ab57614@toxicpanda.com/
> 
> 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



      parent reply	other threads:[~2026-11-11 22:00 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04 20:47 [PATCH net-next v4 00/15] net: skbuff: follow-ups to the BUG_ON() removal Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 01/15] net: skbuff: make skb_copy_and_csum_bits() report unreadable frags Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 02/15] net: skbuff: add skb_checksum_full() that reports failure Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 03/15] net: skbuff: fail checksum validation when the skb can't be checksummed Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 04/15] net: skbuff: fail segmentation when a segment " Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 05/15] net: skbuff: drop hw csum if trim can't checksum tail Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 06/15] net: skbuff: add skb_crc32c_full() that reports failure Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 07/15] net: skbuff: let skb_zerocopy() copy a short skb without head_frag Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 08/15] net: skbuff: make skb_try_coalesce()'s len unsigned Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 09/15] net: skbuff: reject data_len > len in copy_and_csum_dev Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 10/15] net: skbuff: reject a negative length in skb_copy_and_csum_bits() Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 11/15] net: skbuff: don't run off frag_list in skb_segment() Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 12/15] net: test the skb layouts skb_segment() refuses Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 13/15] xen-netfront: don't BUG() when skb_copy_bits() fails in bounce_skb() Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 14/15] net/tls: don't BUG() on leftover length in __skb_nsg() Josef Bacik
2026-11-04 20:47 ` [PATCH net-next v4 15/15] net: update comments that still describe the skb BUG_ON()s Josef Bacik
2026-11-11 22:00 ` patchwork-bot+netdevbpf [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=179443441492.1663612.10817269880170067726.git-patchwork-notify@kernel.org \
    --to=patchwork-bot+netdevbpf@kernel.org \
    --cc=almasrymina@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=david.laight.linux@gmail.com \
    --cc=edumazet@kernel.org \
    --cc=fmancera@suse.de \
    --cc=horms@kernel.org \
    --cc=jgross@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sstabellini@kernel.org \
    --cc=willemb@google.com \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox