diff mbox

[RFC] ixgbe: ixgbe_atr() must check if network header is available in headlen

Message ID 20161015213104.GK31471@oracle.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Sowmini Varadhan Oct. 15, 2016, 9:31 p.m. UTC
For some Tx paths (e.g., tpacket_snd()), ixgbe_atr may be
passed down an sk_buff that has the network and transport
header in the paged data, so it needs to make sure these
headers are available in the headlen bytes to calculate the
l4_proto.

This patch bails out if the headlen is "too short", and does
not attempt to call skb_header_pointer() to get the needed
bytes: the assumption is that the caller should set things
up properly if the l4_proto based tx steering is desired.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Comments

Duyck, Alexander H Oct. 17, 2016, 2:38 p.m. UTC | #1
On Sat, 2016-10-15 at 17:31 -0400, Sowmini Varadhan wrote:
> For some Tx paths (e.g., tpacket_snd()), ixgbe_atr may be

> passed down an sk_buff that has the network and transport

> header in the paged data, so it needs to make sure these

> headers are available in the headlen bytes to calculate the

> l4_proto.

> 

> This patch bails out if the headlen is "too short", and does

> not attempt to call skb_header_pointer() to get the needed

> bytes: the assumption is that the caller should set things

> up properly if the l4_proto based tx steering is desired.

> 

> > Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>

> ---

>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 +++++++++

>  1 files changed, 9 insertions(+), 0 deletions(-)

> 

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

> index a244d9a..0868de1 100644

> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

> @@ -7632,6 +7632,7 @@ static void ixgbe_atr(struct ixgbe_ring *ring,

> >  	struct sk_buff *skb;

> >  	__be16 vlan_id;

> >  	int l4_proto;

> > +	int min_hdr_size = 0;

>  

> >  	/* if ring doesn't have a interrupt vector, cannot perform ATR */

> >  	if (!q_vector)

> @@ -7650,6 +7651,14 @@ static void ixgbe_atr(struct ixgbe_ring *ring,

>  

> >  	/* snag network header to get L4 type and address */

> >  	skb = first->skb;

> > +	if (first->protocol == htons(ETH_P_IP))

> > +		min_hdr_size = sizeof(struct iphdr) +

> > +			       sizeof(struct tcphdr);

> > +	else if (first->protocol == htons(ETH_P_IPV6))

> > +		min_hdr_size = sizeof(struct ipv6hdr) +

> > +			       sizeof(struct tcphdr);

> > +	if (min_hdr_size && skb_headlen(skb) < ETH_HLEN + min_hdr_size)

> > +		return;

> >  	hdr.network = skb_network_header(skb);

> >  	if (skb->encapsulation &&

> >  	    first->protocol == htons(ETH_P_IP) &&


So this doesn't really cover all the cases necessary.  There end up
being essentially 3 spots where we need to perform checks to verify the
header size.

The first one is inside the checks for skb->encapsulation, ETH_P_IP,
and IPPROTO_UDP.  We could probably just verify that skb_tail_pointer
is greater than skb_transport_header + (8 + 8 + 14), the minimum size
needed to support VxLAN.

The second block where we need to perform this check would be after
this check once the network header has been updated.  There we need to
verify that hdr.network + 40 is less than or equal to skb_tail_pointer.
 That covers both IPv4 w/ TCP or IPv6.

The third check that needs to be performed is to verify that
hdr.network + hlen is greater than or equal to skb_tail_pointer - 20.
That is needed to verify we have enough room for the tcp header data to
be pulled.

- Alex
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a244d9a..0868de1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7632,6 +7632,7 @@  static void ixgbe_atr(struct ixgbe_ring *ring,
 	struct sk_buff *skb;
 	__be16 vlan_id;
 	int l4_proto;
+	int min_hdr_size = 0;
 
 	/* if ring doesn't have a interrupt vector, cannot perform ATR */
 	if (!q_vector)
@@ -7650,6 +7651,14 @@  static void ixgbe_atr(struct ixgbe_ring *ring,
 
 	/* snag network header to get L4 type and address */
 	skb = first->skb;
+	if (first->protocol == htons(ETH_P_IP))
+		min_hdr_size = sizeof(struct iphdr) +
+			       sizeof(struct tcphdr);
+	else if (first->protocol == htons(ETH_P_IPV6))
+		min_hdr_size = sizeof(struct ipv6hdr) +
+			       sizeof(struct tcphdr);
+	if (min_hdr_size && skb_headlen(skb) < ETH_HLEN + min_hdr_size)
+		return;
 	hdr.network = skb_network_header(skb);
 	if (skb->encapsulation &&
 	    first->protocol == htons(ETH_P_IP) &&