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
next prev parent 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