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
This commit is contained in:
Mark Johnston 2022-06-16 10:10:45 -04:00
parent c262d5e877
commit f6379f7fde
2 changed files with 29 additions and 12 deletions

View File

@ -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);
}

View File

@ -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)