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

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

[Sowmini Varadhan]

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) &&
-- 
1.7.1

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

[Duyck, Alexander H]

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