Re: [PATCH net v2] net: skbuff: make skb_copy_and_csum_bits() report unreadable frags

[Mina Almasry]

On Tue, Oct 13, 2026 at 11:02 AM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> skb_copy_and_csum_bits() has no way to fail.  When it reaches
> unreadable frags it zero-fills the rest of the buffer and returns 0,
> and since commit ab9414ed70bd ("net: skbuff: don't leave stale bytes in
> skb_copy_and_csum_bits()") that 0 is the correct checksum for what's in
> the buffer.  Its callers can't tell the copy didn't happen.
>
> That matters in the !sg && !csum branch of skb_segment().  A devmem TCP
> skb keeps its whole payload in unreadable frags, so every segment ends
> up with a payload of zeroes, tcp_gso_segment() writes a valid TCP
> checksum over it, and the peer accepts the zeroes as stream data.  This
> is reachable when both SG and TX checksum offload are turned off on a
> device used for devmem TX: sk_setup_caps() still gives TCP SG and
> HW_CSUM, so the dmabuf send is accepted, validate_xmit_unreadable_skb()
> lets the skb through for a NETMEM_TX_NO_DMA device or a matching
> binding, and software GSO segments it with neither feature.  Before
> that commit the segments carried uninitialized memory with a checksum
> that almost never matched.
>
> Make skb_copy_and_csum_bits() return 0 or -EFAULT and hand the checksum
> back through a pointer, and make it __must_check so a new caller can't
> ignore the failure.  Each caller takes the error path it already has
> for a failed copy:
>
>  - skb_segment() fails the segmentation, as the csum branch already
>    does when skb_copy_bits() fails.
>  - icmp_glue_bits() and icmpv6_getfrag() return the error, so the ICMP
>    error isn't sent.
>  - __ip_append_data() and __ip6_append_data() drop the new skb and
>    return -EFAULT, as they do when getfrag fails.
>  - vnet_skb_shape() fails the same way it does when its skb_copy_bits()
>    call fails.
>  - xdr_skb_read_bits() returns 0 without consuming anything, as it does
>    when skb_copy_bits() fails, so desc.count stays non-zero and
>    csum_partial_copy_to_xdr() fails.  csum_partial_copy_to_xdr() used
>    to checksum the bytes it hadn't copied before checking desc.count.
>    desc.offset and desc.count always add up to skb->len, so that only
>    ran when it was about to fail anyway, and on an unreadable skb
>    skb_checksum() would WARN.  Drop it.
>  - skb_copy_and_csum_dev() can't return an error, so it leaves the
>    checksum alone and the frame goes out with a bad one.  Its callers
>    are drivers that never see unreadable skbs anyway.
>
> The zero-fill stays, so a caller that does send the buffer still
> doesn't leak anything.
>
> This was found by the Sashiko AI review of that commit, and
> Mina suggested reporting the error from the helper rather than checking
> for unreadable frags in skb_segment().
>
> Fixes: ab9414ed70bd ("net: skbuff: don't leave stale bytes in skb_copy_and_csum_bits()")
> Fixes: 65249feb6b3d ("net: add support for skbs with unreadable frags")
> Link: https://lore.kernel.org/all/179148235783.434549.14322374227477832817@xxxxxxxxxx/
> Suggested-by: Mina Almasry <almasrymina@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Patch LGTM, but net + stable + fixes is very overkill for me. This is
a very minor issue and only results in some devmem packets maybe
having a bad csum and get dropped. I would suggest fixing in net-next
(and I know the bot will complain it's a fix that's going to net-next,
but in this case, ignore).

If you send to net-next, plaese add:

Reviewed-by: Mina Almasry <almasrymina@xxxxxxxxxx>

-- 
Thanks,
Mina