* [PATCH] bridge: netfilter: orphan skb before invoking ip netfilter hooks
@ 2013-10-24 19:32 Florian Westphal
2013-10-24 20:14 ` Eric Dumazet
0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2013-10-24 19:32 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Pekka Pietikäinen reports xt_socket behavioural change after commit
00028aa37098o (netfilter: xt_socket: use IP early demux).
Reason is xt_socket now no longer does an unconditional sk lookup -
it re-uses existing skb->sk if possible, assuming ->sk was set by
tcp early demux.
However, when netfilter is invoked via bridge, this can cause 'bogus'
sockets to be examined by the match, e.g. a 'tun' device socket.
bridge netfilter should orphan the skb just like the routing path
before invoking ipv4/ipv6 netfilter hooks to avoid this.
Reported-and-tested-by: Pekka Pietikäinen <pp@ee.oulu.fi>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 878f008..80cad2c 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -559,6 +559,8 @@ static struct net_device *setup_pre_routing(struct sk_buff *skb)
else if (skb->protocol == htons(ETH_P_PPP_SES))
nf_bridge->mask |= BRNF_PPPoE;
+ /* Must drop socket now because of tproxy. */
+ skb_orphan(skb);
return skb->dev;
}
--
1.8.1.5
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] bridge: netfilter: orphan skb before invoking ip netfilter hooks
2013-10-24 19:32 [PATCH] bridge: netfilter: orphan skb before invoking ip netfilter hooks Florian Westphal
@ 2013-10-24 20:14 ` Eric Dumazet
2013-10-24 21:00 ` Florian Westphal
0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2013-10-24 20:14 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Thu, 2013-10-24 at 21:32 +0200, Florian Westphal wrote:
> Pekka Pietikäinen reports xt_socket behavioural change after commit
behavioral maybe ? ;)
> 00028aa37098o (netfilter: xt_socket: use IP early demux).
>
> Reason is xt_socket now no longer does an unconditional sk lookup -
> it re-uses existing skb->sk if possible, assuming ->sk was set by
> tcp early demux.
s/tcp/ip/ since it also can work for UDP sockets in net-next
>
> However, when netfilter is invoked via bridge, this can cause 'bogus'
> sockets to be examined by the match, e.g. a 'tun' device socket.
>
> bridge netfilter should orphan the skb just like the routing path
> before invoking ipv4/ipv6 netfilter hooks to avoid this.
>
> Reported-and-tested-by: Pekka Pietikäinen <pp@ee.oulu.fi>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index 878f008..80cad2c 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -559,6 +559,8 @@ static struct net_device *setup_pre_routing(struct sk_buff *skb)
> else if (skb->protocol == htons(ETH_P_PPP_SES))
> nf_bridge->mask |= BRNF_PPPoE;
>
> + /* Must drop socket now because of tproxy. */
> + skb_orphan(skb);
> return skb->dev;
> }
>
This was a nice one, thanks Florian !
Acked-by: Eric Dumazet <edumazet@google.com>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bridge: netfilter: orphan skb before invoking ip netfilter hooks
2013-10-24 20:14 ` Eric Dumazet
@ 2013-10-24 21:00 ` Florian Westphal
2013-10-27 20:46 ` Pablo Neira Ayuso
0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2013-10-24 21:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netfilter-devel
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2013-10-24 at 21:32 +0200, Florian Westphal wrote:
> > Pekka Pietikäinen reports xt_socket behavioural change after commit
>
> behavioral maybe ? ;)
AE vs. BE i guess. I grep'd /usr/share/dict/* before, i swear ;)
> > 00028aa37098o (netfilter: xt_socket: use IP early demux).
> >
> > Reason is xt_socket now no longer does an unconditional sk lookup -
> > it re-uses existing skb->sk if possible, assuming ->sk was set by
> > tcp early demux.
>
> s/tcp/ip/ since it also can work for UDP sockets in net-next
True. Pablo, could you amend this bit if you decide to apply this
to -next tree (i guess -next is fine)?
Thanks for reviewing this Eric!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bridge: netfilter: orphan skb before invoking ip netfilter hooks
2013-10-24 21:00 ` Florian Westphal
@ 2013-10-27 20:46 ` Pablo Neira Ayuso
0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2013-10-27 20:46 UTC (permalink / raw)
To: Florian Westphal; +Cc: Eric Dumazet, netfilter-devel
On Thu, Oct 24, 2013 at 11:00:23PM +0200, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
[...]
> > > it re-uses existing skb->sk if possible, assuming ->sk was set by
> > > tcp early demux.
> >
> > s/tcp/ip/ since it also can work for UDP sockets in net-next
>
> True. Pablo, could you amend this bit if you decide to apply this
> to -next tree (i guess -next is fine)?
Amended and applied, thanks Florian!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-27 20:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-24 19:32 [PATCH] bridge: netfilter: orphan skb before invoking ip netfilter hooks Florian Westphal
2013-10-24 20:14 ` Eric Dumazet
2013-10-24 21:00 ` Florian Westphal
2013-10-27 20:46 ` 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.