d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

aio: Interlock with listen(2)

Browse Source 
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
This commit is contained in:
Mark Johnston 2021-09-10 17:21:11 -04:00
parent 74a68313b5
commit 141fe2dcee
3 changed files with 95 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
sys/kern/sys_socket.c
View File

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

7
sys/kern/uipc_socket.c
View File

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

78
tests/sys/aio/aio_test.c
View File

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