On Mon, 2020-07-06 at 08:51 -0400, xiubli@xxxxxxxxxx wrote: > From: Xiubo Li <xiubli@xxxxxxxxxx> > > 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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx>
