aboutsummaryrefslogtreecommitdiff
path: root/net/bridge
diff options
context:
space:
mode:
authorMichael Braun <michael-dev@fami-braun.de>2010-03-16 00:26:22 -0700
committerDavid S. Miller <davem@davemloft.net>2010-03-16 00:26:22 -0700
commit7f7708f0055e49e331f267700aa8b2ee879f004c (patch)
tree385da7d1b722232c3c9003af52079318aa51326e /net/bridge
parent0821ec55bb1382dd3b9476dc15d5dbbb19f0c3fd (diff)
bridge: Fix br_forward crash in promiscuous mode
From: Michael Braun <michael-dev@fami-braun.de> bridge: Fix br_forward crash in promiscuous mode It's a linux-next kernel from 2010-03-12 on an x86 system and it OOPs in the bridge module in br_pass_frame_up (called by br_handle_frame_finish) because brdev cannot be dereferenced (its set to a non-null value). Adding some BUG_ON statements revealed that BR_INPUT_SKB_CB(skb)->brdev == br-dev (as set in br_handle_frame_finish first) only holds until br_forward is called. The next call to br_pass_frame_up then fails. Digging deeper it seems that br_forward either frees the skb or passes it to NF_HOOK which will in turn take care of freeing the skb. The same is holds for br_pass_frame_ip. So it seems as if two independent skb allocations are required. As far as I can see, commit b33084be192ee1e347d98bb5c9e38a53d98d35e2 ("bridge: Avoid unnecessary clone on forward path") removed skb duplication and so likely causes this crash. This crash does not happen on 2.6.33. I've therefore modified br_forward the same way br_flood has been modified so that the skb is not freed if skb0 is going to be used and I can confirm that the attached patch resolves the issue for me. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/bridge')
-rw-r--r--net/bridge/br_forward.c14
-rw-r--r--net/bridge/br_input.c2
-rw-r--r--net/bridge/br_private.h2
3 files changed, 13 insertions, 5 deletions
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index d61e6f74112..8347916efe8 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -19,6 +19,10 @@
#include <linux/netfilter_bridge.h>
#include "br_private.h"
+static int deliver_clone(struct net_bridge_port *prev, struct sk_buff *skb,
+ void (*__packet_hook)(const struct net_bridge_port *p,
+ struct sk_buff *skb));
+
/* Don't forward packets to originating port or forwarding diasabled */
static inline int should_deliver(const struct net_bridge_port *p,
const struct sk_buff *skb)
@@ -94,14 +98,18 @@ void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
}
/* called with rcu_read_lock */
-void br_forward(const struct net_bridge_port *to, struct sk_buff *skb)
+void br_forward(const struct net_bridge_port *to, struct sk_buff *skb, struct sk_buff *skb0)
{
if (should_deliver(to, skb)) {
- __br_forward(to, skb);
+ if (skb0)
+ deliver_clone(to, skb, __br_forward);
+ else
+ __br_forward(to, skb);
return;
}
- kfree_skb(skb);
+ if (!skb0)
+ kfree_skb(skb);
}
static int deliver_clone(struct net_bridge_port *prev, struct sk_buff *skb,
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 53b39851d87..08a72e63fb8 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -90,7 +90,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
if (skb) {
if (dst)
- br_forward(dst->dst, skb);
+ br_forward(dst->dst, skb, skb2);
else
br_flood_forward(br, skb, skb2);
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index fef0384e3c0..bfb8feb310a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -252,7 +252,7 @@ extern void br_deliver(const struct net_bridge_port *to,
struct sk_buff *skb);
extern int br_dev_queue_push_xmit(struct sk_buff *skb);
extern void br_forward(const struct net_bridge_port *to,
- struct sk_buff *skb);
+ struct sk_buff *skb, struct sk_buff *skb0);
extern int br_forward_finish(struct sk_buff *skb);
extern void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb);
extern void br_flood_forward(struct net_bridge *br, struct sk_buff *skb,