Re: [PATCH 04/18] csum_and_copy_..._user(): pass 0xffffffff instead of 0 as initial sum

From: Al Viro
Date: Tue Jul 21 2020 - 17:11:22 EST


On Tue, Jul 21, 2020 at 01:58:47PM -0700, Linus Torvalds wrote:
> On Tue, Jul 21, 2020 at 1:55 PM Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > This seems dangerous to me.
> >
> > Maybe some implementation depends on the fact that they actually do
> > the csum 16 bits at a time, and never see an overflow in "int",
> > because they keep folding things.
> >
> > You now break that assumption, and give it an initial value that the
> > csum code itself would never generate, and wouldn't handle right.
> >
> > But I didn't check. Maybe we don't have anything that stupid in the kernel.

I did.

> I take it back. The very first place I looked seemed to do exactly that.
>
> See "do_csum()" in the kernel. It doesn't handle carry for any of the
> usual cases, exactly because it knows it doesn't need to.
>
> Ok, so do_csum() doesn't take that initial value, but it's very much
> an example of the kind of algorithm I was thinking of: it does do
> things 32 bits at a time and handles the carry bit in that inner loop,
> but internally it knows that the val;ues are limited in other places,
> and doesn't need to handle carry everywhere.

Theoretically - sure. I can post the full analysis of that stuff (starting
with the proof that all instances of csum_partial() are OK in that respect,
which takes care of the default instances, then instance-by-instance
analysis of the rest); will need to collate the pieces, remove the actionable
obscenities, etc., but I have done that analysis. Made for rather unpleasant
couple of weeks... ;-/