netdev - Re: [PATCH 9/9 v2] net: Add sysctl to trust checksum

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: <CA+mtBx-hAJDvXigrw1w3S1OBhmG4iiZvBBKNtCXZMt+8ZzFduw@mail.gmail.com>
Date:	Sun, 27 Apr 2014 10:03:48 -0700
From:	Tom Herbert <therbert@...gle.com>
To:	Stephen Hemminger <stephen@...workplumber.org>
Cc:	David Miller <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH 9/9 v2] net: Add sysctl to trust checksum_complete

On Sat, Apr 26, 2014 at 7:31 PM, Stephen Hemminger
<stephen@...workplumber.org> wrote:
> On Sat, 26 Apr 2014 14:26:35 -0700 (PDT)
> Tom Herbert <therbert@...gle.com> wrote:
>
>> Currently if a device provides CHECKSUM_COMPLETE but the checksum
>> is calculated to be invalid we recompute the checksum and try
>> again in software. On the other hand, if device returns
>> CHECKSUM_UNNECESSARY we implicitly trust it and don't verify what it
>> did. This seems backwards!
>>
>> Add a sysctl to trust the device and report an invalid checksum when
>> CHECKSUM_COMPLETE shows it is incorrect. sysctl defaults to enabled.
>>
>> Signed-off-by: Tom Herbert <therbert@...gle.com>
>> ---
>
> NO. Make one choice and do it consistently.
>
> Papering over driver bugs or design confusion with a sysctl is not a
> reasonable choice.
>
> If some device (or code path) has invalid checksum logic, it should
> be reported once and go ahead and fix it in software. The problem with
> CHECKSUM_UNNECESSARY is that there is no way to check that the device
> is broken without computing the checksum (catch-22).
>
Unlike CHECKSUM_UNNECESSARY, CHECKSUM_COMPLETE provides information
for both valid and invalid checksums. Also, CHECKSUM_COMPLETE is well
defined and specific as to what the device and driver are returning
and I have to believe should be easier to get right. So I don't see
any reason why we shouldn't use the negative information returned by
CHECKSUM_COMPLETE; always recomputing an invalid checksum in SW is a
waste of CPU cycles and could become a basis for DOS attack at high
speeds.

In the unlikely event that the HW (or driver) is incorrectly computing
the checksum this would manifest itself as checksum errors. This is
quite debuggable, and the sysctl would be useful to narrow down
whether the issue is in checksum calculation logic or actual bad
checksums. Compare this to incorrect reporting of CHECKSUM_UNNECESSARY
which could result in packets with errors being accepted without any
detection-- it might take years to identify such a problem!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html