netdev - Re: [PATCH 3.2 085/115] veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.

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]

Date:	Sat, 30 Apr 2016 14:52:24 -0700
From:	Ben Greear <greearb@...delatech.com>
To:	Vijay Pandurangan <vijayp@...ayp.ca>
CC:	Tom Herbert <tom@...bertland.com>,
	Ben Hutchings <ben@...adent.org.uk>,
	Sabrina Dubroca <sd@...asysnail.net>,
	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	LKML <linux-kernel@...r.kernel.org>, stable@...r.kernel.org,
	akpm@...ux-foundation.org, "David S. Miller" <davem@...emloft.net>,
	Cong Wang <cwang@...pensource.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Evan Jones <ej@...njones.ca>,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	Phil Sutter <phil@....cc>,
	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>,
	Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [PATCH 3.2 085/115] veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.



On 04/30/2016 02:36 PM, Vijay Pandurangan wrote:
> On Sat, Apr 30, 2016 at 5:29 PM, Ben Greear <greearb@...delatech.com> wrote:
>>
>>
>> On 04/30/2016 02:13 PM, Vijay Pandurangan wrote:
>>>
>>> On Sat, Apr 30, 2016 at 4:59 PM, Ben Greear <greearb@...delatech.com>
>>> wrote:
>>>>
>>>>
>>>>
>>>> On 04/30/2016 12:54 PM, Tom Herbert wrote:
>>>>>
>>>>>
>>>>> We've put considerable effort into cleaning up the checksum interface
>>>>> to make it as unambiguous as possible, please be very careful to
>>>>> follow it. Broken checksum processing is really hard to detect and
>>>>> debug.
>>>>>
>>>>> CHECKSUM_UNNECESSARY means that some number of _specific_ checksums
>>>>> (indicated by csum_level) have been verified to be correct in a
>>>>> packet. Blindly promoting CHECKSUM_NONE to CHECKSUM_UNNECESSARY is
>>>>> never right. If CHECKSUM_UNNECESSARY is set in such a manner but the
>>>>> checksum it would refer to has not been verified and is incorrect this
>>>>> is a major bug.
>>>>
>>>>
>>>>
>>>> Suppose I know that the packet received on a packet-socket has
>>>> already been verified by a NIC that supports hardware checksumming.
>>>>
>>>> Then, I want to transmit it on a veth interface using a second
>>>> packet socket.  I do not want veth to recalculate the checksum on
>>>> transmit, nor to validate it on the peer veth on receive, because I do
>>>> not want to waste the CPU cycles.  I am assuming that my app is not
>>>> accidentally corrupting frames, so the checksum can never be bad.
>>>>
>>>> How should the checksumming be configured for the packets going into
>>>> the packet-socket from user-space?
>>>
>>>
>>>
>>> It seems like that only the receiver should decide whether or not to
>>> checksum packets on the veth, not the sender.
>>>
>>> How about:
>>>
>>> We could add a receiving socket option for "don't checksum packets
>>> received from a veth when the other side has marked them as
>>> elide-checksum-suggested" (similar to UDP_NOCHECKSUM), and a sending
>>> socket option for "mark all data sent via this socket to a veth as
>>> elide-checksum-suggested".
>>>
>>> So the process would be:
>>>
>>> Writer:
>>> 1. open read socket
>>> 2. open write socket, with option elide-checksum-for-veth-suggested
>>> 3. write data
>>>
>>> Reader:
>>> 1. open read socket with "follow-elide-checksum-suggestions-on-veth"
>>> 2. read data
>>>
>>> The kernel / module would then need to persist the flag on all packets
>>> that traverse a veth, and drop these data when they leave the veth
>>> module.
>>
>>
>> I'm not sure this works completely.  In my app, the packet flow might be:
>>
>> eth0 <-> raw-socket <-> user-space-bridge <-> raw-socket <-> vethA <-> vethB
>> <-> [kernel router/bridge logic ...] <-> eth1
>
> Good point, so if you had:
>
> eth0 <-> raw <-> user space-bridge <-> raw <-> vethA <-> veth B <->
> userspace-stub <->eth1
>
> and user-space hub enabled this elide flag, things would work, right?
> Then, it seems like what we need is a way to tell the kernel
> router/bridge logic to follow elide signals in packets coming from
> veth. I'm not sure what the best way to do this is because I'm less
> familiar with conventions in that part of the kernel, but assuming
> there's a way to do this, would it be acceptable?

You cannot receive on one veth without transmitting on the other, so
I think the elide csum logic can go on the raw-socket, and apply to packets
in the transmit-from-user-space direction.  Just allowing the socket to make
the veth behave like it used to before this patch in question should be good
enough, since that worked for us for years.  So, just an option to modify the
ip_summed for pkts sent on a socket is probably sufficient.

>> There may be no sockets on the vethB port.  And reader/writer is not
>> a good way to look at it since I am implementing a bi-directional bridge in
>> user-space and each packet-socket is for both rx and tx.
>
> Sure, but we could model a bidrectional connection as two
> unidirectional sockets for our discussions here, right?

Best not to I think, you want to make sure that one socket can
correctly handle tx and rx.  As long as that works, then using
uni-directional sockets should work too.

Thanks,
Ben

-- 
Ben Greear <greearb@...delatech.com>
Candela Technologies Inc  http://www.candelatech.com