Netdev List
 help / color / mirror / Atom feed
From: Jason Baron <jbaron@akamai.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, iant@golang.org,
	Rainer Weikusat <rweikusat@mobileactivedefense.com>
Subject: [PATCH net] af_unix: ensure POLLOUT on remote close() for connected dgram sockets
Date: Mon, 16 Jul 2018 17:00:37 -0400	[thread overview]
Message-ID: <1531774837-27881-1-git-send-email-jbaron@akamai.com> (raw)

Applications use ECONNREFUSED as returned from write() in order to
determine that a socket should be closed. When using connected dgram
unix sockets in a poll/write loop, this relies on POLLOUT being
signaled when the remote end closes. However, due to a race POLLOUT
can be missed when the remote closes:

          thread 1 (client)                   thread 2 (server)

connect() to server
write() returns -EAGAIN
unix_dgram_poll()
 -> unix_recvq_full() is true
                                       close()
                                        ->unix_release_sock()
                                         ->wake_up_interruptible_all()
unix_dgram_poll() (due to the
     wake_up_interruptible_all)
 -> unix_recvq_full() still is true
                                         ->free all skbs


Now thread 1 is stuck and will not receive anymore wakeups. In this
case, when thread 1 gets the -EAGAIN, it has not queued any skbs
otherwise the 'free all skbs' step would in fact cause a wakeup and
a POLLOUT return. So the race here is probably fairly rare because
it means there are no skbs that thread 1 queued and that thread 1
runs before the 'free all skbs' step. Nevertheless, this has been
observed when the syslog daemon closes /dev/log. Tested against
a reproducer that re-creates the syslog hang.

The proposed fix is to move the wake_up_interruptible_all() call
after the 'free all skbs' step.

Reported-by: Ian Lance Taylor <iant@golang.org>
Cc: Rainer Weikusat <rweikusat@mobileactivedefense.com>
Signed-off-by: Jason Baron <jbaron@akamai.com>
---
 net/unix/af_unix.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e5473c0..de242cf 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -529,8 +529,6 @@ static void unix_release_sock(struct sock *sk, int embrion)
 	sk->sk_state = TCP_CLOSE;
 	unix_state_unlock(sk);
 
-	wake_up_interruptible_all(&u->peer_wait);
-
 	skpair = unix_peer(sk);
 
 	if (skpair != NULL) {
@@ -560,6 +558,9 @@ static void unix_release_sock(struct sock *sk, int embrion)
 		kfree_skb(skb);
 	}
 
+	/* after freeing skbs to make sure POLLOUT triggers */
+	wake_up_interruptible_all(&u->peer_wait);
+
 	if (path.dentry)
 		path_put(&path);
 
-- 
2.7.4

                 reply	other threads:[~2018-07-16 21:30 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1531774837-27881-1-git-send-email-jbaron@akamai.com \
    --to=jbaron@akamai.com \
    --cc=davem@davemloft.net \
    --cc=iant@golang.org \
    --cc=netdev@vger.kernel.org \
    --cc=rweikusat@mobileactivedefense.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox