Re: [PATCH 5.6] io_uring: NULL-deref for IOSQE_{ASYNC,DRAIN}

[Pavel Begunkov <asml.silence@xxxxxxxxx>]

On 13/03/2020 23:28, Pavel Begunkov wrote:
> Hmm, found unreliably failing the across-fork test. I don't know whether it's
> this patch specific, but need to take a look there first.

It's good to go, just used outdated tests.
The reproducer is attached.

> 
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
>> ---
>>  fs/io_uring.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 55afae6f0cf4..9d43efbec960 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -4813,6 +4813,9 @@ static int io_req_defer_prep(struct io_kiocb *req,
>>  {
>>  	ssize_t ret = 0;
>>  
>> +	if (!sqe)
>> +		return 0;
>> +
>>  	if (io_op_defs[req->opcode].file_table) {
>>  		ret = io_grab_files(req);
>>  		if (unlikely(ret))
>> @@ -5655,6 +5658,11 @@ static bool io_submit_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe,
>>  		if (sqe_flags & (IOSQE_IO_LINK|IOSQE_IO_HARDLINK)) {
>>  			req->flags |= REQ_F_LINK;
>>  			INIT_LIST_HEAD(&req->link_list);
>> +
>> +			if (io_alloc_async_ctx(req)) {
>> +				ret = -EAGAIN;
>> +				goto err_req;
>> +			}
>>  			ret = io_req_defer_prep(req, sqe);
>>  			if (ret)
>>  				req->flags |= REQ_F_FAIL_LINK;
>>
> 

-- 
Pavel Begunkov

#include <errno.h>
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/poll.h>
#include <sys/eventfd.h>
#include "liburing.h"

#define BS		4096
static struct iovec vecs[1];
static int no_read;

static int create_file(const char *file)
{
	ssize_t ret;
	char *buf;
	int fd;

	buf = malloc(BS);
	memset(buf, 0xaa, BS);

	fd = open(file, O_WRONLY | O_CREAT, 0644);
	if (fd < 0) {
		perror("open file");
		return 1;
	}
	ret = write(fd, buf, BS);
	close(fd);
	return ret != BS;
}


static int create_buffers(void)
{
	if (posix_memalign(&vecs[0].iov_base, BS, BS))
		return 1;
	vecs[0].iov_len = BS;
	return 0;
}

static int test_io(const char *file)
{
	const int nr_links = 100;
	const int link_len = 100;
	const int nr_sqes = nr_links * link_len;
	struct io_uring_sqe *sqe;
	struct io_uring_cqe *cqe;
	struct io_uring ring;
	int i, fd, ret;
	static int warned;

	fd = open(file, O_WRONLY);
	if (fd < 0) {
		perror("file open");
		goto err;
	}

	ret = io_uring_queue_init(nr_sqes, &ring, 0);
	if (ret) {
		fprintf(stderr, "ring create failed: %d\n", ret);
		goto err;
	}

	for (int i = 0; i < nr_links; ++i) {
		for (int j = 0; j < link_len; ++j) {
			sqe = io_uring_get_sqe(&ring);
			if (!sqe) {
				fprintf(stderr, "sqe get failed\n");
				goto err;
			}
			io_uring_prep_writev(sqe, fd, &vecs[0], 1, 0);
			sqe->flags |= IOSQE_ASYNC;
			if (j != link_len - 1)
				sqe->flags |= IOSQE_IO_LINK;
		}
	}

	ret = io_uring_submit(&ring);
	if (ret != nr_sqes) {
		fprintf(stderr, "submit got %d, wanted %d\n", ret, nr_sqes);
		goto err;
	}

	for (i = 0; i < nr_sqes; i++) {
		ret = io_uring_wait_cqe(&ring, &cqe);
		if (ret) {
			fprintf(stderr, "wait_cqe=%d\n", ret);
			goto err;
		}
		if (cqe->res == -EINVAL) {
			if (!warned) {
				fprintf(stdout, "Non-vectored IO not "
					"supported, skipping\n");
				warned = 1;
				no_read = 1;
			}
		} else if (cqe->res != BS) {
			fprintf(stderr, "cqe res %d, wanted %d\n", cqe->res, BS);
			goto err;
		}
		io_uring_cqe_seen(&ring, cqe);
	}

	io_uring_queue_exit(&ring);
	close(fd);
	return 0;
err:
	if (fd != -1)
		close(fd);
	return 1;
}

int main(int argc, char *argv[])
{
	if (create_file(".basic-rw")) {
		fprintf(stderr, "file creation failed\n");
		goto err;
	}
	if (create_buffers()) {
		fprintf(stderr, "file creation failed\n");
		goto err;
	}


	test_io(".basic-rw");
	unlink(".basic-rw");
	return 0;
err:
	unlink(".basic-rw");
	return 1;
}