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: <20200725175434.GP2786714@ZenIV.linux.org.uk>
Date:   Sat, 25 Jul 2020 18:54:34 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
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

On Tue, Jul 21, 2020 at 10:11:18PM +0100, Al Viro wrote:

> 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... ;-/

BTW, "full" in the above refers only to the cases were we do the switchover
from initial sum 0 to initial sum ~0U (csum_and_copy_..._user() and those
csum_partial_copy_nocheck() instances that share helpers with them).

However, now that I started to write that down I went to look at other
csum_partial_copy_nocheck() instances.  Turns out that c6x one is,
strictly speaking, broken - it works for all initial sum values the
kernel ever gives it, but you can come up with a combination of sum and
data that will return the wrong value.

The only reason we never hit it is that there's only one caller
(gone in this series) that ever passes non-zero, and that one caller
(icmp_push_reply()) can't get to the vulnerable values of initial sums
since on c6x the values of csum_partial_copy_nocheck() are initial_sum +
16bit unsigned, and initial sum in question is csum_add() of a csums of
fragments, possibly rotated 8 bits right.  And to get overflow in that
sucker you'd need 0xffff as the top halfword, so you'd need at least
257 fragments, including at least one at odd offset.  Which, AFAICS,
can't happen - all framing headers have even lengths.