: bridge: fix double POST_ROUTING invocation

These two patches for stable-2.6.23 fix two problems with the bridge netfilter code, a double POST_ROUTING hook invocation with routing between two bridge devices, which breaks NAT, and some net_device refcount leaks when queueing bridged packets to userspace.

Please apply, thanks.

commit 42265d7441145c4310926c1dcce339e5bd82b601

Author: Patrick McHardy <[email protected]>
Date:   Tue Jan 29 19:02:30 2008 +0100

[NETFILTER]: bridge: fix double POST_ROUTING invocation

Upstream commit 2948d2ebbb98747b912ac6d0c864b4d02be8a6f5

The bridge code incorrectly causes two POST_ROUTING hook invocations for DNATed packets that end up on the same bridge device. This happens because packets with a changed destination address are passed to dst_output() to make them go through the neighbour output function again to build a new destination MAC address, before they will continue through the IP hooks simulated by bridge netfilter.

The resulting hook order is:
PREROUTING (bridge netfilter)
POSTROUTING (dst_output -> ip_output)
FORWARD (bridge netfilter)
POSTROUTING (bridge netfilter)

The deferred hooks used to abort the first POST_ROUTING invocation, but since the only thing bridge netfilter actually really wants is a new MAC address, we can avoid going through the IP stack completely by simply calling the neighbour output function directly.

Tested, reported and lots of data provided by: Damien Thebault <[email protected]>

index fc13130..ce48d8c 100644
@@ -247,8 +247,9 @@ static void __br_dnat_complain(void)
  * Let us first consider the case that ip_route_input() succeeds:
  *
  * If skb->dst->dev equals the logical bridge device the packet
- * came in on, we can consider this bridging. We then call
- * skb->dst->output() which will make the packet enter br_nf_local_out()
+ * came in on, we can consider this bridging. The packet is passed
+ * through the neighbour output function to build a new destination
+ * MAC address, which will make the packet enter br_nf_local_out()
  * not much later. In that function it is assured that the iptables
  * FORWARD chain is traversed for the packet.
  *

@@ -285,12 +286,17 @@ static int br_nf_pre_routing_finish_bridge(struct sk_buff *skb)
 	skb->nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
 
 	skb->dev = bridge_parent(skb->dev);
-	if (!skb->dev)
-		kfree_skb(skb);
-	else {
+	if (skb->dev) {
+		struct dst_entry *dst = skb->dst;
+
 		nf_bridge_pull_encap_header(skb);
-		skb->dst->output(skb);
+
+		if (dst->hh)
+			return neigh_hh_output(dst->hh, skb);
+		else if (dst->neighbour)
+			return dst->neighbour->output(skb);
 	}
+	kfree_skb(skb);
 	return 0;
 }
 

This is a note to let you know that we have just queued up the patch titled

Subject: Netfilter: bridge: fix double POST_ROUTING invocation

to the 2.6.23-stable tree. Its filename is

netfilter-bridge-fix-double-post_routing-invocation.patch

A git repo of this tree can be found at
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

From: Patrick McHardy <[email protected]>
Date: Tue, 29 Jan 2008 19:08:25 +0100
Subject: Netfilter: bridge: fix double POST_ROUTING invocation
To: [email protected]
Cc: Netfilter Development Mailinglist <[email protected]>, "David S. Miller" <[email protected]>
Message-ID: <[email protected]>

From: Patrick McHardy <[email protected]>

[NETFILTER]: bridge: fix double POST_ROUTING invocation

Upstream commit 2948d2ebbb98747b912ac6d0c864b4d02be8a6f5

The bridge code incorrectly causes two POST_ROUTING hook invocations for DNATed packets that end up on the same bridge device. This happens because packets with a changed destination address are passed to dst_output() to make them go through the neighbour output function again to build a new destination MAC address, before they will continue through the IP hooks simulated by bridge netfilter.

The resulting hook order is:
PREROUTING (bridge netfilter)
POSTROUTING (dst_output -> ip_output)
FORWARD (bridge netfilter)
POSTROUTING (bridge netfilter)

The deferred hooks used to abort the first POST_ROUTING invocation, but since the only thing bridge netfilter actually really wants is a new MAC address, we can avoid going through the IP stack completely by simply calling the neighbour output function directly.

Tested, reported and lots of data provided by: Damien Thebault <[email protected]>

---
 net/bridge/br_netfilter.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

@@ -247,8 +247,9 @@ static void __br_dnat_complain(void)
  * Let us first consider the case that ip_route_input() succeeds:
  *
  * If skb->dst->dev equals the logical bridge device the packet
- * came in on, we can consider this bridging. We then call
- * skb->dst->output() which will make the packet enter br_nf_local_out()
+ * came in on, we can consider this bridging. The packet is passed
+ * through the neighbour output function to build a new destination
+ * MAC address, which will make the packet enter br_nf_local_out()
  * not much later. In that function it is assured that the iptables
  * FORWARD chain is traversed for the packet.
  *

@@ -285,12 +286,17 @@ static int br_nf_pre_routing_finish_brid
 	skb->nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
 
 	skb->dev = bridge_parent(skb->dev);
-	if (!skb->dev)
-		kfree_skb(skb);
-	else {
+	if (skb->dev) {
+		struct dst_entry *dst = skb->dst;
+
 		nf_bridge_pull_encap_header(skb);
-		skb->dst->output(skb);
+
+		if (dst->hh)
+			return neigh_hh_output(dst->hh, skb);
+		else if (dst->neighbour)
+			return dst->neighbour->output(skb);
 	}
+	kfree_skb(skb);
 	return 0;
 }
 

Patches currently in stable-queue which might be from [email protected] are

queue-2.6.23/netfilter-bridge-fix-double-post_routing-invocation.patch queue-2.6.23/netfilter-bridge-netfilter-fix-net_device-refcnt-leaks.patch