From 156722b6d47c9fe1f453b34273b0540dae155082 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 13 Mar 2020 15:42:20 +0100 Subject: [PATCH 1/2] ovl: fix lockdep warning for async write ANBZ: #223 commit c853680453ac235e9010987a8bdaaba0e116d3c8 upstream. Lockdep reports "WARNING: lock held when returning to user space!" due to async write holding freeze lock over the write. Apparently aio.c already deals with this by lying to lockdep about the state of the lock. Do the same here. No need to check for S_IFREG() here since these file ops are regular-only. Reported-by: syzbot+9331a354f4f624a52a55@syzkaller.appspotmail.com Fixes: 2406a307ac7d ("ovl: implement async IO routines") Signed-off-by: Miklos Szeredi [joe: move file_start_write() down into sync/async branches] Signed-off-by: Joseph Qi Reviewed-by: Gao Xiang Reviewed-by: Xiaoguang Wang --- fs/overlayfs/file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 9242713596f4..45f83d9cac40 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -252,6 +252,9 @@ static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) if (iocb->ki_flags & IOCB_WRITE) { struct inode *inode = file_inode(orig_iocb->ki_filp); + /* Actually acquired in ovl_write_iter() */ + __sb_writers_acquired(file_inode(iocb->ki_filp)->i_sb, + SB_FREEZE_WRITE); file_end_write(iocb->ki_filp); ovl_copyattr(ovl_inode_real(inode), inode); } @@ -342,8 +345,8 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) ifl &= ~(IOCB_DSYNC | IOCB_SYNC); old_cred = ovl_override_creds(file_inode(file)->i_sb); - file_start_write(real.file); if (is_sync_kiocb(iocb)) { + file_start_write(real.file); ret = vfs_iter_write(real.file, iter, &iocb->ki_pos, ovl_iocb_to_rwf(ifl)); file_end_write(real.file); @@ -355,11 +358,14 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) GFP_NOFS); if (!aio_req) { ret = -ENOMEM; - file_end_write(real.file); fdput(real); goto out_revert; } + file_start_write(real.file); + /* Pacify lockdep, same trick as done in aio_write() */ + __sb_writers_release(file_inode(real.file)->i_sb, + SB_FREEZE_WRITE); aio_req->fd = real; aio_req->orig_iocb = iocb; kiocb_clone(&aio_req->iocb, iocb, real.file); -- Gitee From 5220a1c2d21ed49cb6afeb982c8acfd14ada3091 Mon Sep 17 00:00:00 2001 From: yangerkun Date: Thu, 30 Sep 2021 11:22:28 +0800 Subject: [PATCH 2/2] ovl: fix use after free in struct ovl_aio_req ANBZ: #223 commit 9a254403760041528bc8f69fe2f5e1ef86950991 upstream. Example for triggering use after free in a overlay on ext4 setup: aio_read ovl_read_iter vfs_iter_read ext4_file_read_iter ext4_dio_read_iter iomap_dio_rw -> -EIOCBQUEUED /* * Here IO is completed in a separate thread, * ovl_aio_cleanup_handler() frees aio_req which has iocb embedded */ file_accessed(iocb->ki_filp); /**BOOM**/ Fix by introducing a refcount in ovl_aio_req similarly to aio_kiocb. This guarantees that iocb is only freed after vfs_read/write_iter() returns on underlying fs. Fixes: 2406a307ac7d ("ovl: implement async IO routines") Signed-off-by: yangerkun Link: https://lore.kernel.org/r/20210930032228.3199690-3-yangerkun@huawei.com/ Cc: # v5.6 Signed-off-by: Miklos Szeredi Signed-off-by: Joseph Qi Reviewed-by: Gao Xiang Reviewed-by: Xiaoguang Wang --- fs/overlayfs/file.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 45f83d9cac40..db3dd10632bf 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -16,6 +16,7 @@ struct ovl_aio_req { struct kiocb iocb; + refcount_t ref; struct kiocb *orig_iocb; struct fd fd; }; @@ -244,6 +245,14 @@ static rwf_t ovl_iocb_to_rwf(int ifl) return flags; } +static inline void ovl_aio_put(struct ovl_aio_req *aio_req) +{ + if (refcount_dec_and_test(&aio_req->ref)) { + fdput(aio_req->fd); + kmem_cache_free(ovl_aio_request_cachep, aio_req); + } +} + static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) { struct kiocb *iocb = &aio_req->iocb; @@ -260,8 +269,7 @@ static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) } orig_iocb->ki_pos = iocb->ki_pos; - fdput(aio_req->fd); - kmem_cache_free(ovl_aio_request_cachep, aio_req); + ovl_aio_put(aio_req); } static void ovl_aio_rw_complete(struct kiocb *iocb, long res, long res2) @@ -307,7 +315,9 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) aio_req->orig_iocb = iocb; kiocb_clone(&aio_req->iocb, iocb, real.file); aio_req->iocb.ki_complete = ovl_aio_rw_complete; + refcount_set(&aio_req->ref, 2); ret = vfs_iocb_iter_read(real.file, &aio_req->iocb, iter); + ovl_aio_put(aio_req); ovl_file_accessed(file); if (ret != -EIOCBQUEUED) ovl_aio_cleanup_handler(aio_req); @@ -371,7 +381,9 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) kiocb_clone(&aio_req->iocb, iocb, real.file); aio_req->iocb.ki_flags = ifl; aio_req->iocb.ki_complete = ovl_aio_rw_complete; + refcount_set(&aio_req->ref, 2); ret = vfs_iocb_iter_write(real.file, &aio_req->iocb, iter); + ovl_aio_put(aio_req); if (ret != -EIOCBQUEUED) ovl_aio_cleanup_handler(aio_req); } -- Gitee