netdev - Re: [RFC PATCH] Fix false positives in can_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: <CALx6S34LzN5z3r3U-N_tu_xpzuohiVu1B9-NO5neyDhbmJ20BA@mail.gmail.com>
Date:	Mon, 5 Oct 2015 09:23:55 -0700
From:	Tom Herbert <tom@...bertland.com>
To:	David Woodhouse <dwmw2@...radead.org>
Cc:	Jesse Brandeburg <jesse.brandeburg@...il.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH] Fix false positives in can_checksum_protocol()

On Mon, Oct 5, 2015 at 4:16 AM, David Woodhouse <dwmw2@...radead.org> wrote:
> On Tue, 2015-09-29 at 15:52 -0700, Tom Herbert wrote:
>> Please look at ixgbe_tx_csum in ixgbe driver. This one example of how
>> a driver can determine whether the checksum being offloaded is TCP or
>> UDP. The bug in this driver is...
>
> I think it serves better as an example of why we don't *want* drivers
> doing that kind of thing for themselves... :)
>
> I propose we steal some high bits from csum_offset, as you suggested,
> and use them to indicate a 'checksum type', which will include TCP and
> UDP.
>
> Then the filter in netif_skb_features() can trivially do the right
> thing for NETIF_F_IP{V6,}_CSUM devices, so avoid feeding them packets
> they can't handle.
>
> You mentioned that you actually want to deprecate those feature flags —
> which works for me, but it's kind of orthogonal. If we do that we'd
> still want to provide generic functions that such drivers can use as
> their .ndo_features_check() method. And we'd *still* want to do the
> check based on a simple flag, rather than grubbing around in the packet
> data. (And the drivers if they *are* asked to do the checksum will
> sometimes care whether it's TCP vs. UDP too).
>
> I don't think we want drivers calling skb_checksum_help() for
> themselves; we want the pre-filter. Mainly because we *definitely*
> don't want drivers calling gso_skb_segment() for themselves in the same
> situation — see the comment I posted on Friday about the r8169 instance
> of that. ('Re: [PATCH net-next 3/3] r8169: support IPv6').
>
David, here is what I am currently thinking the interface should be:

1) Drivers may advertise NETIF_F_HW_CSUM. The stack will indicate
checksum offload exclusively using the
CHECKSUM_PARTIAL/csum_start/csum_offset interface. No additional
interfaces (bits in skbuff should not be needed)
2) A driver may inspect packets via ndo_check to decide if it wants to
offload the checksum, if not cancels NETIF_F_HW_CSUM in the packet.
3) In driver xmit when CHECKSUM_PARTIAL is set the driver MUST
correctly resolve the checksum-- either by properly offloading to the
device or calling skb_checksum_help.
4) To help drivers for devices with limited offload capabilities we'll
define a helper function to check for typical restrictions (.e.g. IPv4
only, TCP/UDP only. no encapsulation, no IPv6 extension headers,
etc.). I am working on this helper function and will send RFC shortly.

Tom

> --
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@...el.com                              Intel Corporation
>
--
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