All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: bridge: fix routing of bridge frames with call-iptables=1
@ 2015-09-14  8:40 Florian Westphal
  2015-09-14 12:51 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 3+ messages in thread
From: Florian Westphal @ 2015-09-14  8:40 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

We need to (re-)init physoutdev to NULL, as its storage area is used
to stash the ip destination to detect L3 nat.

For frames that are bridged this is no problem because the bridge
forward hook initializes physoutdev to the real bridge port.

But in case the skb is delivered locally and then routed we can crash
in the physdev match since nf_bridge->physoutdev is garbage
(ipv4/ipv6 address)

Fixes: 72b1e5e4cac7 ("netfilter: bridge: reduce nf_bridge_info to 32 bytes again")
Reported-and-tested-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/bridge/br_netfilter_hooks.c | 10 +++++++++-
 net/bridge/br_netfilter_ipv6.c  |  7 ++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 33a82ff..824fbc8 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -355,6 +355,7 @@ static int br_nf_pre_routing_finish(struct sock *sk, struct sk_buff *skb)
 	struct iphdr *iph = ip_hdr(skb);
 	struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
 	struct rtable *rt;
+	bool daddr_changed;
 	int err;
 
 	nf_bridge->frag_max_size = IPCB(skb)->frag_max_size;
@@ -363,8 +364,15 @@ static int br_nf_pre_routing_finish(struct sock *sk, struct sk_buff *skb)
 		skb->pkt_type = PACKET_OTHERHOST;
 		nf_bridge->pkt_otherhost = false;
 	}
+
+	daddr_changed = br_nf_ipv4_daddr_was_changed(skb, nf_bridge);
+	/* init physoutdev to NULL. Its set by the bridge forward hook, but
+	 * frame might be routed instead of bridged.
+	 */
+	nf_bridge->physoutdev = NULL;
 	nf_bridge->in_prerouting = 0;
-	if (br_nf_ipv4_daddr_was_changed(skb, nf_bridge)) {
+
+	if (daddr_changed) {
 		if ((err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))) {
 			struct in_device *in_dev = __in_dev_get_rcu(dev);
 
diff --git a/net/bridge/br_netfilter_ipv6.c b/net/bridge/br_netfilter_ipv6.c
index 77383bf..772222b 100644
--- a/net/bridge/br_netfilter_ipv6.c
+++ b/net/bridge/br_netfilter_ipv6.c
@@ -167,6 +167,7 @@ static int br_nf_pre_routing_finish_ipv6(struct sock *sk, struct sk_buff *skb)
 	struct rtable *rt;
 	struct net_device *dev = skb->dev;
 	const struct nf_ipv6_ops *v6ops = nf_get_ipv6_ops();
+	bool daddr_changed;
 
 	nf_bridge->frag_max_size = IP6CB(skb)->frag_max_size;
 
@@ -174,8 +175,12 @@ static int br_nf_pre_routing_finish_ipv6(struct sock *sk, struct sk_buff *skb)
 		skb->pkt_type = PACKET_OTHERHOST;
 		nf_bridge->pkt_otherhost = false;
 	}
+
+	daddr_changed = br_nf_ipv6_daddr_was_changed(skb, nf_bridge);
+	nf_bridge->physoutdev = NULL;
 	nf_bridge->in_prerouting = 0;
-	if (br_nf_ipv6_daddr_was_changed(skb, nf_bridge)) {
+
+	if (daddr_changed) {
 		skb_dst_drop(skb);
 		v6ops->route_input(skb);
 
-- 
2.0.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] netfilter: bridge: fix routing of bridge frames with call-iptables=1
  2015-09-14  8:40 [PATCH nf] netfilter: bridge: fix routing of bridge frames with call-iptables=1 Florian Westphal
@ 2015-09-14 12:51 ` Pablo Neira Ayuso
  2015-09-14 21:38   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2015-09-14 12:51 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Mon, Sep 14, 2015 at 10:40:35AM +0200, Florian Westphal wrote:
> We need to (re-)init physoutdev to NULL, as its storage area is used
> to stash the ip destination to detect L3 nat.
> 
> For frames that are bridged this is no problem because the bridge
> forward hook initializes physoutdev to the real bridge port.
> 
> But in case the skb is delivered locally and then routed we can crash
> in the physdev match since nf_bridge->physoutdev is garbage
> (ipv4/ipv6 address)

Also applied, thanks

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] netfilter: bridge: fix routing of bridge frames with call-iptables=1
  2015-09-14 12:51 ` Pablo Neira Ayuso
@ 2015-09-14 21:38   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2015-09-14 21:38 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Mon, Sep 14, 2015 at 02:51:04PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Sep 14, 2015 at 10:40:35AM +0200, Florian Westphal wrote:
> > We need to (re-)init physoutdev to NULL, as its storage area is used
> > to stash the ip destination to detect L3 nat.
> > 
> > For frames that are bridged this is no problem because the bridge
> > forward hook initializes physoutdev to the real bridge port.
> > 
> > But in case the skb is delivered locally and then routed we can crash
> > in the physdev match since nf_bridge->physoutdev is garbage
> > (ipv4/ipv6 address)
> 
> Also applied, thanks

For the record, I have kept this back as your requested.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-09-14 21:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14  8:40 [PATCH nf] netfilter: bridge: fix routing of bridge frames with call-iptables=1 Florian Westphal
2015-09-14 12:51 ` Pablo Neira Ayuso
2015-09-14 21:38   ` Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.