[lkml]   [2020]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [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.

 \ /
  Last update: 2020-07-25 19:55    [W:0.071 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site