diff mbox

netfilter: xt_socket: add XT_SOCKET_NOWILDCARD flag causes behavioural change in userspace?

Message ID 5269121A.3040104@ee.oulu.fi
State Not Applicable
Headers show

Commit Message

Pekka Pietikäinen Oct. 24, 2013, 12:27 p.m. UTC
On 24/10/13 15:05, Eric Dumazet wrote:
> sk_state 7 means TCP_CLOSE
>
> I do not see how a TCP_CLOSE socket can be matched...
>
Yep, TCP_CLOSE can't be right, sk_state isn't correct with early demux 
perhaps?

Finding 
https://android.googlesource.com/kernel/common/+/experimental/android-3.8%5E!/ 
I managed to get  the old behaviour with the attached patch, but I'm 
having a hard time following what's really happening.

Comments

Florian Westphal Oct. 24, 2013, 12:51 p.m. UTC | #1
Pekka Pietikäinen <pp@ee.oulu.fi> wrote:
> On 24/10/13 15:05, Eric Dumazet wrote:
> >sk_state 7 means TCP_CLOSE
> >
> >I do not see how a TCP_CLOSE socket can be matched...
> >
> Yep, TCP_CLOSE can't be right, sk_state isn't correct with early
> demux perhaps?

What is weird is that early_demux should NOT influence xt_socket
because from the rules you posted you are using this in PREROUTING,
which is before tcp early demux magic.

Do you have any other netfilter rules (-j TPROXY perhaps?) that could
explain why the skb has a socket attached in the first place by
the time it ends up in the netfilter socket match?

[ ip_rcv() orphans the skb before netfilter prerouting, so skb->sk
  should be NULL ]
--
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
Florian Westphal Oct. 24, 2013, 7:14 p.m. UTC | #2
Pekka Pietikäinen <pp@ee.oulu.fi> wrote:
> On 24/10/13 16:29, Florian Westphal wrote:

[ restored nf-devel cc ]

> >I don't understand. The packet seen by xt_socket is arriving from
> >a remote machine, right?
> Virtual machine that's on virbr0 yep, and packet that used to match
> should be a synack to a connection initiated from the host.

Ah.  Now the picture is clear.  Underlying tun device in bridge,
which uses a socket internally (this is why skb->sk->sk_state is
TCP_CLOSE).

> bridge-nf-call-iptables = 0 and your patch + bridge-nf-call-iptable
> = 1 both made my test work with unmodified xt_socket (and adding
> debug prints shows sk_state is now sane)
> 
> Oh. Adding some more debug prints it's the
> "inet_sk(sk)->inet_rcv_saddr == 0" part that triggered.
> 
> Before 3.11 sk=nf_tproxy_get_sock_v4() was always done, so I suppose
> it was always working on sane data.

Yep.  When called from bridge netfilter the skb is not orphaned
properly, so the tun sk is still around by the time xt_socket is
consulted.

Thanks for reporting!
--
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
diff mbox

Patch

--- /usr/src/debug/kernel-3.11.fc19/linux-3.11.6-200.fc19.x86_64/net/netfilter/xt_socket.c	2013-09-02 23:46:10.000000000 +0300
+++ xt_socket.c	2013-10-24 15:07:59.592607433 +0300
@@ -115,6 +115,8 @@ 
 	struct nf_conn const *ct;
 	enum ip_conntrack_info ctinfo;
 #endif
+	if (sk && sk->sk_state == TCP_CLOSE)
+	  sk = NULL;
 
 	if (iph->protocol == IPPROTO_UDP || iph->protocol == IPPROTO_TCP) {
 		hp = skb_header_pointer(skb, ip_hdrlen(skb),