[PATCH] net: fix neigh_resolve_output can cause skb_under_panic

[PATCH] net: fix neigh_resolve_output can cause skb_under_panic

[Ramesh Nagappa]

>From fd023edd911ef12aca38a72b40241661c202684f Mon Sep 17 00:00:00 2001
From: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
Date: Thu, 27 Sep 2012 10:20:58 -0700
Subject: [PATCH] net: fix neigh_resolve_output can cause skb_under_panic

The retry loop in the neigh_resolve_output() and neigh_connected_output() can add
a hard_header without resetting the skb to network header. This causes the
skb_push() in dev_hard_header() to fail.
Signed-off-by: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
Reviewed-by: Billie Alsup <billie.alsup@ericsson.com>
Reviewed-by: Robert Coulson <robert.coulson@ericsson.com>
---
 net/core/neighbour.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 96bb0a3..5a3dfec5 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1327,6 +1327,7 @@ int neigh_resolve_output(struct sk_buff *skb)
 
 		do {
 			seq = read_seqbegin(&neigh->ha_lock);
+			__skb_pull(skb, skb_network_offset(skb));
 			err = dev_hard_header(skb, dev, ntohs(skb->protocol),
 					      neigh->ha, NULL, skb->len);
 		} while (read_seqretry(&neigh->ha_lock, seq));
@@ -1358,10 +1359,10 @@ int neigh_connected_output(struct sk_buff *skb)
 	struct net_device *dev = neigh->dev;
 	unsigned int seq;
 
-	__skb_pull(skb, skb_network_offset(skb));
 
 	do {
 		seq = read_seqbegin(&neigh->ha_lock);
+		__skb_pull(skb, skb_network_offset(skb));
 		err = dev_hard_header(skb, dev, ntohs(skb->protocol),
 				      neigh->ha, NULL, skb->len);
 	} while (read_seqretry(&neigh->ha_lock, seq));
-- 
1.7.3.1

Re: [PATCH] net: fix neigh_resolve_output can cause skb_under_panic

[gregkh]

On Fri, Sep 28, 2012 at 07:40:39PM -0400, Ramesh Nagappa wrote:
> >From fd023edd911ef12aca38a72b40241661c202684f Mon Sep 17 00:00:00 2001
> From: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
> Date: Thu, 27 Sep 2012 10:20:58 -0700
> Subject: [PATCH] net: fix neigh_resolve_output can cause skb_under_panic

Why is all of this in the middle of the changelog section?

I'm guessing you didn't use 'git send-email' for this?

And why are you copying me on the patch?

> The retry loop in the neigh_resolve_output() and neigh_connected_output() can add
> a hard_header without resetting the skb to network header. This causes the
> skb_push() in dev_hard_header() to fail.
> Signed-off-by: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
> Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
> Reviewed-by: Billie Alsup <billie.alsup@ericsson.com>
> Reviewed-by: Robert Coulson <robert.coulson@ericsson.com>

You need a blank line before the first Signed-off-by: line.  Surely one
of the reviewers should have caught this basic thing?

greg k-h

RE: [PATCH] net: fix neigh_resolve_output can cause skb_under_panic

[Ramesh Nagappa]

Sorry, Outlook managled the patch. I will try to find a way to re-submit it.

> 
> You need a blank line before the first Signed-off-by: line.  
> Surely one of the reviewers should have caught this basic thing?
> 
> greg k-h
> 

-- Ramesh

RE: [PATCH] net: fix neigh_resolve_output can cause skb_under_panic

[Ramesh Nagappa]

> 
> Why is all of this in the middle of the changelog section?
> 
> I'm guessing you didn't use 'git send-email' for this?
> 
> And why are you copying me on the patch?

I got the CC list from scripts/get_maintainers.pl

asglx-2-300 $ scripts/get_maintainer.pl 0001-net-fix-neigh_resolve_output-can-cause-skb_under_pan.patch
"David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING [GENERAL],commit_signer:3/4=75%)
Greg Kroah-Hartman <gregkh@linuxfoundation.org> (commit_signer:3/4=75%)
Eric Dumazet <eric.dumazet@gmail.com> (commit_signer:2/4=50%)
Shawn Lu <shawn.lu@ericsson.com> (commit_signer:1/4=25%)
Michel Machado <michel@digirati.com.br> (commit_signer:1/4=25%)
netdev@vger.kernel.org (open list:NETWORKING [GENERAL])
linux-kernel@vger.kernel.org (open list)

> 
> 
> You need a blank line before the first Signed-off-by: line.  
> Surely one of the reviewers should have caught this basic thing?

Outlook mangled the patch. I am unable to use git send-email because of
a corporate firewall on the build machine.

> 
> greg k-h
> 

Thanks,
Ramesh

Re: [PATCH] net: fix neigh_resolve_output can cause skb_under_panic

[gregkh]

On Fri, Sep 28, 2012 at 08:26:51PM -0400, Ramesh Nagappa wrote:
> > 
> > Why is all of this in the middle of the changelog section?
> > 
> > I'm guessing you didn't use 'git send-email' for this?
> > 
> > And why are you copying me on the patch?
> 
> I got the CC list from scripts/get_maintainers.pl
> 
> asglx-2-300 $ scripts/get_maintainer.pl 0001-net-fix-neigh_resolve_output-can-cause-skb_under_pan.patch
> "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING [GENERAL],commit_signer:3/4=75%)
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> (commit_signer:3/4=75%)
> Eric Dumazet <eric.dumazet@gmail.com> (commit_signer:2/4=50%)
> Shawn Lu <shawn.lu@ericsson.com> (commit_signer:1/4=25%)
> Michel Machado <michel@digirati.com.br> (commit_signer:1/4=25%)
> netdev@vger.kernel.org (open list:NETWORKING [GENERAL])
> linux-kernel@vger.kernel.org (open list)

I can't reproduce this:
	$ ./scripts/get_maintainer.pl --file net/core/neighbour.c
	"David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING [GENERAL],commit_signer:22/22=100%)
	Eric Dumazet <edumazet@google.com> (commit_signer:4/22=18%)
	"Eric W. Biederman" <ebiederm@xmission.com> (commit_signer:2/22=9%)
	Pavel Emelyanov <xemul@parallels.com> (commit_signer:2/22=9%)
	netdev@vger.kernel.org (open list:NETWORKING [GENERAL])
	linux-kernel@vger.kernel.org (open list)

What tree are you doing that against?

> > You need a blank line before the first Signed-off-by: line.  
> > Surely one of the reviewers should have caught this basic thing?
> 
> Outlook mangled the patch. I am unable to use git send-email because of
> a corporate firewall on the build machine.

Then your patch would also be corrupted, Outlook, and Exchange, can not
handle patches at all.  Please read Documentation/email_clients.txt for
more details.

Also, ask your coworkers who properly submit patches what they do to
work around your broken email infrastructure.

good luck,

greg k-h

Re: [PATCH] net: fix neigh_resolve_output can cause skb_under_panic

[Ben Hutchings]

On Fri, 2012-09-28 at 17:57 -0700, gregkh@linuxfoundation.org wrote:
[...]
> > > You need a blank line before the first Signed-off-by: line.  
> > > Surely one of the reviewers should have caught this basic thing?
> > 
> > Outlook mangled the patch. I am unable to use git send-email because of
> > a corporate firewall on the build machine.
> 
> Then your patch would also be corrupted, Outlook, and Exchange, can not
> handle patches at all.  Please read Documentation/email_clients.txt for
> more details.
> 
> Also, ask your coworkers who properly submit patches what they do to
> work around your broken email infrastructure.

I successfully send patches using git-imap-send and Evolution talking to
Exchange 2010 through its IMAP and SMTP submission interfaces.  (I think
the previous version worked as well.)  But I would agree that Outlook is
probably hopeless.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.