netdev - Re: [PATCH] xen-netfront: Fix handling packets on compound pages with skb

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: <20140805.161521.1117004847394195837.davem@davemloft.net>
Date:	Tue, 05 Aug 2014 16:15:21 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	zoltan.kiss@...rix.com
Cc:	konrad.wilk@...cle.com, boris.ostrovsky@...cle.com,
	david.vrabel@...rix.com, wei.liu2@...rix.com,
	Ian.Campbell@...rix.com, paul.durrant@...rix.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	xen-devel@...ts.xenproject.org
Subject: Re: [PATCH] xen-netfront: Fix handling packets on compound pages
 with skb_segment

From: Zoltan Kiss <zoltan.kiss@...rix.com>
Date: Mon, 4 Aug 2014 18:29:34 +0100

> On 31/07/14 21:25, David Miller wrote:
>> Secondly, for something like UDP you can't just split the packet up
>> like this, or for any other datagram protocol for that matter.
> The netback/netfront interface currently only supports TSO and
> TSO6. That's why I did the pktgen TCP patch

Do a sendfile() with MSG_MORE over UDP, I bet you can construct a
sequence that violates your constraints too.

It doesn't make sense to focus on TSO, it's a fundamental issue.

Packets can come from anywhere, and you have to be prepared to
generically handle a MAX_SKB_FRAGS loaded SKB with arbitrary
start/end/length fragment configurations.

> Currently netback limits each skb sent through to 18 slots, because it
> has to map every grant ref to a frag. There was an idea to handle this
> problem by removing this limit and let the backend coalesce the
> scattered buffers into a brand new piece, but then the backend would
> pay the price, and it would be huge as most of the packet should be
> copied.

18 slots means that even with linearization the maximum SKB size
you can support is 64K.   (16 * 4096) == 64K, please one extra slot
on each side for potential partial pages, gives us 18.

> We haven't seen this problem very often, and it's also a bit hard to
> reproduce (hence my frag offset-size pktgen patches), but we can't
> afford the assumption that it won't happen very often.

It's trivial to reproduce, I've already shown how one could trigger it
_without_ TSO being involved at all.  I'll state it again:

	Set TCP_CORK, or use MSG_MORE on the socket.  Do a sequence of
	many 1 byte sendfile() requests over a file, skipping around
	the offset on every call in order to prevent coalescing.

	Clear TCP_CORK or MSG_MORE, you should see a MAX_SKB_FRAGS skb
	end up in the driver transmit function.

> The main concept in this solution is that if it turns out the packet
> needs too many slots in start_xmit, pretend that netfront is not GSO
> capable, and fall back to the software segmentation, which will result
> in packets which can fit.

This is the fundamental issue with your solution.  It is not a GSO
problem.

You therefore have to fully linearize the packet when you encounter
this situation.
--
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