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

[Josef Bacik]

On Wed, Oct 14, 2026 at 12:30 PM David Laight
<david.laight.linux@xxxxxxxxx> wrote:
>
> On Wed, 14 Oct 2026 14:04:45 +0000
> Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> > On Wed, Oct 14, 2026 at 01:51:54PM +0100, David Laight wrote:
> > > Can't you use that fact that the csum function should only return
> > > either 0 or ~0 and use the other value to indicate an error.
> > >
> > > ISTR this coming up before with the 'copy_to/from_user_and_csum' code.
> >
> > skb_copy_and_csum_bits() returns an unfolded __wsum starting from 0, so
> > 0 is a valid result today.  It would work if it started from ~0 the way
> > csum_and_copy_from_user() does, since that doesn't change the folded
> > result and the sum can never come out as 0.
>
> The result can only be 0 if either the length is zero or all the bytes
> are zero.
> You can probably prove that is never required.
> It is likely better to initialise the csum to either 0xffff or ~0.
>
> IIRC both IPv4 and IPv6 complement the checksum before sending it.
> For IPv6 a zero is invalid (it means checksum not calculated for IPv4),
> I think the best was to achieve that is to initialise the csum to 1
> and then add 1 after inverting it.
>
> > But this is one of those "you have to know the behavior of the function
> > to use it properly." Explicit errors are always better, it removes
> > ambiguity and creates cleaner and less error prone code. I'm happy to
> > change it to start with ~0 as well to add the extra protection, but
> > explicit errors are always best. Thanks,
>
> Look at all the places ERR_PTR() gets used.
> An in-band error indication gives better code and is probably less buggy!
>

...ERR_PTR would be fine too? I'm arguing for explicit return values,
not "0 means failure". But I don't care enough to argue, I'm
respinning. Thanks,

Josef