Skip to contentSkip to conversation

New: shutdown(2) does not fully shut down socket any more

Bugme-new Andrew Morton 7 participants 13 messages Apr 12, 2011 – Apr 14, 2011 1 message not in the archive

Subsystems (3)
Networking Network drivers TCP
lore
Share & tools
lore

Andrew Morton

author
#1 lore raw
In reply to a message not in the archive

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 1 line · On Wed, 6 Apr 2011 22:42:38 GMT [email protected] wrote:

https://bugzilla.kernel.org/show_bug.cgi?id=32832

There is a tescase attached to this bugzilla report.

Quoted 48 lines
           Summary: shutdown(2) does not fully shut down socket any more
           Product: Networking
           Version: 2.5
    Kernel Version: 2.6.38
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: IPV4
        AssignedTo: [email protected]
        ReportedBy: [email protected]
        Regression: Yes

In 2.6.35 and earlier, shutdown(2) will fully remove a socket. This does not appear to be true any more and is causing software to misbehave.

2.6.35:

$ ./testcase
parent: 5957
before:
tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN     
after:
child: 5961
$ ./testcase
parent: 6001
before:
tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN     
after:
child: 6002

2.6.38:

$ ./testcase
parent: 1138
before:
tcp        0      0 0.0.0.0:12345           0.0.0.0:*               LISTEN     
after:
child: 1142
$ ./testcase
bind: Address already in use

The listener doesn't show up in netstat any more, but as long as the child process is running, the socket is unavailable. It is as if the shutdown(2) behavior has partially reverted to close(2) behavior (but in the case of using close(2), the child's socket would remain visible in netstat).

maintainer
#2 lore raw

From: Andrew Morton <[email protected]>
Date: Tue, 12 Apr 2011 16:15:56 -0700

Quoted 3 lines

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Stephen Hemminger forwarded this to the list last week, and Eric Dumazet is actively working on a fix.

author
#3 lore raw
Quoted 9 lines · On Tue, 12 Apr 2011 16:17:44 -0700 (PDT) David Miller <[email protected]>…

From: Andrew Morton <[email protected]>
Date: Tue, 12 Apr 2011 16:15:56 -0700

Quoted 3 lines

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Stephen Hemminger forwarded this to the list last week, and Eric Dumazet is actively working on a fix.

OK.

Please don't forward bugzilla reports! Instead do a reply-to-all and add the cc's. That way, bugzilla will capture the email discussion and you won't get akpmspammed.

Sigh, kernel bugzilla is a PITA. Some people do seem to like and expect it though.

#4 lore raw
In reply to David Miller
Quoted 9 lines · David Miller

From: Andrew Morton <[email protected]>
Date: Tue, 12 Apr 2011 16:15:56 -0700

Quoted 3 lines

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Stephen Hemminger forwarded this to the list last week, and Eric Dumazet is actively working on a fix.

I worked on it this week end to discover FreeBSD 8.1 would not allow several CLOSE sockets bound to same port even with REUSEADDR.

So haproxy claim is a bit wrong (its trick doesnt work on FreeBSD), and used an undocumented linux feature.

I feel this case is a call for SO_REUSEPORT, eventually.

http://www.unixguide.net/network/socketfaq/4.11.shtml

SO_REUSEADDR allows your server to bind to an address which is in a TIME_WAIT state. It does not allow more than one server to bind to the same address. It was mentioned that use of this flag can create a security risk because another server can bind to a the same port, by binding to a specific address as opposed to INADDR_ANY. The SO_REUSEPORT flag allows multiple processes to bind to the same address provided all of them use the SO_REUSEPORT option.

Since SO_REUSEPORT is not a 'stable fix', I suggest we revert the patch, and eventually work on SO_REUSEPORT on net-next-2.6

What do you think ?

#5 lore raw
Quoted 5 lines · Eric Dumazet

Since SO_REUSEPORT is not a 'stable fix', I suggest we revert the patch, and eventually work on SO_REUSEPORT on net-next-2.6

What do you think ?

