]>
Commit | Line | Data |
---|---|---|
7748c0ed SL |
1 | From 850e61e6ba4702fd5f7b8f55da1e30e5bcdd23e5 Mon Sep 17 00:00:00 2001 |
2 | From: Christoph Hellwig <hch@lst.de> | |
3 | Date: Thu, 17 Jan 2019 08:58:58 -0800 | |
4 | Subject: iomap: fix a use after free in iomap_dio_rw | |
5 | ||
6 | [ Upstream commit 4ea899ead2786a30aaa8181fefa81a3df4ad28f6 ] | |
7 | ||
8 | Introduce a local wait_for_completion variable to avoid an access to the | |
9 | potentially freed dio struture after dropping the last reference count. | |
10 | ||
11 | Also use the chance to document the completion behavior to make the | |
12 | refcounting clear to the reader of the code. | |
13 | ||
14 | Fixes: ff6a9292e6 ("iomap: implement direct I/O") | |
15 | Reported-by: Chandan Rajendra <chandan@linux.ibm.com> | |
16 | Reported-by: Darrick J. Wong <darrick.wong@oracle.com> | |
17 | Signed-off-by: Christoph Hellwig <hch@lst.de> | |
18 | Tested-by: Chandan Rajendra <chandan@linux.ibm.com> | |
19 | Tested-by: Darrick J. Wong <darrick.wong@oracle.com> | |
20 | Reviewed-by: Dave Chinner <dchinner@redhat.com> | |
21 | Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> | |
22 | Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> | |
23 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
24 | --- | |
25 | fs/iomap.c | 28 +++++++++++++++++++++------- | |
26 | 1 file changed, 21 insertions(+), 7 deletions(-) | |
27 | ||
28 | diff --git a/fs/iomap.c b/fs/iomap.c | |
29 | index 2e3e64012db7..fac45206418a 100644 | |
30 | --- a/fs/iomap.c | |
31 | +++ b/fs/iomap.c | |
32 | @@ -1787,6 +1787,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, | |
33 | loff_t pos = iocb->ki_pos, start = pos; | |
34 | loff_t end = iocb->ki_pos + count - 1, ret = 0; | |
35 | unsigned int flags = IOMAP_DIRECT; | |
36 | + bool wait_for_completion = is_sync_kiocb(iocb); | |
37 | struct blk_plug plug; | |
38 | struct iomap_dio *dio; | |
39 | ||
40 | @@ -1806,7 +1807,6 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, | |
41 | dio->end_io = end_io; | |
42 | dio->error = 0; | |
43 | dio->flags = 0; | |
44 | - dio->wait_for_completion = is_sync_kiocb(iocb); | |
45 | ||
46 | dio->submit.iter = iter; | |
47 | dio->submit.waiter = current; | |
48 | @@ -1861,7 +1861,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, | |
49 | dio_warn_stale_pagecache(iocb->ki_filp); | |
50 | ret = 0; | |
51 | ||
52 | - if (iov_iter_rw(iter) == WRITE && !dio->wait_for_completion && | |
53 | + if (iov_iter_rw(iter) == WRITE && !wait_for_completion && | |
54 | !inode->i_sb->s_dio_done_wq) { | |
55 | ret = sb_init_dio_done_wq(inode->i_sb); | |
56 | if (ret < 0) | |
57 | @@ -1877,7 +1877,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, | |
58 | if (ret <= 0) { | |
59 | /* magic error code to fall back to buffered I/O */ | |
60 | if (ret == -ENOTBLK) { | |
61 | - dio->wait_for_completion = true; | |
62 | + wait_for_completion = true; | |
63 | ret = 0; | |
64 | } | |
65 | break; | |
66 | @@ -1899,8 +1899,24 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, | |
67 | if (dio->flags & IOMAP_DIO_WRITE_FUA) | |
68 | dio->flags &= ~IOMAP_DIO_NEED_SYNC; | |
69 | ||
70 | + /* | |
71 | + * We are about to drop our additional submission reference, which | |
72 | + * might be the last reference to the dio. There are three three | |
73 | + * different ways we can progress here: | |
74 | + * | |
75 | + * (a) If this is the last reference we will always complete and free | |
76 | + * the dio ourselves. | |
77 | + * (b) If this is not the last reference, and we serve an asynchronous | |
78 | + * iocb, we must never touch the dio after the decrement, the | |
79 | + * I/O completion handler will complete and free it. | |
80 | + * (c) If this is not the last reference, but we serve a synchronous | |
81 | + * iocb, the I/O completion handler will wake us up on the drop | |
82 | + * of the final reference, and we will complete and free it here | |
83 | + * after we got woken by the I/O completion handler. | |
84 | + */ | |
85 | + dio->wait_for_completion = wait_for_completion; | |
86 | if (!atomic_dec_and_test(&dio->ref)) { | |
87 | - if (!dio->wait_for_completion) | |
88 | + if (!wait_for_completion) | |
89 | return -EIOCBQUEUED; | |
90 | ||
91 | for (;;) { | |
92 | @@ -1917,9 +1933,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, | |
93 | __set_current_state(TASK_RUNNING); | |
94 | } | |
95 | ||
96 | - ret = iomap_dio_complete(dio); | |
97 | - | |
98 | - return ret; | |
99 | + return iomap_dio_complete(dio); | |
100 | ||
101 | out_free_dio: | |
102 | kfree(dio); | |
103 | -- | |
104 | 2.19.1 | |
105 |