public inbox for [email protected]
 help / color / mirror / Atom feed
From: Pavel Begunkov <[email protected]>
To: Jens Axboe <[email protected]>, [email protected]
Subject: [PATCH 1/2] io_uring: fix dead-hung for non-iter fixed rw
Date: Sun, 24 Nov 2019 01:49:43 +0300	[thread overview]
Message-ID: <1633e8791ecd0661ba59302cc79c3847d56cc714.1574549055.git.asml.silence@gmail.com> (raw)
In-Reply-To: <[email protected]>

Read/write requests to devices without implemented read/write_iter
using fixed buffers causes general protection fault, which totally
hanged up a machine.

io_import_fixed() initialises iov_iter as a bvec, but loop_rw_iter()
tries to access its unitialised iov, so dereferencing random address.

Fix it by passing a userspace ptr instead in this case.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 60 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 8119cbae4fb6..566e987c6dab 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -172,6 +172,11 @@ struct io_mapped_ubuf {
 	unsigned int	nr_bvecs;
 };
 
+struct io_ubuf_iter {
+	struct iov_iter	it;
+	u64		ubuf;
+};
+
 struct fixed_file_table {
 	struct file		**files;
 };
@@ -1486,7 +1491,7 @@ static void kiocb_done(struct kiocb *kiocb, ssize_t ret, struct io_kiocb **nxt,
 
 static ssize_t io_import_fixed(struct io_ring_ctx *ctx, int rw,
 				const struct io_uring_sqe *sqe,
-				struct iov_iter *iter)
+				struct io_ubuf_iter *iter)
 {
 	size_t len = READ_ONCE(sqe->len);
 	struct io_mapped_ubuf *imu;
@@ -1513,12 +1518,14 @@ static ssize_t io_import_fixed(struct io_ring_ctx *ctx, int rw,
 	if (buf_addr < imu->ubuf || buf_addr + len > imu->ubuf + imu->len)
 		return -EFAULT;
 
+	iter->ubuf = buf_addr;
+
 	/*
 	 * May not be a start of buffer, set size appropriately
 	 * and advance us to the beginning.
 	 */
 	offset = buf_addr - imu->ubuf;
-	iov_iter_bvec(iter, rw, imu->bvec, imu->nr_bvecs, offset + len);
+	iov_iter_bvec(&iter->it, rw, imu->bvec, imu->nr_bvecs, offset + len);
 
 	if (offset) {
 		/*
@@ -1540,7 +1547,7 @@ static ssize_t io_import_fixed(struct io_ring_ctx *ctx, int rw,
 		const struct bio_vec *bvec = imu->bvec;
 
 		if (offset <= bvec->bv_len) {
-			iov_iter_advance(iter, offset);
+			iov_iter_advance(&iter->it, offset);
 		} else {
 			unsigned long seg_skip;
 
@@ -1548,25 +1555,27 @@ static ssize_t io_import_fixed(struct io_ring_ctx *ctx, int rw,
 			offset -= bvec->bv_len;
 			seg_skip = 1 + (offset >> PAGE_SHIFT);
 
-			iter->bvec = bvec + seg_skip;
-			iter->nr_segs -= seg_skip;
-			iter->count -= bvec->bv_len + offset;
-			iter->iov_offset = offset & ~PAGE_MASK;
+			iter->it.bvec = bvec + seg_skip;
+			iter->it.nr_segs -= seg_skip;
+			iter->it.count -= bvec->bv_len + offset;
+			iter->it.iov_offset = offset & ~PAGE_MASK;
 		}
 	}
 
-	return iter->count;
+	return iter->it.count;
 }
 
 static ssize_t io_import_iovec(struct io_ring_ctx *ctx, int rw,
 			       const struct sqe_submit *s, struct iovec **iovec,
-			       struct iov_iter *iter)
+			       struct io_ubuf_iter *iter)
 {
 	const struct io_uring_sqe *sqe = s->sqe;
 	void __user *buf = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	size_t sqe_len = READ_ONCE(sqe->len);
 	u8 opcode;
 
+	iter->ubuf = 0;
+
 	/*
 	 * We're reading ->opcode for the second time, but the first read
 	 * doesn't care whether it's _FIXED or not, so it doesn't matter
@@ -1587,10 +1596,10 @@ static ssize_t io_import_iovec(struct io_ring_ctx *ctx, int rw,
 #ifdef CONFIG_COMPAT
 	if (ctx->compat)
 		return compat_import_iovec(rw, buf, sqe_len, UIO_FASTIOV,
-						iovec, iter);
+						iovec, &iter->it);
 #endif
 
-	return import_iovec(rw, buf, sqe_len, UIO_FASTIOV, iovec, iter);
+	return import_iovec(rw, buf, sqe_len, UIO_FASTIOV, iovec, &iter->it);
 }
 
 /*
@@ -1598,7 +1607,7 @@ static ssize_t io_import_iovec(struct io_ring_ctx *ctx, int rw,
  * by looping over ->read() or ->write() manually.
  */
 static ssize_t loop_rw_iter(int rw, struct file *file, struct kiocb *kiocb,
-			   struct iov_iter *iter)
+			   struct io_ubuf_iter *iter)
 {
 	ssize_t ret = 0;
 
@@ -1612,10 +1621,17 @@ static ssize_t loop_rw_iter(int rw, struct file *file, struct kiocb *kiocb,
 	if (kiocb->ki_flags & IOCB_NOWAIT)
 		return -EAGAIN;
 
-	while (iov_iter_count(iter)) {
-		struct iovec iovec = iov_iter_iovec(iter);
+	while (iov_iter_count(&iter->it)) {
+		struct iovec iovec;
 		ssize_t nr;
 
+		if (iter_is_iovec(&iter->it)) {
+			iovec = iov_iter_iovec(&iter->it);
+		} else {
+			iovec.iov_base = (void __user *)iter->ubuf;
+			iovec.iov_len = iov_iter_count(&iter->it);
+		}
+
 		if (rw == READ) {
 			nr = file->f_op->read(file, iovec.iov_base,
 					      iovec.iov_len, &kiocb->ki_pos);
@@ -1630,9 +1646,11 @@ static ssize_t loop_rw_iter(int rw, struct file *file, struct kiocb *kiocb,
 			break;
 		}
 		ret += nr;
+		iter->ubuf += nr;
+
 		if (nr != iovec.iov_len)
 			break;
-		iov_iter_advance(iter, nr);
+		iov_iter_advance(&iter->it, nr);
 	}
 
 	return ret;
@@ -1643,7 +1661,7 @@ static int io_read(struct io_kiocb *req, struct io_kiocb **nxt,
 {
 	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
 	struct kiocb *kiocb = &req->rw;
-	struct iov_iter iter;
+	struct io_ubuf_iter iter;
 	struct file *file;
 	size_t iov_count;
 	ssize_t read_size, ret;
@@ -1664,13 +1682,13 @@ static int io_read(struct io_kiocb *req, struct io_kiocb **nxt,
 	if (req->flags & REQ_F_LINK)
 		req->result = read_size;
 
-	iov_count = iov_iter_count(&iter);
+	iov_count = iov_iter_count(&iter.it);
 	ret = rw_verify_area(READ, file, &kiocb->ki_pos, iov_count);
 	if (!ret) {
 		ssize_t ret2;
 
 		if (file->f_op->read_iter)
-			ret2 = call_read_iter(file, kiocb, &iter);
+			ret2 = call_read_iter(file, kiocb, &iter.it);
 		else
 			ret2 = loop_rw_iter(READ, file, kiocb, &iter);
 
@@ -1701,7 +1719,7 @@ static int io_write(struct io_kiocb *req, struct io_kiocb **nxt,
 {
 	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
 	struct kiocb *kiocb = &req->rw;
-	struct iov_iter iter;
+	struct io_ubuf_iter iter;
 	struct file *file;
 	size_t iov_count;
 	ssize_t ret;
@@ -1721,7 +1739,7 @@ static int io_write(struct io_kiocb *req, struct io_kiocb **nxt,
 	if (req->flags & REQ_F_LINK)
 		req->result = ret;
 
-	iov_count = iov_iter_count(&iter);
+	iov_count = iov_iter_count(&iter.it);
 
 	ret = -EAGAIN;
 	if (force_nonblock && !(kiocb->ki_flags & IOCB_DIRECT))
@@ -1747,7 +1765,7 @@ static int io_write(struct io_kiocb *req, struct io_kiocb **nxt,
 		kiocb->ki_flags |= IOCB_WRITE;
 
 		if (file->f_op->write_iter)
-			ret2 = call_write_iter(file, kiocb, &iter);
+			ret2 = call_write_iter(file, kiocb, &iter.it);
 		else
 			ret2 = loop_rw_iter(WRITE, file, kiocb, &iter);
 		if (!force_nonblock || ret2 != -EAGAIN)
-- 
2.24.0


  reply	other threads:[~2019-11-23 22:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-23 22:49 [RFC 0/2] fix in-kernel segfault Pavel Begunkov
2019-11-23 22:49 ` Pavel Begunkov [this message]
2019-11-23 22:49 ` [PATCH 2/2] io_uring: fix linked fixed !iter rw Pavel Begunkov
2019-11-23 22:53 ` [RFC 0/2] fix in-kernel segfault Jens Axboe
2019-11-23 23:08 ` Jens Axboe
2019-11-24  8:57   ` Pavel Begunkov
2019-11-24 16:36     ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1633e8791ecd0661ba59302cc79c3847d56cc714.1574549055.git.asml.silence@gmail.com \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox