: Fix skb->nf_bridge lifetime issues
Subsystems (3)
Bart, can you please have a look at this patch and ACK/NACK it? We have a bugreport in the netfilter bugzilla of broken conntrack with tunnels on top of bridge devices (#448), which should be cured by this patch.
There is also another report of broken conntrack with vlan on top of bridge devices (#400) that looks related, but probably this patch won't help.
[NETFILTER]: Fix skb->nf_bridge lifetime issues
The bridge netfilter code simulates the NF_IP_PRE_ROUTING hook and skips the real hook by registering with high priority and returning NF_STOP if skb->nf_bridge is present and the BRNF_NF_BRIDGE_PREROUTING flag is not set. The flag is only set during the simulated hook.
Because skb->nf_bridge is only freed when the packet is destroyed, the packet will not only skip the first invocation of NF_IP_PRE_ROUTING, but in the case of tunnel devices on top of the bridge also all further ones. Forwarded packets from a bridge encapsulated by a tunnel device and sent as locally outgoing packet will also still have the incorrect bridge information from the input path attached.
We already have nf_reset calls on all RX/TX paths of tunnel devices, so simply reset the nf_bridge field there too. As an added bonus, the bridge information for locally delivered packets is now also freed when the packet is queued to a socket.
---
commit 22e4dcb796d276db47a61c2382bdf2eaf76db32e
tree ccc5173acbe70506aa60e826e1881a4513f8c868
parent 337ba256a7e68f174a88ffba805b40622297fc22
author Patrick McHardy <[email protected]> Mon, 20 Feb 2006 08:41:48 +0100 committer Patrick McHardy <[email protected]> Mon, 20 Feb 2006 08:41:48 +0100
include/linux/skbuff.h | 24 ++++++++++++++---------- net/ipv4/netfilter/ipt_REJECT.c | 4 ---- 2 files changed, 14 insertions(+), 14 deletions(-)
include/linux/skbuff.h +14 -10
@@ -1351,16 +1351,6 @@ static inline void nf_conntrack_put_reaskfree_skb (skb); }#endif -static inline void nf_reset (struct sk_buff *skb) -{ -nf_conntrack_put (skb->nfct); - skb->nfct =NULL ; -#if defined(CONFIG_NF_CONNTRACK ) || defined(CONFIG_NF_CONNTRACK_MODULE ) -nf_conntrack_put_reasm (skb->nfct_reasm); - skb->nfct_reasm =NULL ; -#endif -} -#ifdef CONFIG_BRIDGE_NETFILTER static inline void nf_bridge_put (struct nf_bridge_info *nf_bridge) {
@@ -1373,6 +1363,20 @@ static inline void nf_bridge_get(structatomic_inc (&nf_bridge->use); }#endif /* CONFIG_BRIDGE_NETFILTER */ +static inline void nf_reset (struct sk_buff *skb) +{ +nf_conntrack_put (skb->nfct); + skb->nfct =NULL ; +#if defined(CONFIG_NF_CONNTRACK ) || defined(CONFIG_NF_CONNTRACK_MODULE ) +nf_conntrack_put_reasm (skb->nfct_reasm); + skb->nfct_reasm =NULL ; +#endif +#ifdef CONFIG_BRIDGE_NETFILTER +nf_bridge_put (skb->nfct); + skb->nfct =NULL ; +#endif +} +#else /* CONFIG_NETFILTER */ static inline void nf_reset (struct sk_buff *skb) {}#endif /* CONFIG_NETFILTER */
net/ipv4/netfilter/ipt_REJECT.c +0 -4
@@ -154,10 +154,6 @@ static void send_reset(struct sk_buff *o/* This packet will not be the same as the other: clear nf fields */ nf_reset (nskb); nskb->nfmark =0 ; -#ifdef CONFIG_BRIDGE_NETFILTER -nf_bridge_put (nskb->nf_bridge); - nskb->nf_bridge =NULL ; -#endif tcph = (struct tcphdr *)((u_int32_t *)nskb->nh.iph + nskb->nh.iph->ihl);
Quoted 8 lines · Patrick McHardy
Bart, can you please have a look at this patch and ACK/NACK it? We have a bugreport in the netfilter bugzilla of broken conntrack with tunnels on top of bridge devices (#448), which should be cured by this patch.
(quoted diff) +3
-0+static inline void nf_reset(struct sk_buff *skb) +{ + nf_conntrack_put(skb->nfct);
Review
Quoted 10 lines
(quoted diff) +10
-0+ skb->nfct = NULL; +#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE) + nf_conntrack_put_reasm(skb->nfct_reasm); + skb->nfct_reasm = NULL; +#endif +#ifdef CONFIG_BRIDGE_NETFILTER + nf_bridge_put(skb->nfct); + skb->nfct = NULL; +#endif +}
This time compile tested and s/nfct/nf_bridge/ above.
[NETFILTER]: Fix skb->nf_bridge lifetime issues
The bridge netfilter code simulates the NF_IP_PRE_ROUTING hook and skips the real hook by registering with high priority and returning NF_STOP if skb->nf_bridge is present and the BRNF_NF_BRIDGE_PREROUTING flag is not set. The flag is only set during the simulated hook.
Because skb->nf_bridge is only freed when the packet is destroyed, the packet will not only skip the first invocation of NF_IP_PRE_ROUTING, but in the case of tunnel devices on top of the bridge also all further ones. Forwarded packets from a bridge encapsulated by a tunnel device and sent as locally outgoing packet will also still have the incorrect bridge information from the input path attached.
We already have nf_reset calls on all RX/TX paths of tunnel devices, so simply reset the nf_bridge field there too. As an added bonus, the bridge information for locally delivered packets is now also freed when the packet is queued to a socket.
---
commit 5e5c34345f3ead2608e38d75f998dfeb7bb5df1c
tree 17bbdf2d4efc19888ec4fc0fd27c544677bd1949
parent 337ba256a7e68f174a88ffba805b40622297fc22
author Patrick McHardy <[email protected]> Mon, 20 Feb 2006 10:33:14 +0100 committer Patrick McHardy <[email protected]> Mon, 20 Feb 2006 10:33:14 +0100
include/linux/skbuff.h | 24 ++++++++++++++---------- net/ipv4/netfilter/ipt_REJECT.c | 4 ---- 2 files changed, 14 insertions(+), 14 deletions(-)
include/linux/skbuff.h +14 -10
@@ -1351,16 +1351,6 @@ static inline void nf_conntrack_put_reaskfree_skb (skb); }#endif -static inline void nf_reset (struct sk_buff *skb) -{ -nf_conntrack_put (skb->nfct); - skb->nfct =NULL ; -#if defined(CONFIG_NF_CONNTRACK ) || defined(CONFIG_NF_CONNTRACK_MODULE ) -nf_conntrack_put_reasm (skb->nfct_reasm); - skb->nfct_reasm =NULL ; -#endif -} -#ifdef CONFIG_BRIDGE_NETFILTER static inline void nf_bridge_put (struct nf_bridge_info *nf_bridge) {
@@ -1373,6 +1363,20 @@ static inline void nf_bridge_get(structatomic_inc (&nf_bridge->use); }#endif /* CONFIG_BRIDGE_NETFILTER */ +static inline void nf_reset (struct sk_buff *skb) +{ +nf_conntrack_put (skb->nfct); + skb->nfct =NULL ; +#if defined(CONFIG_NF_CONNTRACK ) || defined(CONFIG_NF_CONNTRACK_MODULE ) +nf_conntrack_put_reasm (skb->nfct_reasm); + skb->nfct_reasm =NULL ; +#endif +#ifdef CONFIG_BRIDGE_NETFILTER +nf_bridge_put (skb->nf_bridge); + skb->nf_bridge =NULL ; +#endif +} +#else /* CONFIG_NETFILTER */ static inline void nf_reset (struct sk_buff *skb) {}#endif /* CONFIG_NETFILTER */
net/ipv4/netfilter/ipt_REJECT.c +0 -4
@@ -154,10 +154,6 @@ static void send_reset(struct sk_buff *o/* This packet will not be the same as the other: clear nf fields */ nf_reset (nskb); nskb->nfmark =0 ; -#ifdef CONFIG_BRIDGE_NETFILTER -nf_bridge_put (nskb->nf_bridge); - nskb->nf_bridge =NULL ; -#endif tcph = (struct tcphdr *)((u_int32_t *)nskb->nh.iph + nskb->nh.iph->ihl);
Op ma, 20-02-2006 te 10:33 +0100, schreef Patrick McHardy:
Quoted 5 lines
Quoted 4 lines · Patrick McHardy
Bart, can you please have a look at this patch and ACK/NACK it? We have a bugreport in the netfilter bugzilla of broken conntrack with tunnels on top of bridge devices (#448), which should be cured by this patch.
Looks fine to me. Nice work.
cheers,
Bart
David S. Miller
maintainerFrom: Patrick McHardy <[email protected]>
Date: Mon, 20 Feb 2006 10:33:34 +0100
Quoted 1 line
This time compile tested and s/nfct/nf_bridge/ above.
Want me to push this into net-2.6 now that Bart has ACK'd it?
Thanks.
Quoted 8 lines · David S. Miller
From: Patrick McHardy <[email protected]>
Date: Mon, 20 Feb 2006 10:33:34 +0100Quoted 1 line
This time compile tested and s/nfct/nf_bridge/ above.
Want me to push this into net-2.6 now that Bart has ACK'd it?
I think net-2.6.17 is better, the bridging stuff is really to fragile for this to be sure it won't break anything and this has never worked so far.
David S. Miller
maintainerFrom: Patrick McHardy <[email protected]>
Date: Tue, 21 Feb 2006 05:17:04 +0100
Quoted 3 lines
I think net-2.6.17 is better, the bridging stuff is really to fragile for this to be sure it won't break anything and this has never worked so far.
Ok, done.