Patchwork [Precise/Quantal] bridge: Pull ip header into skb->data before looking into ip header.

login
register
mail settings
Submitter Jesse Sung
Date Oct. 11, 2012, 7:50 a.m.
Message ID <1349941841-11798-1-git-send-email-jesse.sung@canonical.com>
Download mbox | patch
Permalink /patch/190828/
State New
Headers show

Comments

Jesse Sung - Oct. 11, 2012, 7:50 a.m.
From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>

BugLink: https://launchpad.net/bugs/1065150

If lower layer driver leaves the ip header in the skb fragment, it needs to
be first pulled into skb->data before inspecting ip header length or ip version
number.

Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 6caab7b0544e83e6c160b5e80f5a4a7dd69545c7 net.git)

Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com>
---
 net/bridge/br_netfilter.c |    3 +++
 1 file changed, 3 insertions(+)
Andy Whitcroft - Oct. 11, 2012, 10:42 a.m.
On Thu, Oct 11, 2012 at 03:50:41PM +0800, Jesse Sung wrote:
> From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> 
> BugLink: https://launchpad.net/bugs/1065150
> 
> If lower layer driver leaves the ip header in the skb fragment, it needs to
> be first pulled into skb->data before inspecting ip header length or ip version
> number.
> 
> Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 6caab7b0544e83e6c160b5e80f5a4a7dd69545c7 net.git)

Is this going to be in mainline soon?  I do not see it in Linus' tree as
yet, which is presumably why it is not in stable as yet.

> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com>
> ---
>  net/bridge/br_netfilter.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index 577ea5d..7c1745d 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -245,6 +245,9 @@ static int br_parse_ip_options(struct sk_buff *skb)
>  	struct net_device *dev = skb->dev;
>  	u32 len;
>  
> +	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
> +		goto inhdr_error;
> +
>  	iph = ip_hdr(skb);
>  	opt = &(IPCB(skb)->opt);

The description in the bug is rather better at explaining why we need
this.

The patch looks ok overall, I am mildy worried about when it is going to
hit mainline.  But it also sounds pretty important for bridged
scenarios which would include virtual setups.  So overall probabally:

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Sarveshwar Bandi - Oct. 11, 2012, 12:15 p.m.
Andy,
  This patch was applied to David Miller's net tree only yesterday. Patches from net tree are typically pushed to Linus tree once a week. 

Thanks,
Sarvesh

-----Original Message-----
From: Andy Whitcroft [mailto:apw@canonical.com] 
Sent: Thursday, October 11, 2012 4:12 PM
To: Jesse Sung
Cc: kernel-team@lists.ubuntu.com; Bandi,Sarveshwar
Subject: [Acked/comment] [Precise/Quantal][PATCH] bridge: Pull ip header into skb->data before looking into ip header.

On Thu, Oct 11, 2012 at 03:50:41PM +0800, Jesse Sung wrote:
> From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> 
> BugLink: https://launchpad.net/bugs/1065150
> 
> If lower layer driver leaves the ip header in the skb fragment, it 
> needs to be first pulled into skb->data before inspecting ip header 
> length or ip version number.
> 
> Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> Signed-off-by: David S. Miller <davem@davemloft.net> (cherry picked 
> from commit 6caab7b0544e83e6c160b5e80f5a4a7dd69545c7 net.git)

Is this going to be in mainline soon?  I do not see it in Linus' tree as yet, which is presumably why it is not in stable as yet.

> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com>
> ---
>  net/bridge/br_netfilter.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c 
> index 577ea5d..7c1745d 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -245,6 +245,9 @@ static int br_parse_ip_options(struct sk_buff *skb)
>  	struct net_device *dev = skb->dev;
>  	u32 len;
>  
> +	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
> +		goto inhdr_error;
> +
>  	iph = ip_hdr(skb);
>  	opt = &(IPCB(skb)->opt);

The description in the bug is rather better at explaining why we need this.

The patch looks ok overall, I am mildy worried about when it is going to hit mainline.  But it also sounds pretty important for bridged scenarios which would include virtual setups.  So overall probabally:

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Tim Gardner - Oct. 11, 2012, 5 p.m.

Herton Ronaldo Krzesinski - Oct. 11, 2012, 5:04 p.m.

Patch

diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 577ea5d..7c1745d 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -245,6 +245,9 @@  static int br_parse_ip_options(struct sk_buff *skb)
 	struct net_device *dev = skb->dev;
 	u32 len;
 
+	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
+		goto inhdr_error;
+
 	iph = ip_hdr(skb);
 	opt = &(IPCB(skb)->opt);