Commit 4ea899ea authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Darrick J. Wong

iomap: fix a use after free in iomap_dio_rw

Introduce a local wait_for_completion variable to avoid an access to the
potentially freed dio struture after dropping the last reference count.

Also use the chance to document the completion behavior to make the
refcounting clear to the reader of the code.

Fixes: ff6a9292 ("iomap: implement direct I/O")
Reported-by: default avatarChandan Rajendra <chandan@linux.ibm.com>
Reported-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Tested-by: default avatarChandan Rajendra <chandan@linux.ibm.com>
Tested-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 8e47a457
...@@ -1813,6 +1813,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ...@@ -1813,6 +1813,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
loff_t pos = iocb->ki_pos, start = pos; loff_t pos = iocb->ki_pos, start = pos;
loff_t end = iocb->ki_pos + count - 1, ret = 0; loff_t end = iocb->ki_pos + count - 1, ret = 0;
unsigned int flags = IOMAP_DIRECT; unsigned int flags = IOMAP_DIRECT;
bool wait_for_completion = is_sync_kiocb(iocb);
struct blk_plug plug; struct blk_plug plug;
struct iomap_dio *dio; struct iomap_dio *dio;
...@@ -1832,7 +1833,6 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ...@@ -1832,7 +1833,6 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
dio->end_io = end_io; dio->end_io = end_io;
dio->error = 0; dio->error = 0;
dio->flags = 0; dio->flags = 0;
dio->wait_for_completion = is_sync_kiocb(iocb);
dio->submit.iter = iter; dio->submit.iter = iter;
dio->submit.waiter = current; dio->submit.waiter = current;
...@@ -1887,7 +1887,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ...@@ -1887,7 +1887,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
dio_warn_stale_pagecache(iocb->ki_filp); dio_warn_stale_pagecache(iocb->ki_filp);
ret = 0; ret = 0;
if (iov_iter_rw(iter) == WRITE && !dio->wait_for_completion && if (iov_iter_rw(iter) == WRITE && !wait_for_completion &&
!inode->i_sb->s_dio_done_wq) { !inode->i_sb->s_dio_done_wq) {
ret = sb_init_dio_done_wq(inode->i_sb); ret = sb_init_dio_done_wq(inode->i_sb);
if (ret < 0) if (ret < 0)
...@@ -1903,7 +1903,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ...@@ -1903,7 +1903,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
if (ret <= 0) { if (ret <= 0) {
/* magic error code to fall back to buffered I/O */ /* magic error code to fall back to buffered I/O */
if (ret == -ENOTBLK) { if (ret == -ENOTBLK) {
dio->wait_for_completion = true; wait_for_completion = true;
ret = 0; ret = 0;
} }
break; break;
...@@ -1925,8 +1925,24 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ...@@ -1925,8 +1925,24 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
if (dio->flags & IOMAP_DIO_WRITE_FUA) if (dio->flags & IOMAP_DIO_WRITE_FUA)
dio->flags &= ~IOMAP_DIO_NEED_SYNC; dio->flags &= ~IOMAP_DIO_NEED_SYNC;
/*
* We are about to drop our additional submission reference, which
* might be the last reference to the dio. There are three three
* different ways we can progress here:
*
* (a) If this is the last reference we will always complete and free
* the dio ourselves.
* (b) If this is not the last reference, and we serve an asynchronous
* iocb, we must never touch the dio after the decrement, the
* I/O completion handler will complete and free it.
* (c) If this is not the last reference, but we serve a synchronous
* iocb, the I/O completion handler will wake us up on the drop
* of the final reference, and we will complete and free it here
* after we got woken by the I/O completion handler.
*/
dio->wait_for_completion = wait_for_completion;
if (!atomic_dec_and_test(&dio->ref)) { if (!atomic_dec_and_test(&dio->ref)) {
if (!dio->wait_for_completion) if (!wait_for_completion)
return -EIOCBQUEUED; return -EIOCBQUEUED;
for (;;) { for (;;) {
...@@ -1943,9 +1959,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ...@@ -1943,9 +1959,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
__set_current_state(TASK_RUNNING); __set_current_state(TASK_RUNNING);
} }
ret = iomap_dio_complete(dio); return iomap_dio_complete(dio);
return ret;
out_free_dio: out_free_dio:
kfree(dio); kfree(dio);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment