bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()
Subsystems (3)
PATCH 1/1 bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()
if dst is not local br_handle_frame_finish() does not clone original skb and forgets to reset IPCB before return to IP stack. it can lead to stack corruption in icmp_send()
--- net/bridge/br_input.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
net/bridge/br_input.c +3 -2
@@ -93,10 +93,11 @@ int br_handle_frame_finish(struct sk_buff *skb) skb2 = skb; br->dev->stats.multicast++; - }else if ((dst =__br_fdb_get (br, dest)) && dst->is_local) { + }else if ((dst =__br_fdb_get (br, dest)) !=NULL ) { skb2 = skb;/* Do not forward the packet since it's local. */ - skb =NULL ; +if (dst->is_local) { + skb =NULL ; }if (skb) {
-- 1.7.5.4
Quoted 3 lines · Vasily Averin
if dst is not local
br_handle_frame_finish()does not clone original skb and forgets to reset IPCB before return to IP stack. it can lead to stack corruption inicmp_send()
example of stack corruption
http://bugzilla.openvz.org/show_bug.cgi?id=2016
Quoted 24 lines
Signed-off-by Vasily Averin <[email protected]>--- net/bridge/br_input.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)net/bridge/br_input.c +3
-3@@ -93,10 +93,11 @@ int br_handle_frame_finish(struct sk_buff *skb) skb2 = skb; br->dev->stats.multicast++; - }else if ((dst =__br_fdb_get (br, dest)) && dst->is_local) { + }else if ((dst =__br_fdb_get (br, dest)) !=NULL ) { skb2 = skb;/* Do not forward the packet since it's local. */ - skb =NULL ; +if (dst->is_local) { + skb =NULL ; }if (skb) { -- 1.7.5.4
Quoted 17 lines · On Wed, 02 Nov 2011 23:08:57 +0400 Vasily Averin <[email protected]> wrote:
if dst is not local
br_handle_frame_finish()does not clone original skb and forgets to reset IPCB before return to IP stack. it can lead to stack corruption inicmp_send()Signed-off-by Vasily Averin <[email protected]>--- net/bridge/br_input.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)net/bridge/br_input.c +0
-0@@ -93,10 +93,11 @@ int br_handle_frame_finish(struct sk_buff *skb) skb2 = skb; br->dev->stats.multicast++;
net/bridge/br_input.c int br_handle_frame_finish(struct sk_buff *skb) · line 103
Quoted 10 lines
net/bridge/br_input.c +3
-2- }else if ((dst =__br_fdb_get (br, dest)) && dst->is_local) { + }else if ((dst =__br_fdb_get (br, dest)) !=NULL ) { skb2 = skb;/* Do not forward the packet since it's local. */ - skb =NULL ; +if (dst->is_local) { + skb =NULL ; }if (skb) {
What kernel version are you using? There were several previous fixes in br_netfilter to deal with this type of issue over the last year.
Quoted 3 lines · Stephen Hemminger
What kernel version are you using? There were several previous fixes in
br_netfilterto deal with this type of issue over the last year.
It's a kernel from this obscure company called Red Hat, and they include it in this product called Red Hat Enterprise Linux 6.
Specifically, 042stab37.1 :)
-Daniel
Quoted 11 lines · On Wed, 2 Nov 2011 13:45:29 -0600 Daniel Robbins <[email protected]> wrote:
Quoted 3 lines · Stephen Hemminger
What kernel version are you using? There were several previous fixes in
br_netfilterto deal with this type of issue over the last year.It's a kernel from this obscure company called Red Hat, and they include it in this product called Red Hat Enterprise Linux 6.
Specifically, 042stab37.1 :)
-Daniel
The pay for their tech support and have them fix it. BTW - the problem was fixed upstream a long time ago, the upstream kernel moves fast and RHEL probably froze many months before their release.
David Miller
maintainerFrom: Stephen Hemminger <[email protected]>
Date: Wed, 2 Nov 2011 13:14:45 -0700
Quoted 11 lines
Quoted 8 lines · On Wed, 2 Nov 2011 13:45:29 -0600 Daniel Robbins <[email protected]> wrote:
Quoted 3 lines · Stephen Hemminger
What kernel version are you using? There were several previous fixes in
br_netfilterto deal with this type of issue over the last year.It's a kernel from this obscure company called Red Hat, and they include it in this product called Red Hat Enterprise Linux 6.
Quoted 8 lines
Quoted 3 lines
Specifically, 042stab37.1 :)
-Daniel
The pay for their tech support and have them fix it. BTW - the problem was fixed upstream a long time ago, the upstream kernel moves fast and RHEL probably froze many months before their release.
So the original patch not only wasn't build tested, the submitter also didn't even check if upstream even _had_ the bug to begin with.
Quoted 5 lines · Stephen Hemminger
The pay for their tech support and have them fix it. BTW - the problem was fixed upstream a long time ago, the upstream kernel moves fast and RHEL probably froze many months before their release.
OK, thanks for the info. I moved my server to Linux ~3.1.0 and will be tracking mainline instead of using RHEL from this point forward. I am sick of dealing with the stale networking code in RHEL kernels.
Thanks and Regards,
Daniel
Quoted 9 lines · Stephen Hemminger
Quoted 3 lines · On Wed, 02 Nov 2011 23:08:57 +0400 Vasily Averin <[email protected]> wrote:
if dst is not local
br_handle_frame_finish()does not clone original skb and forgets to reset IPCB before return to IP stack. it can lead to stack corruption inicmp_send()What kernel version are you using? There were several previous fixes in
br_netfilterto deal with this type of issue over the last year.
Originally it was noticed on RHEL6-based kernel
You are right, in mainline this issue was fixed in br_nf_forward_ip() long time ago.
thank you,
Vasily Averin
David Miller
maintainerFrom: Vasily Averin <[email protected]>
Date: Wed, 02 Nov 2011 23:08:57 +0400
Quoted 5 lines
if dst is not local
br_handle_frame_finish()does not clone original skb and forgets to reset IPCB before return to IP stack. it can lead to stack corruption inicmp_send()Signed-off-by Vasily Averin <[email protected]>
Nothing is worse than posting a patch that doesn't even compile.
And I really mean _nothing_.