git: f6379f7fdec0 - main - socket: Fix a race between kevent(2) and listen(2)
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 16 Jun 2022 14:36:26 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=f6379f7fdec030e2ede75454cb61d25517606eaa
commit f6379f7fdec030e2ede75454cb61d25517606eaa
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-06-16 14:10:45 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-06-16 14:20:04 +0000
socket: Fix a race between kevent(2) and listen(2)
When locking the knote list for a socket, we check whether the socket is
a listening socket in order to select the appropriate mutex; a listening
socket uses the socket lock, while data sockets use socket buffer
mutexes.
If SOLISTENING(so) is false and the knote lock routine locks a socket
buffer, then it must re-check whether the socket is a listening socket
since solisten_proto() could have changed the socket's identity while we
were blocked on the socket buffer lock.
Reported by: syzkaller
Reviewed by: glebius
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D35492
---
sys/kern/uipc_socket.c | 36 ++++++++++++++++++++++++------------
sys/sys/socketvar.h | 5 +++++
2 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 2a01f656d258..a2241464e35b 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -4273,10 +4273,16 @@ so_rdknl_lock(void *arg)
{
struct socket *so = arg;
- if (SOLISTENING(so))
- SOCK_LOCK(so);
- else
+retry:
+ if (SOLISTENING(so)) {
+ SOLISTEN_LOCK(so);
+ } else {
SOCK_RECVBUF_LOCK(so);
+ if (__predict_false(SOLISTENING(so))) {
+ SOCK_RECVBUF_UNLOCK(so);
+ goto retry;
+ }
+ }
}
static void
@@ -4285,7 +4291,7 @@ so_rdknl_unlock(void *arg)
struct socket *so = arg;
if (SOLISTENING(so))
- SOCK_UNLOCK(so);
+ SOLISTEN_UNLOCK(so);
else
SOCK_RECVBUF_UNLOCK(so);
}
@@ -4297,12 +4303,12 @@ so_rdknl_assert_lock(void *arg, int what)
if (what == LA_LOCKED) {
if (SOLISTENING(so))
- SOCK_LOCK_ASSERT(so);
+ SOLISTEN_LOCK_ASSERT(so);
else
SOCK_RECVBUF_LOCK_ASSERT(so);
} else {
if (SOLISTENING(so))
- SOCK_UNLOCK_ASSERT(so);
+ SOLISTEN_UNLOCK_ASSERT(so);
else
SOCK_RECVBUF_UNLOCK_ASSERT(so);
}
@@ -4313,10 +4319,16 @@ so_wrknl_lock(void *arg)
{
struct socket *so = arg;
- if (SOLISTENING(so))
- SOCK_LOCK(so);
- else
+retry:
+ if (SOLISTENING(so)) {
+ SOLISTEN_LOCK(so);
+ } else {
SOCK_SENDBUF_LOCK(so);
+ if (__predict_false(SOLISTENING(so))) {
+ SOCK_SENDBUF_UNLOCK(so);
+ goto retry;
+ }
+ }
}
static void
@@ -4325,7 +4337,7 @@ so_wrknl_unlock(void *arg)
struct socket *so = arg;
if (SOLISTENING(so))
- SOCK_UNLOCK(so);
+ SOLISTEN_UNLOCK(so);
else
SOCK_SENDBUF_UNLOCK(so);
}
@@ -4337,12 +4349,12 @@ so_wrknl_assert_lock(void *arg, int what)
if (what == LA_LOCKED) {
if (SOLISTENING(so))
- SOCK_LOCK_ASSERT(so);
+ SOLISTEN_LOCK_ASSERT(so);
else
SOCK_SENDBUF_LOCK_ASSERT(so);
} else {
if (SOLISTENING(so))
- SOCK_UNLOCK_ASSERT(so);
+ SOLISTEN_UNLOCK_ASSERT(so);
else
SOCK_SENDBUF_UNLOCK_ASSERT(so);
}
diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h
index 249e0800f915..85aa7cbfea0f 100644
--- a/sys/sys/socketvar.h
+++ b/sys/sys/socketvar.h
@@ -254,6 +254,11 @@ struct socket {
KASSERT(SOLISTENING(sol), \
("%s: %p not listening", __func__, (sol))); \
} while (0)
+#define SOLISTEN_UNLOCK_ASSERT(sol) do { \
+ mtx_assert(&(sol)->so_lock, MA_NOTOWNED); \
+ KASSERT(SOLISTENING(sol), \
+ ("%s: %p not listening", __func__, (sol))); \
+} while (0)
/*
* Socket buffer locks. These are strongly preferred over SOCKBUF_LOCK(sb)