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

Re: [PATCH net-next 04/15] net: skbuff: fail segmentation when a segment can't be checksummed



Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit fixes a real, user-visible TX bug but has no Fixes:
  tag (and no Cc: stable).

--

net: skbuff: fail segmentation when a segment can't be checksummed

This patch changes the sg && !csum software checksum path in
skb_segment() to use skb_checksum_full(). The segmentation now fails with
-EFAULT when a segment can't be checksummed. This covers devmem TCP skbs
whose payload is in unreadable frags: skb_checksum() used to warn and
return 0, so the TCP checksum didn't cover the payload.

Link: 
https://lore.kernel.org/all/20261009-b4-skb-segment-nosg-unreadable-v1-1-7e2334a55fe8@xxxxxxxxxxxxxx/

> Found by the Sashiko AI review of an earlier fix for the same path with
> SG off.
>
> Link: 
> https://lore.kernel.org/all/20261009-b4-skb-segment-nosg-unreadable-v1-1-7e2334a55fe8@xxxxxxxxxxxxxx/
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

[Severity: Low]
Should this have a Fixes: tag, and possibly Cc: stable?

Before this patch, the perform_csum_check branch of skb_segment() called
skb_checksum() on segments that share the devmem skb's net_iov frags.
zerocopy_fill_skb_from_devmem() in net/core/datagram.c never sets
SKBFL_SHARED_FRAG, so the __skb_linearize() fallback is skipped and
__skb_checksum() ends up here:

    if (WARN_ON_ONCE(!skb_frags_readable(skb)))
        return 0;

That WARN is fatal with panic_on_warn. tcp_gso_segment() then writes a
TCP checksum that doesn't cover the payload that goes out.

This path became reachable when unreadable-frag TX skbs were added by:

bd61848900bf ("net: devmem: Implement TX path")

That looks like the right Fixes: target. Another patch in this series,
"net: skbuff: fail checksum validation when the skb can't be checksummed",
does carry a Fixes: tag. Without one here, the stable tooling won't pick
up this fix.

This patch also calls skb_checksum_full(), which is added by the earlier
patch in this series, "net: skbuff: add skb_checksum_full() that reports
failure". Could the commit message mention that dependency, so a backport
takes both patches?

-- 
Sashiko AI review · 
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261019-b4-skbuff-bug-on-followups-v1-0-41dbef50b9f7%40toxicpanda.com



 


Rackspace

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