Sorry, I should have mentioned commit id : c191a836a908d1dd6 (tcp: disallow bind() to reuse addr/port)

#11 lore raw
In reply to Eric Dumazet
Quoted 2 lines · Eric Dumazet

I worked on it this week end to discover FreeBSD 8.1 would not allow several CLOSE sockets bound to same port even with REUSEADDR.

Just to complete the information, yes it does, but only after a shutdown() call. And this is the use case of haproxy, amavisd (quoted in the bugzilla bug report), and others.

Quoted 2 lines

So haproxy claim is a bit wrong (its trick doesnt work on FreeBSD), and used an undocumented linux feature.

Both test cases (the one I provided to explain the haproxy issue and the one provided by Kees) are not about binding 2 sockets at the same time but binding a new socket after the first one has been shutdown.
Sadly this also looks undocumented on FreeBSD (only saw a reference on it in a code comment).

Quoted 4 lines

Since SO_REUSEPORT is not a 'stable fix', I suggest we revert the patch, and eventually work on SO_REUSEPORT on net-next-2.6

What do you think ?

Agree.

Many thanks for the time you already spent on that.

#12 lore raw
Quoted 7 lines · Cyril Bonté
Quoted 2 lines · Eric Dumazet

I worked on it this week end to discover FreeBSD 8.1 would not allow several CLOSE sockets bound to same port even with REUSEADDR.

Just to complete the information, yes it does, but only after a shutdown() call. And this is the use case of haproxy, amavisd (quoted in the bugzilla bug report), and others.

Yes, but after a shutdown(), FreeBSD doesnt allow a reuse of the socket. listen() is not available anymore. Its a bit like an unbind, or a full close().

#6 lore raw
In reply to Eric Dumazet

Hi Eric, Cyril,

Quoted 8 lines
Quoted 5 lines

Since SO_REUSEPORT is not a 'stable fix', I suggest we revert the patch, and eventually work on SO_REUSEPORT on net-next-2.6

What do you think ?

Sorry, I should have mentioned commit id : c191a836a908d1dd6 (tcp: disallow bind() to reuse addr/port)

Cyril's use case looks suspect. I don't think that this is a good reason for reverting this commit.

I would love to help on adding SO_REUSEPORT option in kernel.

thanks,
Daniel.

maintainer
#7 lore raw

From: Daniel Baluta <[email protected]>
Date: Wed, 13 Apr 2011 14:57:18 +0300

Quoted 2 lines

Cyril's use case looks suspect. I don't think that this is a good reason for reverting this commit.

I complete disagree.

Something that worked perfectly fine, probably for years, we broke.

We simply cannot do that, especially since we do not have a reasonable alternative at this time.

Adding SO_REUSEPORT is a long range option, and not something that will provide a fix for users right now.

So please don't even pretend to suggest that we shouldn't fix this with a revert unless a simple, obvious, kernel fix presents itself.

#8 lore raw
Quoted 10 lines · On Wed, 13 Apr 2011 10:43:17 -0700 (PDT) David Miller <[email protected]>…

From: Daniel Baluta <[email protected]>
Date: Wed, 13 Apr 2011 14:57:18 +0300

Quoted 2 lines

Cyril's use case looks suspect. I don't think that this is a good reason for reverting this commit.

I complete disagree.

Something that worked perfectly fine, probably for years, we broke.

Quoted 8 lines

We simply cannot do that, especially since we do not have a reasonable alternative at this time.

Adding SO_REUSEPORT is a long range option, and not something that will provide a fix for users right now.

So please don't even pretend to suggest that we shouldn't fix this with a revert unless a simple, obvious, kernel fix presents itself.

Just to echo what Dave said.
Even though the semantics of this is not documented in some standard, applications have been built on Linux expecting a certain behavior. If you want to change what happens in this case, you have to have a really good reason (like crash, security hole, or standards violation).

maintainer
#9 lore raw
In reply to Eric Dumazet

From: Eric Dumazet <[email protected]>
Date: Wed, 13 Apr 2011 05:00:08 +0200

