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