| Message ID | 1294744462.2927.53.camel@edumazet-laptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jan 11, 2011 at 1:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le mercredi 05 janvier 2011 à 11:00 +0200, Daniel Baluta a écrit : > >> Going back to my first email, are there any follow ups on your >> "tcp: bind() fix when many ports are bound" patch. I've searched >> netdev archives but no luck. I might have missed something. >> >> I really appreciate your help. >> > > I believe following patch should solve the problem. I have tested the patch and it works. > > Thanks for reminding us this issue ! thanks, Daniel. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 11 Jan 2011 12:14:22 +0100 > [PATCH] tcp: disallow bind() to reuse addr/port > > inet_csk_bind_conflict() logic currently disallows a bind() if > it finds a friend socket (a socket bound on same address/port) > satisfying a set of conditions : > > 1) Current (to be bound) socket doesnt have sk_reuse set > OR > 2) other socket doesnt have sk_reuse set > OR > 3) other socket is in LISTEN state > > We should add the CLOSE state in the 3) condition, in order to avoid two > REUSEADDR sockets in CLOSE state with same local address/port, since > this can deny further operations. > > Note : a prior patch tried to address the problem in a different (and > buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports > are bound). > > Reported-by: Gaspar Chilingarov <gasparch@gmail.com> > Reported-by: Daniel Baluta <daniel.baluta@gmail.com> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 11, 2011 at 2:03 PM, David Miller <davem@davemloft.net> wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Tue, 11 Jan 2011 12:14:22 +0100 > >> [PATCH] tcp: disallow bind() to reuse addr/port >> >> inet_csk_bind_conflict() logic currently disallows a bind() if >> it finds a friend socket (a socket bound on same address/port) >> satisfying a set of conditions : >> >> 1) Current (to be bound) socket doesnt have sk_reuse set >> OR >> 2) other socket doesnt have sk_reuse set >> OR >> 3) other socket is in LISTEN state >> >> We should add the CLOSE state in the 3) condition, in order to avoid two >> REUSEADDR sockets in CLOSE state with same local address/port, since >> this can deny further operations. >> >> Note : a prior patch tried to address the problem in a different (and >> buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports >> are bound). >> >> Reported-by: Gaspar Chilingarov <gasparch@gmail.com> >> Reported-by: Daniel Baluta <daniel.baluta@gmail.com> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > Applied, thanks. > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > OK, just saw this, so please disregard my earlier. George -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mercredi 27 avril 2011 à 10:37 -0700, George B. a écrit : > On Tue, Jan 11, 2011 at 2:03 PM, David Miller <davem@davemloft.net> wrote: > > From: Eric Dumazet <eric.dumazet@gmail.com> > > Date: Tue, 11 Jan 2011 12:14:22 +0100 > > > >> [PATCH] tcp: disallow bind() to reuse addr/port > >> > >> inet_csk_bind_conflict() logic currently disallows a bind() if > >> it finds a friend socket (a socket bound on same address/port) > >> satisfying a set of conditions : > >> > >> 1) Current (to be bound) socket doesnt have sk_reuse set > >> OR > >> 2) other socket doesnt have sk_reuse set > >> OR > >> 3) other socket is in LISTEN state > >> > >> We should add the CLOSE state in the 3) condition, in order to avoid two > >> REUSEADDR sockets in CLOSE state with same local address/port, since > >> this can deny further operations. > >> > >> Note : a prior patch tried to address the problem in a different (and > >> buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports > >> are bound). > >> > >> Reported-by: Gaspar Chilingarov <gasparch@gmail.com> > >> Reported-by: Daniel Baluta <daniel.baluta@gmail.com> > >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > > > Applied, thanks. > > -- > > To unsubscribe from this list: send the line "unsubscribe netdev" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > OK, just saw this, so please disregard my earlier. Hmm... you'll discover this patch was reverted, because it broke some applications. So your problem remains. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >> OK, just saw this, so please disregard my earlier. > > Hmm... you'll discover this patch was reverted, because it broke some > applications. > > So your problem remains. Just to clarify, both the previous patch from last year *AND* this patch in January were reverted? I can't seem to find anything showing the new one being reverted and am now confused. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mercredi 27 avril 2011 à 10:54 -0700, George B. a écrit : > >> > >> OK, just saw this, so please disregard my earlier. > > > > Hmm... you'll discover this patch was reverted, because it broke some > > applications. > > > > So your problem remains. > > Just to clarify, both the previous patch from last year *AND* this > patch in January were reverted? I can't seem to find anything showing > the new one being reverted and am now confused. Yes, all patches were reverted. Last revert was very recent : 3e8c806a08c7beecd972e7ce15c http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=3e8c806a08c7beecd972e7ce15c570b9aba64baa Revert "tcp: disallow bind() to reuse addr/port" This reverts commit c191a836a908d1dd6b40c503741f91b914de3348. It causes known regressions for programs that expect to be able to use SO_REUSEADDR to shutdown a socket, then successfully rebind another socket to the same ID. Programs such as haproxy and amavisd expect this to work. This should fix kernel bugzilla 32832. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> It causes known regressions for programs that expect to be able to use > SO_REUSEADDR to shutdown a socket, then successfully rebind another > socket to the same ID. > > Programs such as haproxy and amavisd expect this to work. > > This should fix kernel bugzilla 32832. Thank you very much for the clarification. It just seems on the surface like it should be a simple problem (don't they all, at first?). Instead of checking to see if we have more than the number of ephemeral ports in use globally, see if we have more than that number in use on the requested IP address. The problem I am having is if the number of ports in use globally is greater than the number of configured ephemeral ports, I can't open a socket on a specific source IP even though that IP has plenty of ports available. It would seem like a simple bounds checking problem. Thanks again for taking the time to respond. George -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 25e3181..9f6d585 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -73,7 +73,7 @@ int inet_csk_bind_conflict(const struct sock *sk, !sk2->sk_bound_dev_if || sk->sk_bound_dev_if == sk2->sk_bound_dev_if)) { if (!reuse || !sk2->sk_reuse || - sk2->sk_state == TCP_LISTEN) { + ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) { const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2); if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) || sk2_rcv_saddr == sk_rcv_saddr(sk)) @@ -122,7 +122,8 @@ again: (tb->num_owners < smallest_size || smallest_size == -1)) { smallest_size = tb->num_owners; smallest_rover = rover; - if (atomic_read(&hashinfo->bsockets) > (high - low) + 1) { + if (atomic_read(&hashinfo->bsockets) > (high - low) + 1 && + !inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) { spin_unlock(&head->lock); snum = smallest_rover; goto have_snum; diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c index e46305d..d144e62 100644 --- a/net/ipv6/inet6_connection_sock.c +++ b/net/ipv6/inet6_connection_sock.c @@ -44,7 +44,7 @@ int inet6_csk_bind_conflict(const struct sock *sk, !sk2->sk_bound_dev_if || sk->sk_bound_dev_if == sk2->sk_bound_dev_if) && (!sk->sk_reuse || !sk2->sk_reuse || - sk2->sk_state == TCP_LISTEN) && + ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) && ipv6_rcv_saddr_equal(sk, sk2)) break; }