Daniel Robbins | 2 Nov 18:07 2011

RHEL 6 stack corruption bug in bridge code identified

Hi Stephen,

The OpenVZ developers have identified a linux bridging stack
corruption bug in the RHEL 6 kernel:

http://bugzilla.openvz.org/show_bug.cgi?id=2016#c15

Can you confirm that this appears to be a stack corruption bug? Has it
already been fixed? If so, which kernel has it fixed? If not, can you
get a fix into an upcoming vanilla kernel? Thanks :)

-Daniel
Stephen Hemminger | 2 Nov 20:31 2011

Re: [PATCH] bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()

On Wed, 02 Nov 2011 23:08:57 +0400
Vasily Averin <vvs <at> parallels.com> 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
> in icmp_send()
> 
> Signed-off-by: Vasily Averin <vvs <at> sw.ru>
> ---
>  net/bridge/br_input.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index f06ee39..6be8d00 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
>  <at>  <at>  -93,10 +93,11  <at>  <at>  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) {
(Continue reading)

Daniel Robbins | 2 Nov 20:45 2011

Re: [PATCH] bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()

On Wed, Nov 2, 2011 at 1:31 PM, Stephen Hemminger <shemminger <at> vyatta.com> wrote:
>
> 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.

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
David Miller | 2 Nov 21:09 2011
Picon

Re: [PATCH] bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()

From: Vasily Averin <vvs <at> parallels.com>
Date: Wed, 02 Nov 2011 23:08:57 +0400

> 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()
> 
> Signed-off-by: Vasily Averin <vvs <at> sw.ru>

Nothing is worse than posting a patch that doesn't even compile.

And I really mean _nothing_.
Stephen Hemminger | 2 Nov 21:14 2011

Re: [PATCH] bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()

On Wed, 2 Nov 2011 13:45:29 -0600
Daniel Robbins <drobbins <at> funtoo.org> wrote:

> On Wed, Nov 2, 2011 at 1:31 PM, Stephen Hemminger <shemminger <at> vyatta.com> wrote:
> >
> > 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.
> 
> 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 | 2 Nov 21:16 2011
Picon

Re: [PATCH] bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()

From: Stephen Hemminger <shemminger <at> vyatta.com>
Date: Wed, 2 Nov 2011 13:14:45 -0700

> On Wed, 2 Nov 2011 13:45:29 -0600
> Daniel Robbins <drobbins <at> funtoo.org> wrote:
> 
>> On Wed, Nov 2, 2011 at 1:31 PM, Stephen Hemminger <shemminger <at> vyatta.com> wrote:
>> >
>> > 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.
>> 
>> 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.

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.
Daniel Robbins | 2 Nov 21:23 2011

Re: [PATCH] bridge: Reset IPCB on forward non-local packets in br_handle_frame_finish()

On Wed, Nov 2, 2011 at 2:14 PM, Stephen Hemminger <shemminger <at> vyatta.com> wrote:
>
> 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
Martino Fornasa | 8 Nov 10:37 2011
Picon

RSTP implementation status

Hi all.
I'm interested in RSTP implementation (and maybe in contributing to it).

I read this 
[http://lkml.indiana.edu/hypermail/linux/net/0807.1/0011.html] message, 
and I had a look on the ongoing RSTP project hosted at 
[https://github.com/shemminger/RSTP]. Is this project still running?

I noticed that such a ongoing project is based on rstplib 
[http://rstplib.sourceforge.net/]. By taking a brief look at the code, 
it seems to me that such a code is based on the obsolete 802.1w 
specification, while the newest specification for RSTP is contained in 
802.1D-2004; and such a specification (beside incorporating 
compatibility with STP) is radically different from 802.1w (e.g., 
different set of states on state machines...), and it is not easy to 
understand the amount of interoperability between the two.

I think that a serious issue in implementing network bridging/routing 
protocols is to proper validate and testing it. For example, I know that 
there are companies that offer commercial validation system for network 
protocol. As some RSTP implementations as been put in the past in the 
kernel, and STP is already in it, are such implementations have been 
tested and validated? How?

B.R., Martino Fornasa.
Vitalii Demianets | 8 Nov 11:32 2011
Picon

Re: RSTP implementation status

On Tuesday 08 November 2011 11:37:56 Martino Fornasa wrote:
> Hi all.
> I'm interested in RSTP implementation (and maybe in contributing to it).
>
> I read this
> [http://lkml.indiana.edu/hypermail/linux/net/0807.1/0011.html] message,
> and I had a look on the ongoing RSTP project hosted at
> [https://github.com/shemminger/RSTP]. Is this project still running?
>
> I noticed that such a ongoing project is based on rstplib
> [http://rstplib.sourceforge.net/]. By taking a brief look at the code,
> it seems to me that such a code is based on the obsolete 802.1w
> specification, while the newest specification for RSTP is contained in
> 802.1D-2004; and such a specification (beside incorporating
> compatibility with STP) is radically different from 802.1w (e.g.,
> different set of states on state machines...), and it is not easy to
> understand the amount of interoperability between the two.
>

Hello, Martino!
Not really an expert in 802.1w flavor of RSTP, so can't really say if there 
are any differences in behavior of 802.1w- and 802.1D- compatible  
implementations. Just want to point at two alternative projects based on more 
recent standards:

1)
There is 802.1D-compatible RSTP implementation by Aji Srinivas which he 
announced on this list:
http://lists.linux-foundation.org/pipermail/bridge/2009-February/006178.html

(Continue reading)

David Miller | 14 Nov 06:37 2011
Picon

Re: [PATCH] net, bridge: print log message after state changed

From: Holger Brunck <holger.brunck <at> keymile.com>
Date: Thu, 10 Nov 2011 17:18:54 +0100

> From: Wolfgang Fritz <wolfgang.fritz <at> keymile.com>
> 
> Function br_log_state writes log message "... entering XXXX state" so it
> should be called after the state has changed and not before.
> 
> Signed-off-by: Wolfgang Fritz <wolfgang.fritz <at> keymile.com>
> Signed-off-by: Holger Brunck <holger.brunck <at> keymile.com>

"entering" can roughly mean "about to enter"

The message is therefore appropriately timed as far as I'm concerned.

I'm not applying this patch.

Gmane