The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=f6379f7fdec030e2ede75454cb61d25517606eaa
commit f6379f7fdec030e2ede75454cb61d25517606eaa Author: Mark Johnston <[email protected]> AuthorDate: 2022-06-16 14:10:45 +0000 Commit: Mark Johnston <[email protected]> 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)
