bridge: Fix br_forward crash in promiscuous mode (7f7708f0) · Commits · 戴 / test

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 ("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>

net/bridge/br_forward.c

+11 −3

Original line number	Diff line number	Diff line
		@@ -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,13 +98,17 @@ 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)) {
		if (skb0)
		deliver_clone(to, skb, __br_forward);
		else
		__br_forward(to, skb);
		return;
		}

		if (!skb0)
		kfree_skb(skb);
		}

net/bridge/br_input.c

+1 −1

Original line number	Diff line number	Diff line
		@@ -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);
		}

net/bridge/br_private.h

+1 −1

Original line number	Diff line number	Diff line
		@@ -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,

Admin message