git: 141fe2dceeae - main - aio: Interlock with listen(2)
Mark Johnston
markj at FreeBSD.org
Fri Sep 10 21:27:59 UTC 2021
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=141fe2dceeaeefaaffc2242c8652345a081e825a
commit 141fe2dceeaeefaaffc2242c8652345a081e825a
Author: Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-09-10 21:21:11 +0000
Commit: Mark Johnston <markj at 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 at syzkaller.appspotmail.com
Reported by: syzbot+39af117d43d4f0faf512 at syzkaller.appspotmail.com
Reported by: syzbot+60cceb9569145a0b993b at syzkaller.appspotmail.com
Reported by: syzbot+2d522c5db87710277ca5 at 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);
More information about the dev-commits-src-all
mailing list