Date: Fri, 10 Sep 2021 21:27:59 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 141fe2dceeae - main - aio: Interlock with listen(2) Message-ID: <202109102127.18ALRx1g020689@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=141fe2dceeaeefaaffc2242c8652345a081e825a commit 141fe2dceeaeefaaffc2242c8652345a081e825a Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2021-09-10 21:21:11 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2021-09-10 21:21:11 +0000 aio: Interlock with listen(2) soo_aio_queue() did not handle the possibility that the provided socket is a listening socket. Up until recently, to fix this one would have to acquire the socket lock first and check, since the socket buffer locks were destroyed by listen(2). Now that the socket buffer locks belong to the socket, simply check SOLISTENING(so) after acquiring them, and make listen(2) return an error if any AIO jobs are enqueued on the socket. Add a couple of simple regression test cases. Note that this fixes things only for the default AIO implementation; cxgbe(4)'s TCP offload has a separate pru_aio_queue implementation which requires its own solution. Reported by: syzbot+c8aa122fa2c6a4e2a28b@syzkaller.appspotmail.com Reported by: syzbot+39af117d43d4f0faf512@syzkaller.appspotmail.com Reported by: syzbot+60cceb9569145a0b993b@syzkaller.appspotmail.com Reported by: syzbot+2d522c5db87710277ca5@syzkaller.appspotmail.com Reviewed by: tuexen, gallatin, jhb Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D31901 --- sys/kern/sys_socket.c | 12 +++++++- sys/kern/uipc_socket.c | 7 +++++ tests/sys/aio/aio_test.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c index e53b0367960b..83dc1cb2622b 100644 --- a/sys/kern/sys_socket.c +++ b/sys/kern/sys_socket.c @@ -808,18 +808,28 @@ soo_aio_queue(struct file *fp, struct kaiocb *job) if (error == 0) return (0); + /* Lock through the socket, since this may be a listening socket. */ switch (job->uaiocb.aio_lio_opcode & (LIO_WRITE | LIO_READ)) { case LIO_READ: sb = &so->so_rcv; + SOCK_RECVBUF_LOCK(so); break; case LIO_WRITE: sb = &so->so_snd; + SOCK_SENDBUF_LOCK(so); break; default: return (EINVAL); } - SOCKBUF_LOCK(sb); + if (SOLISTENING(so)) { + if (sb == &so->so_rcv) + SOCK_RECVBUF_UNLOCK(so); + else + SOCK_SENDBUF_UNLOCK(so); + return (EINVAL); + } + if (!aio_set_cancel_function(job, soo_aio_cancel)) panic("new job was cancelled"); TAILQ_INSERT_TAIL(&sb->sb_aiojobq, job, list); diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c index a502b06ce00e..cbddd80e0546 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -928,6 +928,13 @@ solisten_proto_check(struct socket *so) } mtx_lock(&so->so_snd_mtx); mtx_lock(&so->so_rcv_mtx); + + /* Interlock with soo_aio_queue(). */ + if ((so->so_snd.sb_flags & (SB_AIO | SB_AIO_RUNNING)) != 0 || + (so->so_rcv.sb_flags & (SB_AIO | SB_AIO_RUNNING)) != 0) { + solisten_proto_abort(so); + return (EINVAL); + } return (0); } diff --git a/tests/sys/aio/aio_test.c b/tests/sys/aio/aio_test.c index 1c694ad0c18b..a81cb906e38a 100644 --- a/tests/sys/aio/aio_test.c +++ b/tests/sys/aio/aio_test.c @@ -40,11 +40,12 @@ */ #include <sys/param.h> +#include <sys/mdioctl.h> #include <sys/module.h> #include <sys/resource.h> #include <sys/socket.h> #include <sys/stat.h> -#include <sys/mdioctl.h> +#include <sys/un.h> #include <aio.h> #include <err.h> @@ -1177,6 +1178,79 @@ ATF_TC_BODY(aio_socket_blocking_short_write_vectored, tc) aio_socket_blocking_short_write_test(true); } +/* + * Verify that AIO requests fail when applied to a listening socket. + */ +ATF_TC_WITHOUT_HEAD(aio_socket_listen_fail); +ATF_TC_BODY(aio_socket_listen_fail, tc) +{ + struct aiocb iocb; + struct sockaddr_un sun; + char buf[16]; + int s; + + s = socket(AF_LOCAL, SOCK_STREAM, 0); + ATF_REQUIRE(s != -1); + + memset(&sun, 0, sizeof(sun)); + snprintf(sun.sun_path, sizeof(sun.sun_path), "%s", "listen.XXXXXX"); + mktemp(sun.sun_path); + sun.sun_family = AF_LOCAL; + sun.sun_len = SUN_LEN(&sun); + + ATF_REQUIRE(bind(s, (struct sockaddr *)&sun, SUN_LEN(&sun)) == 0); + ATF_REQUIRE(listen(s, 5) == 0); + + memset(buf, 0, sizeof(buf)); + memset(&iocb, 0, sizeof(iocb)); + iocb.aio_fildes = s; + iocb.aio_buf = buf; + iocb.aio_nbytes = sizeof(buf); + + ATF_REQUIRE_ERRNO(EINVAL, aio_read(&iocb) == -1); + ATF_REQUIRE_ERRNO(EINVAL, aio_write(&iocb) == -1); + + ATF_REQUIRE(unlink(sun.sun_path) == 0); + close(s); +} + +/* + * Verify that listen(2) fails if a socket has pending AIO requests. + */ +ATF_TC_WITHOUT_HEAD(aio_socket_listen_pending); +ATF_TC_BODY(aio_socket_listen_pending, tc) +{ + struct aiocb iocb; + struct sockaddr_un sun; + char buf[16]; + int s; + + s = socket(AF_LOCAL, SOCK_STREAM, 0); + ATF_REQUIRE(s != -1); + + memset(&sun, 0, sizeof(sun)); + snprintf(sun.sun_path, sizeof(sun.sun_path), "%s", "listen.XXXXXX"); + mktemp(sun.sun_path); + sun.sun_family = AF_LOCAL; + sun.sun_len = SUN_LEN(&sun); + + ATF_REQUIRE(bind(s, (struct sockaddr *)&sun, SUN_LEN(&sun)) == 0); + + memset(buf, 0, sizeof(buf)); + memset(&iocb, 0, sizeof(iocb)); + iocb.aio_fildes = s; + iocb.aio_buf = buf; + iocb.aio_nbytes = sizeof(buf); + ATF_REQUIRE(aio_read(&iocb) == 0); + + ATF_REQUIRE_ERRNO(EINVAL, listen(s, 5) == -1); + + ATF_REQUIRE(aio_cancel(s, &iocb) != -1); + + ATF_REQUIRE(unlink(sun.sun_path) == 0); + close(s); +} + /* * This test verifies that cancelling a partially completed socket write * returns a short write rather than ECANCELED. @@ -1808,6 +1882,8 @@ ATF_TP_ADD_TCS(tp) ATF_TP_ADD_TC(tp, aio_socket_two_reads); ATF_TP_ADD_TC(tp, aio_socket_blocking_short_write); ATF_TP_ADD_TC(tp, aio_socket_blocking_short_write_vectored); + ATF_TP_ADD_TC(tp, aio_socket_listen_fail); + ATF_TP_ADD_TC(tp, aio_socket_listen_pending); ATF_TP_ADD_TC(tp, aio_socket_short_write_cancel); ATF_TP_ADD_TC(tp, aio_writev_dos_iov_len); ATF_TP_ADD_TC(tp, aio_writev_dos_iovcnt);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202109102127.18ALRx1g020689>