Quoted 10 lines · Eric Dumazet
Quoted 6 lines

Since SO_REUSEPORT is not a 'stable fix', I suggest we revert the patch, and eventually work on SO_REUSEPORT on net-next-2.6

What do you think ?

Sorry, I should have mentioned commit id : c191a836a908d1dd6 (tcp: disallow bind() to reuse addr/port)

I'm commiting the revert as follows to net-2.6, and will queue it up for -stable as well:

--------------------
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.

Signed-off-by David S. Miller <[email protected]>
---
 net/ipv4/inet_connection_sock.c  |    5 ++---
 net/ipv6/inet6_connection_sock.c |    2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)
net/ipv4/inet_connection_sock.c +2 -3
index 6c0b7f4..38f23e7 100644
@@ -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 ||
-			    ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) {
+			    sk2->sk_state == TCP_LISTEN) {
 				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,8 +122,7 @@ 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 &&
-						    !inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
+						if (atomic_read(&hashinfo->bsockets) > (high - low) + 1) {
 							spin_unlock(&head->lock);
 							snum = smallest_rover;
 							goto have_snum;
net/ipv6/inet6_connection_sock.c +1 -1
index 1660546..f2c5b0f 100644
@@ -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 ||
-		     ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) &&
+		     sk2->sk_state == TCP_LISTEN) &&
 		     ipv6_rcv_saddr_equal(sk, sk2))
 			break;
 	}
Signature · 1 line
1.7.4.3
#10 lore raw
Quoted 26 lines · David Miller

From: Eric Dumazet <[email protected]>
Date: Wed, 13 Apr 2011 05:00:08 +0200

Quoted 10 lines · Eric Dumazet
Quoted 6 lines

Since SO_REUSEPORT is not a 'stable fix', I suggest we revert the patch, and eventually work on SO_REUSEPORT on net-next-2.6

What do you think ?

Sorry, I should have mentioned commit id : c191a836a908d1dd6 (tcp: disallow bind() to reuse addr/port)

I'm commiting the revert as follows to net-2.6, and will queue it up for -stable as well:

--------------------
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.

Quoted 6 lines

Programs such as haproxy and amavisd expect this to work.

This should fix kernel bugzilla 32832.

Signed-off-by David S. Miller <[email protected]>

---

Thanks David, I was planning to submit this revert this morning, you beat me ;)

#13 lore raw
In reply to Eric Dumazet
Quoted 30 lines · Eric Dumazet
Quoted 9 lines · David Miller

From: Andrew Morton <[email protected]>
Date: Tue, 12 Apr 2011 16:15:56 -0700

Quoted 3 lines

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Stephen Hemminger forwarded this to the list last week, and Eric Dumazet is actively working on a fix.

I worked on it this week end to discover FreeBSD 8.1 would not allow several CLOSE sockets bound to same port even with REUSEADDR.

So haproxy claim is a bit wrong (its trick doesnt work on FreeBSD), and used an undocumented linux feature.

I feel this case is a call for SO_REUSEPORT, eventually.

http://www.unixguide.net/network/socketfaq/4.11.shtml

SO_REUSEADDR allows your server to bind to an address which is in a TIME_WAIT state. It does not allow more than one server to bind to the same address. It was mentioned that use of this flag can create a security risk because another server can bind to a the same port, by binding to a specific address as opposed to INADDR_ANY. The SO_REUSEPORT flag allows multiple processes to bind to the same address provided all of them use the SO_REUSEPORT option.

Quoted 4 lines

Since SO_REUSEPORT is not a 'stable fix', I suggest we revert the patch, and eventually work on SO_REUSEPORT on net-next-2.6

What do you think ?

Not entirely related, but FWIW I think that SO_REUSEPORT would be rather useful for haproxy.

I've been working on allowing haproxy to be reconfigured without dropping or reusing connections. I have done this by re-using open sockets. But it would have been rather a lot easier to achieve using SO_REUSEPORT - assuming that I understand SO_REUSEPORT correctly.