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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <773d830b89814ab8a92dc892ec6e65e2@AcuMS.aculab.com>
Date:   Wed, 22 Jul 2020 09:45:23 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Linus Torvalds' <torvalds@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>
CC:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>
Subject: RE: [PATCH 04/18] csum_and_copy_..._user(): pass 0xffffffff instead
 of 0 as initial sum

From: Linus Torvalds
> Sent: 21 July 2020 21:55
> On Tue, Jul 21, 2020 at 1:25 PM Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > Preparation for the change of calling conventions; right now all
> > callers pass 0 as initial sum.  Passing 0xffffffff instead yields
> > the values comparable mod 0xffff and guarantees that 0 will not
> > be returned on success.
> 
> 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.

It isn't necessarily stupid :-)
A 64bit sum can be reduced to 16bits using shifts and adds
(as us usually done) of using 'sum % 0xffff'.
Provided the compiler uses 'multiply by reciprocal' the code
isn't that bad - it might even be difficult to say which is faster.
However that makes the output domain 0..fffe not 1..ffff.

The checksum generation code really needs to know which is used.
So it is best never to use the % version.
If the sum is known to be 1..0xffff then after inversion it is
0..fffe but the required domain is 1..ffff.
This can be fixed by adding 1 - provided a compensating 1 is
added in before the inversion.
The easy place to do this is to feed 1 (not 0 or ~0) into the
first checksum block.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)