Patchwork [Bugme-new,Bug,12327] New: Intermittent TCP issues with => 2.6.27

login
register
mail settings
Submitter Herbert Xu
Date Jan. 9, 2009, 12:04 p.m.
Message ID <20090109120455.GB12486@gondor.apana.org.au>
Download mbox | patch
Permalink /patch/17502/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Herbert Xu - Jan. 9, 2009, 12:04 p.m.
On Fri, Jan 09, 2009 at 10:55:15PM +1100, Herbert Xu wrote:
> 
> It turns out that even though we have sysctl's that's supposed
> to control pppoe/vlan processing, they don't actually work.
> 
> This patch should make them work.

With that we can actually turn them off.

bridge: Disable PPPOE/VLAN processing by default

The PPPOE/VLAN processing code in the bridge netfilter is broken
by design.  The VLAN tag and the PPPOE session ID are an integral
part of the packet flow information, yet they're completely
ignored by the bridge netfilter.  This is potentially a security
hole as it treats all VLANs and PPPOE sessions as the same.

What's more, it's actually broken for PPPOE as the bridge netfilter
tries to trim the packets to the IP length without adjusting the
PPPOE header (and adjusting the PPPOE header isn't much better
since the PPPOE peer may require the padding to be present).

Therefore we should disable this by default.

It does mean that people relying on this feature may lose networking
depending on how their bridge netfilter rules are configured.
However, IMHO the problems this code causes are serious enough to
warrant this.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>


Cheers,
Patrick McHardy - Jan. 12, 2009, 5:30 a.m.
Herbert Xu wrote:
> bridge: Disable PPPOE/VLAN processing by default
> 
> The PPPOE/VLAN processing code in the bridge netfilter is broken
> by design.  The VLAN tag and the PPPOE session ID are an integral
> part of the packet flow information, yet they're completely
> ignored by the bridge netfilter.  This is potentially a security
> hole as it treats all VLANs and PPPOE sessions as the same.
> 
> What's more, it's actually broken for PPPOE as the bridge netfilter
> tries to trim the packets to the IP length without adjusting the
> PPPOE header (and adjusting the PPPOE header isn't much better
> since the PPPOE peer may require the padding to be present).
> 
> Therefore we should disable this by default.
> 
> It does mean that people relying on this feature may lose networking
> depending on how their bridge netfilter rules are configured.
> However, IMHO the problems this code causes are serious enough to
> warrant this.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

A good reason to disable this crap :)

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Speedster - Jan. 13, 2009, 10:50 a.m.
Herbert Xu wrote:
> On Fri, Jan 09, 2009 at 10:55:15PM +1100, Herbert Xu wrote:
>> It turns out that even though we have sysctl's that's supposed
>> to control pppoe/vlan processing, they don't actually work.
>>
>> This patch should make them work.
> 

<snip>

> Cheers,

I can confirm this resolves the issue. I have compiled a 2.6.28 kernel 
with Herbert's patches and I can now use the packaged 2.6.27-9 kernel in 
the Ubuntu guest.

Cheers
Dean
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Speedster - March 6, 2009, 10:39 a.m.
Have these commits made it into a kernel release yet? I haven't seen 
them in any of the Changelogs and am keen to get back to a packaged 
distribution kernel :)

Sorry if if I have missed it, but if not is there any indication of when 
it should make it in?

Cheers
Dean
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ilpo Järvinen - March 25, 2009, 1:26 p.m.
On Fri, 6 Mar 2009, Dean Holland wrote:

> Have these commits made it into a kernel release yet? I haven't seen them in
> any of the Changelogs and am keen to get back to a packaged distribution
> kernel :)
> 
> Sorry if if I have missed it, but if not is there any indication of when it
> should make it in?

They are included in, commits:

a2bd40ad3151d4d346fd167e01fb84b06f7247fc
47e0e1ca13d64eeeb687995fbe4e239e743d7544

I think they are in 2.9.29-rc2 already and thus in 2.6.29 (if I read 
gitk's output correctly).

Patch

diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 9a1cd75..cf754ac 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -58,11 +58,11 @@  static struct ctl_table_header *brnf_sysctl_header;
 static int brnf_call_iptables __read_mostly = 1;
 static int brnf_call_ip6tables __read_mostly = 1;
 static int brnf_call_arptables __read_mostly = 1;
-static int brnf_filter_vlan_tagged __read_mostly = 1;
-static int brnf_filter_pppoe_tagged __read_mostly = 1;
+static int brnf_filter_vlan_tagged __read_mostly = 0;
+static int brnf_filter_pppoe_tagged __read_mostly = 0;
 #else
-#define brnf_filter_vlan_tagged 1
-#define brnf_filter_pppoe_tagged 1
+#define brnf_filter_vlan_tagged 0
+#define brnf_filter_pppoe_tagged 0
 #endif
 
 static inline __be16 vlan_proto(const struct sk_buff *skb)