[PATCH V3 net-next] ixgbe: Avoid unaligned access in ixgbe_atr() for LLC packets

Subsystems: intel ethernet drivers, networking drivers, the rest

STALE3828d

4 messages, 3 authors, 2016-03-17 · open the first message on its own page

[PATCH V3 net-next] ixgbe: Avoid unaligned access in ixgbe_atr() for LLC packets

From: Sowmini Varadhan <hidden>
Date: 2016-03-14 19:47:29

For LLC based protocols like lldp, stp etc., the ethernet header
is an 802.3 header with a h_proto that is not 0x800, 0x86dd, or
even 0x806.  In this world, the skb_network_header() points at
the DSAP/SSAP/..  and is not likely to be NET_IP_ALIGNed in
ixgbe_atr().

With LLC, drivers are not likely to correctly find IPVERSION,
or "6", at hdr.ipv4->version, but will instead just needlessly
trigger an unaligned access. (IPv4/IPv6 over LLC is almost never
implemented).

The unaligned access is thus avoidable: bail out quickly after
examining skb->protocol. The only Ethernet II protocols handled
are IPv4 and IPv6

Signed-off-by: Sowmini Varadhan <redacted>
---
v2: Alexander Duyck comments
v3: filter out all ethertypes  except for Ethernet II and (IPv4 or IPv6)

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 4d6223d..aeab2fe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7574,6 +7574,11 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
 	if (!ring->atr_sample_rate)
 		return;
 
+	/* this function only handles IPv4 or IPv6 */
+	if (first->protocol != htons(ETH_P_IP) &&
+	    first->protocol != htons(ETH_P_IPV6))
+		return;
+
 	ring->atr_count++;
 
 	/* snag network header to get L4 type and address */
-- 
1.7.1

Re: [PATCH V3 net-next] ixgbe: Avoid unaligned access in ixgbe_atr() for LLC packets

From: Jeff Kirsher <hidden>
Date: 2016-03-16 22:58:59

On Mon, 2016-03-14 at 15:47 -0400, Sowmini Varadhan wrote:
For LLC based protocols like lldp, stp etc., the ethernet header
is an 802.3 header with a h_proto that is not 0x800, 0x86dd, or
even 0x806.  In this world, the skb_network_header() points at
the DSAP/SSAP/..  and is not likely to be NET_IP_ALIGNed in
ixgbe_atr().

With LLC, drivers are not likely to correctly find IPVERSION,
or "6", at hdr.ipv4->version, but will instead just needlessly
trigger an unaligned access. (IPv4/IPv6 over LLC is almost never
implemented).

The unaligned access is thus avoidable: bail out quickly after
examining skb->protocol. The only Ethernet II protocols handled
are IPv4 and IPv6

Signed-off-by: Sowmini Varadhan <redacted>
---
v2: Alexander Duyck comments
v3: filter out all ethertypes  except for Ethernet II and (IPv4 or
IPv6)

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
This does not apply since Alex Duyck beat you to the fix.  Here is the
patch he submitted on 3/15 which corrects the issue.  So I am dropping
your patch from the queue.

commit 04b8b51c34837765cf6250f69d419c439dc393bf
Author: Alexander Duyck [off-list ref]
Date:   Tue Mar 15 15:10:22 2016 -0700

    ixgbe: Fix ATR so that it correctly handles IPv6 extension headers
    
    The ATR code was assuming that it would be able to use tcp_hdr for
    every TCP frame that came through.  However this isn't the case as
it
    is possible for a frame to arrive that is TCP but sent through
something
    like a raw socket.  As a result the driver was setting up bad
filters in
    which tcp_hdr was really pointing to the network header so the data
was
    all invalid.
    
    In order to correct this I have added a bit of parsing logic that
will
    determine the TCP header location based off of the network header
and
    either the offset in the case of the IPv4 header, or a walk through
the
    IPv6 extension headers until it encounters the header that
indicates
    IPPROTO_TCP.  In addition I have added checks to verify that the
lowest
    protocol provided is recognized as IPv4 or IPv6 to help mitigate
raw
    sockets using ETH_P_ALL from having ATR applied to them.
    
    Signed-off-by: Alexander Duyck [off-list ref]

Re: [PATCH V3 net-next] ixgbe: Avoid unaligned access in ixgbe_atr() for LLC packets

From: Sowmini Varadhan <hidden>
Date: 2016-03-16 23:02:19

On (03/16/16 15:58), Jeff Kirsher wrote:
This does not apply since Alex Duyck beat you to the fix.  Here is the
patch he submitted on 3/15 which corrects the issue.  So I am dropping
your patch from the queue.
That's weird because Alexander was reviewing my fix on the various
lists yesterday, so he could have just told me about this.

But, whatever.

--sowmini

Re: [PATCH V3 net-next] ixgbe: Avoid unaligned access in ixgbe_atr() for LLC packets

From: Alexander Duyck <hidden>
Date: 2016-03-17 01:31:43

On Wed, Mar 16, 2016 at 4:01 PM, Sowmini Varadhan
[off-list ref] wrote:
On (03/16/16 15:58), Jeff Kirsher wrote:
quoted
This does not apply since Alex Duyck beat you to the fix.  Here is the
patch he submitted on 3/15 which corrects the issue.  So I am dropping
your patch from the queue.
That's weird because Alexander was reviewing my fix on the various
lists yesterday, so he could have just told me about this.

But, whatever.
Sowmini,

Just for the record I didn't submit it on March 15 I submitted it on
January 26th as per the log in patchwork,
http://patchwork.ozlabs.org/patch/573058/.

Sorry I didn't realize the patch was not against the next-queue.  Back
when I made that fix I was focused on the IPv6 portion of it so that
piece of it had slipped my mind.  If it is any consolation at least my
patch is 7+ weeks ahead in the queue so hopefully the fix will make it
into net-next as soon as the merge window opens.

- Alex
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help