* [PATCH] ceph: do not access the kiocb after aio reqeusts
@ 2020-07-06 12:51 xiubli
2020-07-06 16:17 ` Jeff Layton
0 siblings, 1 reply; 3+ messages in thread
From: xiubli @ 2020-07-06 12:51 UTC (permalink / raw)
To: jlayton; +Cc: idryomov, zyan, pdonnell, ceph-devel, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
In aio case, if the completion comes very fast just before the
ceph_read_iter() returns to fs/aio.c, the kiocb will be freed in
the completion callback, then if ceph_read_iter() access again
we will potentially hit the use-after-free bug.
URL: https://tracker.ceph.com/issues/45649
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
fs/ceph/file.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 160644ddaeed..704bae794054 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1538,6 +1538,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
struct inode *inode = file_inode(filp);
struct ceph_inode_info *ci = ceph_inode(inode);
struct page *pinned_page = NULL;
+ bool direct_lock = false;
ssize_t ret;
int want, got = 0;
int retry_op = 0, read = 0;
@@ -1546,10 +1547,12 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
dout("aio_read %p %llx.%llx %llu~%u trying to get caps on %p\n",
inode, ceph_vinop(inode), iocb->ki_pos, (unsigned)len, inode);
- if (iocb->ki_flags & IOCB_DIRECT)
+ if (iocb->ki_flags & IOCB_DIRECT) {
ceph_start_io_direct(inode);
- else
+ direct_lock = true;
+ } else {
ceph_start_io_read(inode);
+ }
if (fi->fmode & CEPH_FILE_MODE_LAZY)
want = CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO;
@@ -1603,7 +1606,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
}
ceph_put_cap_refs(ci, got);
- if (iocb->ki_flags & IOCB_DIRECT)
+ if (direct_lock)
ceph_end_io_direct(inode);
else
ceph_end_io_read(inode);
--
2.27.0.221.ga08a83d.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ceph: do not access the kiocb after aio reqeusts
2020-07-06 12:51 [PATCH] ceph: do not access the kiocb after aio reqeusts xiubli
@ 2020-07-06 16:17 ` Jeff Layton
2020-07-06 22:55 ` Xiubo Li
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2020-07-06 16:17 UTC (permalink / raw)
To: xiubli; +Cc: idryomov, zyan, pdonnell, ceph-devel
On Mon, 2020-07-06 at 08:51 -0400, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
>
> In aio case, if the completion comes very fast just before the
> ceph_read_iter() returns to fs/aio.c, the kiocb will be freed in
> the completion callback, then if ceph_read_iter() access again
> we will potentially hit the use-after-free bug.
>
> URL: https://tracker.ceph.com/issues/45649
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
> fs/ceph/file.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 160644ddaeed..704bae794054 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -1538,6 +1538,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
> struct inode *inode = file_inode(filp);
> struct ceph_inode_info *ci = ceph_inode(inode);
> struct page *pinned_page = NULL;
> + bool direct_lock = false;
Looks good. I made a slight change to this patch and had it initialize
this variable to iocb->ki_flags & IOCB_DIRECT, and then use that rather
than setting direct_lock in the true case. Merged into testing.
Thanks!
> ssize_t ret;
> int want, got = 0;
> int retry_op = 0, read = 0;
> @@ -1546,10 +1547,12 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
> dout("aio_read %p %llx.%llx %llu~%u trying to get caps on %p\n",
> inode, ceph_vinop(inode), iocb->ki_pos, (unsigned)len, inode);
>
> - if (iocb->ki_flags & IOCB_DIRECT)
> + if (iocb->ki_flags & IOCB_DIRECT) {
> ceph_start_io_direct(inode);
> - else
> + direct_lock = true;
> + } else {
> ceph_start_io_read(inode);
> + }
>
> if (fi->fmode & CEPH_FILE_MODE_LAZY)
> want = CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO;
> @@ -1603,7 +1606,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
> }
> ceph_put_cap_refs(ci, got);
>
> - if (iocb->ki_flags & IOCB_DIRECT)
> + if (direct_lock)
> ceph_end_io_direct(inode);
> else
> ceph_end_io_read(inode);
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ceph: do not access the kiocb after aio reqeusts
2020-07-06 16:17 ` Jeff Layton
@ 2020-07-06 22:55 ` Xiubo Li
0 siblings, 0 replies; 3+ messages in thread
From: Xiubo Li @ 2020-07-06 22:55 UTC (permalink / raw)
To: Jeff Layton; +Cc: idryomov, zyan, pdonnell, ceph-devel
On 2020/7/7 0:17, Jeff Layton wrote:
> On Mon, 2020-07-06 at 08:51 -0400, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> In aio case, if the completion comes very fast just before the
>> ceph_read_iter() returns to fs/aio.c, the kiocb will be freed in
>> the completion callback, then if ceph_read_iter() access again
>> we will potentially hit the use-after-free bug.
>>
>> URL: https://tracker.ceph.com/issues/45649
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
>> ---
>> fs/ceph/file.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> index 160644ddaeed..704bae794054 100644
>> --- a/fs/ceph/file.c
>> +++ b/fs/ceph/file.c
>> @@ -1538,6 +1538,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
>> struct inode *inode = file_inode(filp);
>> struct ceph_inode_info *ci = ceph_inode(inode);
>> struct page *pinned_page = NULL;
>> + bool direct_lock = false;
> Looks good. I made a slight change to this patch and had it initialize
> this variable to iocb->ki_flags & IOCB_DIRECT, and then use that rather
> than setting direct_lock in the true case. Merged into testing.
Okay, looks good to me.
Thanks Jeff.
> Thanks!
>
>> ssize_t ret;
>> int want, got = 0;
>> int retry_op = 0, read = 0;
>> @@ -1546,10 +1547,12 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
>> dout("aio_read %p %llx.%llx %llu~%u trying to get caps on %p\n",
>> inode, ceph_vinop(inode), iocb->ki_pos, (unsigned)len, inode);
>>
>> - if (iocb->ki_flags & IOCB_DIRECT)
>> + if (iocb->ki_flags & IOCB_DIRECT) {
>> ceph_start_io_direct(inode);
>> - else
>> + direct_lock = true;
>> + } else {
>> ceph_start_io_read(inode);
>> + }
>>
>> if (fi->fmode & CEPH_FILE_MODE_LAZY)
>> want = CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO;
>> @@ -1603,7 +1606,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
>> }
>> ceph_put_cap_refs(ci, got);
>>
>> - if (iocb->ki_flags & IOCB_DIRECT)
>> + if (direct_lock)
>> ceph_end_io_direct(inode);
>> else
>> ceph_end_io_read(inode);
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-06 22:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 12:51 [PATCH] ceph: do not access the kiocb after aio reqeusts xiubli
2020-07-06 16:17 ` Jeff Layton
2020-07-06 22:55 ` Xiubo Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.