diff mbox

[net,v2] flow_dissector: Check skb for VLAN only if skb specified.

Message ID 1476736212-21238-1-git-send-email-e@erig.me
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Garver Oct. 17, 2016, 8:30 p.m. UTC
Fixes a panic when calling eth_get_headlen(). Noticed on i40e driver.

Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
Signed-off-by: Eric Garver <e@erig.me>
---
 net/core/flow_dissector.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Jakub Sitnicki Oct. 18, 2016, 12:44 p.m. UTC | #1
On Mon, Oct 17, 2016 at 08:30 PM GMT, Eric Garver wrote:
> Fixes a panic when calling eth_get_headlen(). Noticed on i40e driver.
>
> Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
> Signed-off-by: Eric Garver <e@erig.me>
> ---
>  net/core/flow_dissector.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 1a7b80f73376..44e6ba9d3a6b 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -247,12 +247,10 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>  	case htons(ETH_P_8021Q): {
>  		const struct vlan_hdr *vlan;
>  
> -		if (skb_vlan_tag_present(skb))
> +		if (skb && skb_vlan_tag_present(skb))
>  			proto = skb->protocol;
>  

I was a bit confused that we check skb for VLAN tag again later in the
same block but this time without a NULL check. However, this only
happens when we get called from skb_flow_dissect() or
skb_flow_dissect_flow_keys() variants, which take an skb and pass it to
us.

Feel free to add:

Reviewed-by: Jakub Sitnicki <jkbs@redhat.com>
Or Gerlitz Oct. 18, 2016, 1:59 p.m. UTC | #2
On Mon, Oct 17, 2016 at 11:30 PM, Eric Garver <e@erig.me> wrote:
> Fixes a panic when calling eth_get_headlen(). Noticed on i40e driver.
>
> Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
> Signed-off-by: Eric Garver <e@erig.me>

Dave,

Hadar is OOO and I have asked Amir to look on the fix, will appreciate
if we can have 24 hours to respond

Or.
Amir Vadai Oct. 19, 2016, 8:44 a.m. UTC | #3
On Tue, Oct 18, 2016 at 4:59 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Mon, Oct 17, 2016 at 11:30 PM, Eric Garver <e@erig.me> wrote:
>> Fixes a panic when calling eth_get_headlen(). Noticed on i40e driver.
>>
>> Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
>> Signed-off-by: Eric Garver <e@erig.me>
>
> Dave,
>
> Hadar is OOO and I have asked Amir to look on the fix, will appreciate
> if we can have 24 hours to respond
>
> Or.

Reviewed and tested.

Acked-by: Amir Vadai <amir@vadai.me>
Eric Garver Oct. 19, 2016, 2 p.m. UTC | #4
On Wed, Oct 19, 2016 at 11:44:10AM +0300, Amir Vadai wrote:
> On Tue, Oct 18, 2016 at 4:59 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > On Mon, Oct 17, 2016 at 11:30 PM, Eric Garver <e@erig.me> wrote:
> >> Fixes a panic when calling eth_get_headlen(). Noticed on i40e driver.
> >>
> >> Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
> >> Signed-off-by: Eric Garver <e@erig.me>
> >
> > Dave,
> >
> > Hadar is OOO and I have asked Amir to look on the fix, will appreciate
> > if we can have 24 hours to respond
> >
> > Or.
> 
> Reviewed and tested.
> 
> Acked-by: Amir Vadai <amir@vadai.me>

Amir,

Your ACK may have been lost on its way to the list. I received it, but
it's not showing on any list archives or patchwork.
David Miller Oct. 19, 2016, 2:36 p.m. UTC | #5
From: Eric Garver <e@erig.me>
Date: Mon, 17 Oct 2016 16:30:12 -0400

> Fixes a panic when calling eth_get_headlen(). Noticed on i40e driver.
> 
> Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
> Signed-off-by: Eric Garver <e@erig.me>

Applied, thanks.
diff mbox

Patch

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 1a7b80f73376..44e6ba9d3a6b 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -247,12 +247,10 @@  bool __skb_flow_dissect(const struct sk_buff *skb,
 	case htons(ETH_P_8021Q): {
 		const struct vlan_hdr *vlan;
 
-		if (skb_vlan_tag_present(skb))
+		if (skb && skb_vlan_tag_present(skb))
 			proto = skb->protocol;
 
-		if (!skb_vlan_tag_present(skb) ||
-		    proto == cpu_to_be16(ETH_P_8021Q) ||
-		    proto == cpu_to_be16(ETH_P_8021AD)) {
+		if (eth_type_vlan(proto)) {
 			struct vlan_hdr _vlan;
 
 			vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),