* [PATCH 0/2] ovl: bugfix for ovl_aio_req @ 2021-09-30 3:22 yangerkun 2021-09-30 3:22 ` [PATCH 1/2] ovl: factor out ovl_get_aio_req yangerkun 2021-09-30 3:22 ` [PATCH 2/2] ovl: fix UAF for ovl_aio_req yangerkun 0 siblings, 2 replies; 12+ messages in thread From: yangerkun @ 2021-09-30 3:22 UTC (permalink / raw) To: miklos, amir73il, jiufei.xue; +Cc: linux-unionfs, yangerkun, yukuai3 yangerkun (2): ovl: factor out ovl_get_aio_req ovl: fix UAF for ovl_aio_req fs/overlayfs/file.c | 51 ++++++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 14 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] ovl: factor out ovl_get_aio_req 2021-09-30 3:22 [PATCH 0/2] ovl: bugfix for ovl_aio_req yangerkun @ 2021-09-30 3:22 ` yangerkun 2021-09-30 3:22 ` [PATCH 2/2] ovl: fix UAF for ovl_aio_req yangerkun 1 sibling, 0 replies; 12+ messages in thread From: yangerkun @ 2021-09-30 3:22 UTC (permalink / raw) To: miklos, amir73il, jiufei.xue; +Cc: linux-unionfs, yangerkun, yukuai3 Factor out a common function to remove duplicate code in ovl_read_iter/ovl_write_iter. Signed-off-by: yangerkun <yangerkun@huawei.com> --- fs/overlayfs/file.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index d081faa55e83..4ac3cd698c7d 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -282,6 +282,23 @@ static void ovl_aio_rw_complete(struct kiocb *iocb, long res, long res2) orig_iocb->ki_complete(orig_iocb, res, res2); } +static struct ovl_aio_req *ovl_get_aio_req(struct kiocb *iocb, struct fd *real, int ifl) +{ + struct ovl_aio_req *req; + + req = kmem_cache_zalloc(ovl_aio_request_cachep, GFP_KERNEL); + if (!req) + return NULL; + + req->fd = *real; + real->flags = 0; + req->orig_iocb = iocb; + kiocb_clone(&req->iocb, iocb, real->file); + req->iocb.ki_flags = ifl; + req->iocb.ki_complete = ovl_aio_rw_complete; + return req; +} + static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) { struct file *file = iocb->ki_filp; @@ -304,15 +321,10 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) struct ovl_aio_req *aio_req; ret = -ENOMEM; - aio_req = kmem_cache_zalloc(ovl_aio_request_cachep, GFP_KERNEL); + aio_req = ovl_get_aio_req(iocb, &real, iocb->ki_flags); if (!aio_req) goto out; - aio_req->fd = real; - real.flags = 0; - aio_req->orig_iocb = iocb; - kiocb_clone(&aio_req->iocb, iocb, real.file); - aio_req->iocb.ki_complete = ovl_aio_rw_complete; ret = vfs_iocb_iter_read(real.file, &aio_req->iocb, iter); if (ret != -EIOCBQUEUED) ovl_aio_cleanup_handler(aio_req); @@ -364,7 +376,7 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) struct ovl_aio_req *aio_req; ret = -ENOMEM; - aio_req = kmem_cache_zalloc(ovl_aio_request_cachep, GFP_KERNEL); + aio_req = ovl_get_aio_req(iocb, &real, ifl); if (!aio_req) goto out; @@ -372,12 +384,6 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) /* 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; - real.flags = 0; - aio_req->orig_iocb = iocb; - kiocb_clone(&aio_req->iocb, iocb, real.file); - aio_req->iocb.ki_flags = ifl; - aio_req->iocb.ki_complete = ovl_aio_rw_complete; ret = vfs_iocb_iter_write(real.file, &aio_req->iocb, iter); if (ret != -EIOCBQUEUED) ovl_aio_cleanup_handler(aio_req); -- 2.31.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-09-30 3:22 [PATCH 0/2] ovl: bugfix for ovl_aio_req yangerkun 2021-09-30 3:22 ` [PATCH 1/2] ovl: factor out ovl_get_aio_req yangerkun @ 2021-09-30 3:22 ` yangerkun 2021-10-08 1:25 ` yangerkun 2021-10-08 15:13 ` Miklos Szeredi 1 sibling, 2 replies; 12+ messages in thread From: yangerkun @ 2021-09-30 3:22 UTC (permalink / raw) To: miklos, amir73il, jiufei.xue; +Cc: linux-unionfs, yangerkun, yukuai3 Our testcase trigger follow UAF: [ 153.939147] ================================================================== [ 153.942199] BUG: KASAN: use-after-free in ext4_dio_read_iter+0xcc/0x1a0 [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task fsstress/726 [ 153.948182] Call Trace: [ 153.948628] ? dump_stack_lvl+0x73/0x9f [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 [ 153.951693] ? kasan_report.cold+0x81/0x165 [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 [ 153.953190] ? __asan_load8+0x74/0x110 [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 [ 153.956916] ? ovl_read_iter+0x15f/0x270 [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 [ 153.958436] ? aio_setup_rw+0xbf/0xe0 [ 153.959101] ? aio_read+0x190/0x2d0 [ 153.959750] ? aio_write+0x3e0/0x3e0 [ 153.960404] ? __kasan_check_read+0x1d/0x30 [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 [ 153.961978] ? __put_user_ns+0x40/0x40 [ 153.962714] ? kasan_poison+0x40/0x90 [ 153.963384] ? set_alloc_info+0x46/0x70 [ 153.964102] ? __kasan_check_write+0x20/0x30 [ 153.964856] ? __fget_files+0x106/0x180 [ 153.965571] ? io_submit_one+0xaa7/0x1480 [ 153.966325] ? aio_poll_complete_work+0x590/0x590 [ 153.967713] ? ioctl_file_clone+0x110/0x110 [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 [ 153.975575] ? __kasan_check_read+0x1d/0x30 [ 153.976749] ? do_syscall_64+0x35/0x80 [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae [ 153.979436] [ 153.979738] Allocated by task 726: [ 153.980352] kasan_save_stack+0x23/0x60 [ 153.981054] set_alloc_info+0x46/0x70 [ 153.981745] __kasan_slab_alloc+0x4c/0x90 [ 153.982492] kmem_cache_alloc+0x153/0x750 [ 153.983953] ovl_read_iter+0x13b/0x270 [ 153.984689] aio_read+0x190/0x2d0 [ 153.985332] io_submit_one+0xaa7/0x1480 [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 [ 153.986878] do_syscall_64+0x35/0x80 [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 153.988516] [ 153.988824] Freed by task 750: [ 153.989417] kasan_save_stack+0x23/0x60 [ 153.990140] kasan_set_track+0x24/0x40 [ 153.990871] kasan_set_free_info+0x30/0x60 [ 153.991639] __kasan_slab_free+0x137/0x210 [ 153.992390] kmem_cache_free+0xf8/0x590 [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 [ 153.994205] ovl_aio_rw_complete+0x31/0x60 [ 153.995043] iomap_dio_complete_work+0x4b/0x60 [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 [ 153.996742] bio_endio+0x40d/0x440 [ 153.997376] blk_update_request+0x38f/0x820 [ 153.998160] scsi_end_request+0x56/0x320 [ 153.998872] scsi_io_completion+0x10a/0xb60 [ 153.999655] scsi_finish_command+0x194/0x2b0 [ 154.000464] scsi_complete+0xd7/0x1f0 [ 154.001184] blk_complete_reqs+0x92/0xb0 [ 154.001930] blk_done_softirq+0x29/0x40 [ 154.002696] __do_softirq+0x133/0x57f ext4_dio_read_iter ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) file_accessed(iocb->ki_filp) <== this trigger UAF Ret can be -EIOCBQUEUED which means we will not wait for io completion in iomap_dio_rw. So the release for ovl_aio_req will be done when io completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler in ovl_read_iter will do this). But once io finish soon, we may already release ovl_aio_req before we call file_access in ext4_dio_read_iter. This can trigger the upper UAF. Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. Fixes: 2406a307ac7d ("ovl: implement async IO routines") Signed-off-by: yangerkun <yangerkun@huawei.com> --- fs/overlayfs/file.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index 4ac3cd698c7d..0a0acab3bf2b 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -17,6 +17,7 @@ struct ovl_aio_req { struct kiocb iocb; + refcount_t ref; struct kiocb *orig_iocb; struct fd fd; }; @@ -252,6 +253,17 @@ 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)) + kmem_cache_free(ovl_aio_request_cachep, aio_req); +} + +static inline void ovl_aio_get(struct ovl_aio_req *aio_req) +{ + refcount_inc(&aio_req->ref); +} + static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) { struct kiocb *iocb = &aio_req->iocb; @@ -269,7 +281,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) @@ -296,6 +308,7 @@ static struct ovl_aio_req *ovl_get_aio_req(struct kiocb *iocb, struct fd *real, kiocb_clone(&req->iocb, iocb, real->file); req->iocb.ki_flags = ifl; req->iocb.ki_complete = ovl_aio_rw_complete; + refcount_set(&req->ref, 1); return req; } @@ -325,7 +338,9 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) if (!aio_req) goto out; + ovl_aio_get(aio_req); ret = vfs_iocb_iter_read(real.file, &aio_req->iocb, iter); + ovl_aio_put(aio_req); if (ret != -EIOCBQUEUED) ovl_aio_cleanup_handler(aio_req); } @@ -384,7 +399,9 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) /* Pacify lockdep, same trick as done in aio_write() */ __sb_writers_release(file_inode(real.file)->i_sb, SB_FREEZE_WRITE); + ovl_aio_get(aio_req); 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); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-09-30 3:22 ` [PATCH 2/2] ovl: fix UAF for ovl_aio_req yangerkun @ 2021-10-08 1:25 ` yangerkun 2021-10-08 15:13 ` Miklos Szeredi 1 sibling, 0 replies; 12+ messages in thread From: yangerkun @ 2021-10-08 1:25 UTC (permalink / raw) To: miklos, amir73il, jiufei.xue; +Cc: linux-unionfs, yukuai3 Gently ping... 在 2021/9/30 11:22, yangerkun 写道: > Our testcase trigger follow UAF: > > [ 153.939147] ================================================================== > [ 153.942199] BUG: KASAN: use-after-free in ext4_dio_read_iter+0xcc/0x1a0 > [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task fsstress/726 > [ 153.948182] Call Trace: > [ 153.948628] ? dump_stack_lvl+0x73/0x9f > [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.951693] ? kasan_report.cold+0x81/0x165 > [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.953190] ? __asan_load8+0x74/0x110 > [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 > [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 > [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 > [ 153.956916] ? ovl_read_iter+0x15f/0x270 > [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 > [ 153.958436] ? aio_setup_rw+0xbf/0xe0 > [ 153.959101] ? aio_read+0x190/0x2d0 > [ 153.959750] ? aio_write+0x3e0/0x3e0 > [ 153.960404] ? __kasan_check_read+0x1d/0x30 > [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 > [ 153.961978] ? __put_user_ns+0x40/0x40 > [ 153.962714] ? kasan_poison+0x40/0x90 > [ 153.963384] ? set_alloc_info+0x46/0x70 > [ 153.964102] ? __kasan_check_write+0x20/0x30 > [ 153.964856] ? __fget_files+0x106/0x180 > [ 153.965571] ? io_submit_one+0xaa7/0x1480 > [ 153.966325] ? aio_poll_complete_work+0x590/0x590 > [ 153.967713] ? ioctl_file_clone+0x110/0x110 > [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 > [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 > [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 > [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 > [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 > [ 153.975575] ? __kasan_check_read+0x1d/0x30 > [ 153.976749] ? do_syscall_64+0x35/0x80 > [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 153.979436] > [ 153.979738] Allocated by task 726: > [ 153.980352] kasan_save_stack+0x23/0x60 > [ 153.981054] set_alloc_info+0x46/0x70 > [ 153.981745] __kasan_slab_alloc+0x4c/0x90 > [ 153.982492] kmem_cache_alloc+0x153/0x750 > [ 153.983953] ovl_read_iter+0x13b/0x270 > [ 153.984689] aio_read+0x190/0x2d0 > [ 153.985332] io_submit_one+0xaa7/0x1480 > [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 > [ 153.986878] do_syscall_64+0x35/0x80 > [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 153.988516] > [ 153.988824] Freed by task 750: > [ 153.989417] kasan_save_stack+0x23/0x60 > [ 153.990140] kasan_set_track+0x24/0x40 > [ 153.990871] kasan_set_free_info+0x30/0x60 > [ 153.991639] __kasan_slab_free+0x137/0x210 > [ 153.992390] kmem_cache_free+0xf8/0x590 > [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 > [ 153.994205] ovl_aio_rw_complete+0x31/0x60 > [ 153.995043] iomap_dio_complete_work+0x4b/0x60 > [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 > [ 153.996742] bio_endio+0x40d/0x440 > [ 153.997376] blk_update_request+0x38f/0x820 > [ 153.998160] scsi_end_request+0x56/0x320 > [ 153.998872] scsi_io_completion+0x10a/0xb60 > [ 153.999655] scsi_finish_command+0x194/0x2b0 > [ 154.000464] scsi_complete+0xd7/0x1f0 > [ 154.001184] blk_complete_reqs+0x92/0xb0 > [ 154.001930] blk_done_softirq+0x29/0x40 > [ 154.002696] __do_softirq+0x133/0x57f > > ext4_dio_read_iter > ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) > file_accessed(iocb->ki_filp) <== this trigger UAF > > Ret can be -EIOCBQUEUED which means we will not wait for io completion in > iomap_dio_rw. So the release for ovl_aio_req will be done when io > completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler > in ovl_read_iter will do this). But once io finish soon, we may already > release ovl_aio_req before we call file_access in ext4_dio_read_iter. > This can trigger the upper UAF. > > Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. > > Fixes: 2406a307ac7d ("ovl: implement async IO routines") > Signed-off-by: yangerkun <yangerkun@huawei.com> > --- > fs/overlayfs/file.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c > index 4ac3cd698c7d..0a0acab3bf2b 100644 > --- a/fs/overlayfs/file.c > +++ b/fs/overlayfs/file.c > @@ -17,6 +17,7 @@ > > struct ovl_aio_req { > struct kiocb iocb; > + refcount_t ref; > struct kiocb *orig_iocb; > struct fd fd; > }; > @@ -252,6 +253,17 @@ 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)) > + kmem_cache_free(ovl_aio_request_cachep, aio_req); > +} > + > +static inline void ovl_aio_get(struct ovl_aio_req *aio_req) > +{ > + refcount_inc(&aio_req->ref); > +} > + > static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) > { > struct kiocb *iocb = &aio_req->iocb; > @@ -269,7 +281,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) > @@ -296,6 +308,7 @@ static struct ovl_aio_req *ovl_get_aio_req(struct kiocb *iocb, struct fd *real, > kiocb_clone(&req->iocb, iocb, real->file); > req->iocb.ki_flags = ifl; > req->iocb.ki_complete = ovl_aio_rw_complete; > + refcount_set(&req->ref, 1); > return req; > } > > @@ -325,7 +338,9 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) > if (!aio_req) > goto out; > > + ovl_aio_get(aio_req); > ret = vfs_iocb_iter_read(real.file, &aio_req->iocb, iter); > + ovl_aio_put(aio_req); > if (ret != -EIOCBQUEUED) > ovl_aio_cleanup_handler(aio_req); > } > @@ -384,7 +399,9 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) > /* Pacify lockdep, same trick as done in aio_write() */ > __sb_writers_release(file_inode(real.file)->i_sb, > SB_FREEZE_WRITE); > + ovl_aio_get(aio_req); > 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); > } > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-09-30 3:22 ` [PATCH 2/2] ovl: fix UAF for ovl_aio_req yangerkun 2021-10-08 1:25 ` yangerkun @ 2021-10-08 15:13 ` Miklos Szeredi 2021-10-18 14:26 ` yangerkun 1 sibling, 1 reply; 12+ messages in thread From: Miklos Szeredi @ 2021-10-08 15:13 UTC (permalink / raw) To: yangerkun; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: > > Our testcase trigger follow UAF: > > [ 153.939147] ================================================================== > [ 153.942199] BUG: KASAN: use-after-free in ext4_dio_read_iter+0xcc/0x1a0 > [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task fsstress/726 > [ 153.948182] Call Trace: > [ 153.948628] ? dump_stack_lvl+0x73/0x9f > [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.951693] ? kasan_report.cold+0x81/0x165 > [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.953190] ? __asan_load8+0x74/0x110 > [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 > [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 > [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 > [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 > [ 153.956916] ? ovl_read_iter+0x15f/0x270 > [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 > [ 153.958436] ? aio_setup_rw+0xbf/0xe0 > [ 153.959101] ? aio_read+0x190/0x2d0 > [ 153.959750] ? aio_write+0x3e0/0x3e0 > [ 153.960404] ? __kasan_check_read+0x1d/0x30 > [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 > [ 153.961978] ? __put_user_ns+0x40/0x40 > [ 153.962714] ? kasan_poison+0x40/0x90 > [ 153.963384] ? set_alloc_info+0x46/0x70 > [ 153.964102] ? __kasan_check_write+0x20/0x30 > [ 153.964856] ? __fget_files+0x106/0x180 > [ 153.965571] ? io_submit_one+0xaa7/0x1480 > [ 153.966325] ? aio_poll_complete_work+0x590/0x590 > [ 153.967713] ? ioctl_file_clone+0x110/0x110 > [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 > [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 > [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 > [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 > [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 > [ 153.975575] ? __kasan_check_read+0x1d/0x30 > [ 153.976749] ? do_syscall_64+0x35/0x80 > [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 153.979436] > [ 153.979738] Allocated by task 726: > [ 153.980352] kasan_save_stack+0x23/0x60 > [ 153.981054] set_alloc_info+0x46/0x70 > [ 153.981745] __kasan_slab_alloc+0x4c/0x90 > [ 153.982492] kmem_cache_alloc+0x153/0x750 > [ 153.983953] ovl_read_iter+0x13b/0x270 > [ 153.984689] aio_read+0x190/0x2d0 > [ 153.985332] io_submit_one+0xaa7/0x1480 > [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 > [ 153.986878] do_syscall_64+0x35/0x80 > [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 153.988516] > [ 153.988824] Freed by task 750: > [ 153.989417] kasan_save_stack+0x23/0x60 > [ 153.990140] kasan_set_track+0x24/0x40 > [ 153.990871] kasan_set_free_info+0x30/0x60 > [ 153.991639] __kasan_slab_free+0x137/0x210 > [ 153.992390] kmem_cache_free+0xf8/0x590 > [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 > [ 153.994205] ovl_aio_rw_complete+0x31/0x60 > [ 153.995043] iomap_dio_complete_work+0x4b/0x60 > [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 > [ 153.996742] bio_endio+0x40d/0x440 > [ 153.997376] blk_update_request+0x38f/0x820 > [ 153.998160] scsi_end_request+0x56/0x320 > [ 153.998872] scsi_io_completion+0x10a/0xb60 > [ 153.999655] scsi_finish_command+0x194/0x2b0 > [ 154.000464] scsi_complete+0xd7/0x1f0 > [ 154.001184] blk_complete_reqs+0x92/0xb0 > [ 154.001930] blk_done_softirq+0x29/0x40 > [ 154.002696] __do_softirq+0x133/0x57f > > ext4_dio_read_iter > ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) > file_accessed(iocb->ki_filp) <== this trigger UAF > > Ret can be -EIOCBQUEUED which means we will not wait for io completion in > iomap_dio_rw. So the release for ovl_aio_req will be done when io > completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler > in ovl_read_iter will do this). But once io finish soon, we may already > release ovl_aio_req before we call file_access in ext4_dio_read_iter. > This can trigger the upper UAF. > > Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. Looks good at a glance. Will review more fully. Thanks, MIklos ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-10-08 15:13 ` Miklos Szeredi @ 2021-10-18 14:26 ` yangerkun 2021-10-28 12:41 ` yangerkun 0 siblings, 1 reply; 12+ messages in thread From: yangerkun @ 2021-10-18 14:26 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 在 2021/10/8 23:13, Miklos Szeredi 写道: > On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: >> >> Our testcase trigger follow UAF: >> >> [ 153.939147] ================================================================== >> [ 153.942199] BUG: KASAN: use-after-free in ext4_dio_read_iter+0xcc/0x1a0 >> [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task fsstress/726 >> [ 153.948182] Call Trace: >> [ 153.948628] ? dump_stack_lvl+0x73/0x9f >> [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 >> [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 >> [ 153.951693] ? kasan_report.cold+0x81/0x165 >> [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 >> [ 153.953190] ? __asan_load8+0x74/0x110 >> [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 >> [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 >> [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 >> [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 >> [ 153.956916] ? ovl_read_iter+0x15f/0x270 >> [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 >> [ 153.958436] ? aio_setup_rw+0xbf/0xe0 >> [ 153.959101] ? aio_read+0x190/0x2d0 >> [ 153.959750] ? aio_write+0x3e0/0x3e0 >> [ 153.960404] ? __kasan_check_read+0x1d/0x30 >> [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 >> [ 153.961978] ? __put_user_ns+0x40/0x40 >> [ 153.962714] ? kasan_poison+0x40/0x90 >> [ 153.963384] ? set_alloc_info+0x46/0x70 >> [ 153.964102] ? __kasan_check_write+0x20/0x30 >> [ 153.964856] ? __fget_files+0x106/0x180 >> [ 153.965571] ? io_submit_one+0xaa7/0x1480 >> [ 153.966325] ? aio_poll_complete_work+0x590/0x590 >> [ 153.967713] ? ioctl_file_clone+0x110/0x110 >> [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 >> [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 >> [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 >> [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 >> [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 >> [ 153.975575] ? __kasan_check_read+0x1d/0x30 >> [ 153.976749] ? do_syscall_64+0x35/0x80 >> [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae >> [ 153.979436] >> [ 153.979738] Allocated by task 726: >> [ 153.980352] kasan_save_stack+0x23/0x60 >> [ 153.981054] set_alloc_info+0x46/0x70 >> [ 153.981745] __kasan_slab_alloc+0x4c/0x90 >> [ 153.982492] kmem_cache_alloc+0x153/0x750 >> [ 153.983953] ovl_read_iter+0x13b/0x270 >> [ 153.984689] aio_read+0x190/0x2d0 >> [ 153.985332] io_submit_one+0xaa7/0x1480 >> [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 >> [ 153.986878] do_syscall_64+0x35/0x80 >> [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae >> [ 153.988516] >> [ 153.988824] Freed by task 750: >> [ 153.989417] kasan_save_stack+0x23/0x60 >> [ 153.990140] kasan_set_track+0x24/0x40 >> [ 153.990871] kasan_set_free_info+0x30/0x60 >> [ 153.991639] __kasan_slab_free+0x137/0x210 >> [ 153.992390] kmem_cache_free+0xf8/0x590 >> [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 >> [ 153.994205] ovl_aio_rw_complete+0x31/0x60 >> [ 153.995043] iomap_dio_complete_work+0x4b/0x60 >> [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 >> [ 153.996742] bio_endio+0x40d/0x440 >> [ 153.997376] blk_update_request+0x38f/0x820 >> [ 153.998160] scsi_end_request+0x56/0x320 >> [ 153.998872] scsi_io_completion+0x10a/0xb60 >> [ 153.999655] scsi_finish_command+0x194/0x2b0 >> [ 154.000464] scsi_complete+0xd7/0x1f0 >> [ 154.001184] blk_complete_reqs+0x92/0xb0 >> [ 154.001930] blk_done_softirq+0x29/0x40 >> [ 154.002696] __do_softirq+0x133/0x57f >> >> ext4_dio_read_iter >> ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) >> file_accessed(iocb->ki_filp) <== this trigger UAF >> >> Ret can be -EIOCBQUEUED which means we will not wait for io completion in >> iomap_dio_rw. So the release for ovl_aio_req will be done when io >> completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler >> in ovl_read_iter will do this). But once io finish soon, we may already >> release ovl_aio_req before we call file_access in ext4_dio_read_iter. >> This can trigger the upper UAF. >> >> Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. > > Looks good at a glance. Will review more fully. Hi, Any comments for this patch? :) > > Thanks, > MIklos > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-10-18 14:26 ` yangerkun @ 2021-10-28 12:41 ` yangerkun 2021-10-29 8:35 ` Miklos Szeredi 2021-10-29 11:40 ` Miklos Szeredi 0 siblings, 2 replies; 12+ messages in thread From: yangerkun @ 2021-10-28 12:41 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 在 2021/10/18 22:26, yangerkun 写道: > > > 在 2021/10/8 23:13, Miklos Szeredi 写道: >> On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: >>> >>> Our testcase trigger follow UAF: >>> >>> [ 153.939147] >>> ================================================================== >>> [ 153.942199] BUG: KASAN: use-after-free in >>> ext4_dio_read_iter+0xcc/0x1a0 >>> [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task >>> fsstress/726 >>> [ 153.948182] Call Trace: >>> [ 153.948628] ? dump_stack_lvl+0x73/0x9f >>> [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 >>> [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 >>> [ 153.951693] ? kasan_report.cold+0x81/0x165 >>> [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 >>> [ 153.953190] ? __asan_load8+0x74/0x110 >>> [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 >>> [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 >>> [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 >>> [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 >>> [ 153.956916] ? ovl_read_iter+0x15f/0x270 >>> [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 >>> [ 153.958436] ? aio_setup_rw+0xbf/0xe0 >>> [ 153.959101] ? aio_read+0x190/0x2d0 >>> [ 153.959750] ? aio_write+0x3e0/0x3e0 >>> [ 153.960404] ? __kasan_check_read+0x1d/0x30 >>> [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 >>> [ 153.961978] ? __put_user_ns+0x40/0x40 >>> [ 153.962714] ? kasan_poison+0x40/0x90 >>> [ 153.963384] ? set_alloc_info+0x46/0x70 >>> [ 153.964102] ? __kasan_check_write+0x20/0x30 >>> [ 153.964856] ? __fget_files+0x106/0x180 >>> [ 153.965571] ? io_submit_one+0xaa7/0x1480 >>> [ 153.966325] ? aio_poll_complete_work+0x590/0x590 >>> [ 153.967713] ? ioctl_file_clone+0x110/0x110 >>> [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 >>> [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 >>> [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 >>> [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 >>> [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 >>> [ 153.975575] ? __kasan_check_read+0x1d/0x30 >>> [ 153.976749] ? do_syscall_64+0x35/0x80 >>> [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae >>> [ 153.979436] >>> [ 153.979738] Allocated by task 726: >>> [ 153.980352] kasan_save_stack+0x23/0x60 >>> [ 153.981054] set_alloc_info+0x46/0x70 >>> [ 153.981745] __kasan_slab_alloc+0x4c/0x90 >>> [ 153.982492] kmem_cache_alloc+0x153/0x750 >>> [ 153.983953] ovl_read_iter+0x13b/0x270 >>> [ 153.984689] aio_read+0x190/0x2d0 >>> [ 153.985332] io_submit_one+0xaa7/0x1480 >>> [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 >>> [ 153.986878] do_syscall_64+0x35/0x80 >>> [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae >>> [ 153.988516] >>> [ 153.988824] Freed by task 750: >>> [ 153.989417] kasan_save_stack+0x23/0x60 >>> [ 153.990140] kasan_set_track+0x24/0x40 >>> [ 153.990871] kasan_set_free_info+0x30/0x60 >>> [ 153.991639] __kasan_slab_free+0x137/0x210 >>> [ 153.992390] kmem_cache_free+0xf8/0x590 >>> [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 >>> [ 153.994205] ovl_aio_rw_complete+0x31/0x60 >>> [ 153.995043] iomap_dio_complete_work+0x4b/0x60 >>> [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 >>> [ 153.996742] bio_endio+0x40d/0x440 >>> [ 153.997376] blk_update_request+0x38f/0x820 >>> [ 153.998160] scsi_end_request+0x56/0x320 >>> [ 153.998872] scsi_io_completion+0x10a/0xb60 >>> [ 153.999655] scsi_finish_command+0x194/0x2b0 >>> [ 154.000464] scsi_complete+0xd7/0x1f0 >>> [ 154.001184] blk_complete_reqs+0x92/0xb0 >>> [ 154.001930] blk_done_softirq+0x29/0x40 >>> [ 154.002696] __do_softirq+0x133/0x57f >>> >>> ext4_dio_read_iter >>> ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) >>> file_accessed(iocb->ki_filp) <== this trigger UAF >>> >>> Ret can be -EIOCBQUEUED which means we will not wait for io >>> completion in >>> iomap_dio_rw. So the release for ovl_aio_req will be done when io >>> completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler >>> in ovl_read_iter will do this). But once io finish soon, we may already >>> release ovl_aio_req before we call file_access in ext4_dio_read_iter. >>> This can trigger the upper UAF. >>> >>> Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. >> >> Looks good at a glance. Will review more fully. > > Hi, > > Any comments for this patch? :) Ping... > >> >> Thanks, >> MIklos >> . >> > . ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-10-28 12:41 ` yangerkun @ 2021-10-29 8:35 ` Miklos Szeredi 2021-10-29 11:40 ` Miklos Szeredi 1 sibling, 0 replies; 12+ messages in thread From: Miklos Szeredi @ 2021-10-29 8:35 UTC (permalink / raw) To: yangerkun; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 On Thu, 28 Oct 2021 at 14:41, yangerkun <yangerkun@huawei.com> wrote: > > > > 在 2021/10/18 22:26, yangerkun 写道: > > > > > > 在 2021/10/8 23:13, Miklos Szeredi 写道: > >> On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: > >>> > >>> Our testcase trigger follow UAF: > >>> > >>> [ 153.939147] > >>> ================================================================== > >>> [ 153.942199] BUG: KASAN: use-after-free in > >>> ext4_dio_read_iter+0xcc/0x1a0 > >>> [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task > >>> fsstress/726 > >>> [ 153.948182] Call Trace: > >>> [ 153.948628] ? dump_stack_lvl+0x73/0x9f > >>> [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 > >>> [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 > >>> [ 153.951693] ? kasan_report.cold+0x81/0x165 > >>> [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 > >>> [ 153.953190] ? __asan_load8+0x74/0x110 > >>> [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 > >>> [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 > >>> [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 > >>> [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 > >>> [ 153.956916] ? ovl_read_iter+0x15f/0x270 > >>> [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 > >>> [ 153.958436] ? aio_setup_rw+0xbf/0xe0 > >>> [ 153.959101] ? aio_read+0x190/0x2d0 > >>> [ 153.959750] ? aio_write+0x3e0/0x3e0 > >>> [ 153.960404] ? __kasan_check_read+0x1d/0x30 > >>> [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 > >>> [ 153.961978] ? __put_user_ns+0x40/0x40 > >>> [ 153.962714] ? kasan_poison+0x40/0x90 > >>> [ 153.963384] ? set_alloc_info+0x46/0x70 > >>> [ 153.964102] ? __kasan_check_write+0x20/0x30 > >>> [ 153.964856] ? __fget_files+0x106/0x180 > >>> [ 153.965571] ? io_submit_one+0xaa7/0x1480 > >>> [ 153.966325] ? aio_poll_complete_work+0x590/0x590 > >>> [ 153.967713] ? ioctl_file_clone+0x110/0x110 > >>> [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 > >>> [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 > >>> [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 > >>> [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 > >>> [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 > >>> [ 153.975575] ? __kasan_check_read+0x1d/0x30 > >>> [ 153.976749] ? do_syscall_64+0x35/0x80 > >>> [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae > >>> [ 153.979436] > >>> [ 153.979738] Allocated by task 726: > >>> [ 153.980352] kasan_save_stack+0x23/0x60 > >>> [ 153.981054] set_alloc_info+0x46/0x70 > >>> [ 153.981745] __kasan_slab_alloc+0x4c/0x90 > >>> [ 153.982492] kmem_cache_alloc+0x153/0x750 > >>> [ 153.983953] ovl_read_iter+0x13b/0x270 > >>> [ 153.984689] aio_read+0x190/0x2d0 > >>> [ 153.985332] io_submit_one+0xaa7/0x1480 > >>> [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 > >>> [ 153.986878] do_syscall_64+0x35/0x80 > >>> [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae > >>> [ 153.988516] > >>> [ 153.988824] Freed by task 750: > >>> [ 153.989417] kasan_save_stack+0x23/0x60 > >>> [ 153.990140] kasan_set_track+0x24/0x40 > >>> [ 153.990871] kasan_set_free_info+0x30/0x60 > >>> [ 153.991639] __kasan_slab_free+0x137/0x210 > >>> [ 153.992390] kmem_cache_free+0xf8/0x590 > >>> [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 > >>> [ 153.994205] ovl_aio_rw_complete+0x31/0x60 > >>> [ 153.995043] iomap_dio_complete_work+0x4b/0x60 > >>> [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 > >>> [ 153.996742] bio_endio+0x40d/0x440 > >>> [ 153.997376] blk_update_request+0x38f/0x820 > >>> [ 153.998160] scsi_end_request+0x56/0x320 > >>> [ 153.998872] scsi_io_completion+0x10a/0xb60 > >>> [ 153.999655] scsi_finish_command+0x194/0x2b0 > >>> [ 154.000464] scsi_complete+0xd7/0x1f0 > >>> [ 154.001184] blk_complete_reqs+0x92/0xb0 > >>> [ 154.001930] blk_done_softirq+0x29/0x40 > >>> [ 154.002696] __do_softirq+0x133/0x57f > >>> > >>> ext4_dio_read_iter > >>> ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) > >>> file_accessed(iocb->ki_filp) <== this trigger UAF > >>> > >>> Ret can be -EIOCBQUEUED which means we will not wait for io > >>> completion in > >>> iomap_dio_rw. So the release for ovl_aio_req will be done when io > >>> completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler > >>> in ovl_read_iter will do this). But once io finish soon, we may already > >>> release ovl_aio_req before we call file_access in ext4_dio_read_iter. > >>> This can trigger the upper UAF. > >>> > >>> Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. > >> > >> Looks good at a glance. Will review more fully. > > > > Hi, > > > > Any comments for this patch? :) > > Ping... Now looking... Thanks, Miklos ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-10-28 12:41 ` yangerkun 2021-10-29 8:35 ` Miklos Szeredi @ 2021-10-29 11:40 ` Miklos Szeredi 2021-11-01 4:02 ` yangerkun 1 sibling, 1 reply; 12+ messages in thread From: Miklos Szeredi @ 2021-10-29 11:40 UTC (permalink / raw) To: yangerkun; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 On Thu, Oct 28, 2021 at 08:41:28PM +0800, yangerkun wrote: > > > 在 2021/10/18 22:26, yangerkun 写道: > > > > > > 在 2021/10/8 23:13, Miklos Szeredi 写道: > > > On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: > > > > > > > > Our testcase trigger follow UAF: > > > > > > > > [ 153.939147] > > > > ================================================================== > > > > [ 153.942199] BUG: KASAN: use-after-free in > > > > ext4_dio_read_iter+0xcc/0x1a0 > > > > [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task > > > > fsstress/726 > > > > [ 153.948182] Call Trace: > > > > [ 153.948628] ? dump_stack_lvl+0x73/0x9f > > > > [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 > > > > [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 > > > > [ 153.951693] ? kasan_report.cold+0x81/0x165 > > > > [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 > > > > [ 153.953190] ? __asan_load8+0x74/0x110 > > > > [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 > > > > [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 > > > > [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 > > > > [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 > > > > [ 153.956916] ? ovl_read_iter+0x15f/0x270 > > > > [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 > > > > [ 153.958436] ? aio_setup_rw+0xbf/0xe0 > > > > [ 153.959101] ? aio_read+0x190/0x2d0 > > > > [ 153.959750] ? aio_write+0x3e0/0x3e0 > > > > [ 153.960404] ? __kasan_check_read+0x1d/0x30 > > > > [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 > > > > [ 153.961978] ? __put_user_ns+0x40/0x40 > > > > [ 153.962714] ? kasan_poison+0x40/0x90 > > > > [ 153.963384] ? set_alloc_info+0x46/0x70 > > > > [ 153.964102] ? __kasan_check_write+0x20/0x30 > > > > [ 153.964856] ? __fget_files+0x106/0x180 > > > > [ 153.965571] ? io_submit_one+0xaa7/0x1480 > > > > [ 153.966325] ? aio_poll_complete_work+0x590/0x590 > > > > [ 153.967713] ? ioctl_file_clone+0x110/0x110 > > > > [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 > > > > [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 > > > > [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 > > > > [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 > > > > [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 > > > > [ 153.975575] ? __kasan_check_read+0x1d/0x30 > > > > [ 153.976749] ? do_syscall_64+0x35/0x80 > > > > [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae > > > > [ 153.979436] > > > > [ 153.979738] Allocated by task 726: > > > > [ 153.980352] kasan_save_stack+0x23/0x60 > > > > [ 153.981054] set_alloc_info+0x46/0x70 > > > > [ 153.981745] __kasan_slab_alloc+0x4c/0x90 > > > > [ 153.982492] kmem_cache_alloc+0x153/0x750 > > > > [ 153.983953] ovl_read_iter+0x13b/0x270 > > > > [ 153.984689] aio_read+0x190/0x2d0 > > > > [ 153.985332] io_submit_one+0xaa7/0x1480 > > > > [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 > > > > [ 153.986878] do_syscall_64+0x35/0x80 > > > > [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae > > > > [ 153.988516] > > > > [ 153.988824] Freed by task 750: > > > > [ 153.989417] kasan_save_stack+0x23/0x60 > > > > [ 153.990140] kasan_set_track+0x24/0x40 > > > > [ 153.990871] kasan_set_free_info+0x30/0x60 > > > > [ 153.991639] __kasan_slab_free+0x137/0x210 > > > > [ 153.992390] kmem_cache_free+0xf8/0x590 > > > > [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 > > > > [ 153.994205] ovl_aio_rw_complete+0x31/0x60 > > > > [ 153.995043] iomap_dio_complete_work+0x4b/0x60 > > > > [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 > > > > [ 153.996742] bio_endio+0x40d/0x440 > > > > [ 153.997376] blk_update_request+0x38f/0x820 > > > > [ 153.998160] scsi_end_request+0x56/0x320 > > > > [ 153.998872] scsi_io_completion+0x10a/0xb60 > > > > [ 153.999655] scsi_finish_command+0x194/0x2b0 > > > > [ 154.000464] scsi_complete+0xd7/0x1f0 > > > > [ 154.001184] blk_complete_reqs+0x92/0xb0 > > > > [ 154.001930] blk_done_softirq+0x29/0x40 > > > > [ 154.002696] __do_softirq+0x133/0x57f > > > > > > > > ext4_dio_read_iter > > > > ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) > > > > file_accessed(iocb->ki_filp) <== this trigger UAF > > > > > > > > Ret can be -EIOCBQUEUED which means we will not wait for io > > > > completion in > > > > iomap_dio_rw. So the release for ovl_aio_req will be done when io > > > > completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler > > > > in ovl_read_iter will do this). But once io finish soon, we may already > > > > release ovl_aio_req before we call file_access in ext4_dio_read_iter. > > > > This can trigger the upper UAF. > > > > > > > > Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. > > > > > > Looks good at a glance. Will review more fully. > > > > Hi, > > > > Any comments for this patch? :) Thanks for the patch. I found one issue: the fdput() needs to be moved to the destruction as well, since the iocb->ki_filp reference comes from aio_req->fd. Other than that, it looks good. Minor things: - instead of setting refcount to 1 and incrementing it, just initialize it to 2 - code deduplication can come after the fix (if at all), this helps backporting - cleaned up header comment Here's the updated patch, can you please verify that I didn't break anything? Thanks, Miklos --- From: yangerkun <yangerkun@huawei.com> Subject: ovl: fix use after free in struct ovl_aio_req Date: Thu, 30 Sep 2021 11:22:28 +0800 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 <yangerkun@huawei.com> Link: https://lore.kernel.org/r/20210930032228.3199690-3-yangerkun@huawei.com/ Cc: <stable@vger.kernel.org> # v5.6 Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/overlayfs/file.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -17,6 +17,7 @@ struct ovl_aio_req { struct kiocb iocb; + refcount_t ref; struct kiocb *orig_iocb; struct fd fd; }; @@ -252,6 +253,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; @@ -268,8 +277,7 @@ static void ovl_aio_cleanup_handler(stru } 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) @@ -319,7 +327,9 @@ static ssize_t ovl_read_iter(struct kioc 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); if (ret != -EIOCBQUEUED) ovl_aio_cleanup_handler(aio_req); } @@ -390,7 +400,9 @@ static ssize_t ovl_write_iter(struct kio 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); } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-10-29 11:40 ` Miklos Szeredi @ 2021-11-01 4:02 ` yangerkun 2021-11-01 9:15 ` Miklos Szeredi 0 siblings, 1 reply; 12+ messages in thread From: yangerkun @ 2021-11-01 4:02 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 On 2021/10/29 19:40, Miklos Szeredi wrote: > On Thu, Oct 28, 2021 at 08:41:28PM +0800, yangerkun wrote: >> >> >> 在 2021/10/18 22:26, yangerkun 写道: >>> >>> >>> 在 2021/10/8 23:13, Miklos Szeredi 写道: >>>> On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: >>>>> >>>>> Our testcase trigger follow UAF: >>>>> >>>>> [ 153.939147] >>>>> ================================================================== >>>>> [ 153.942199] BUG: KASAN: use-after-free in >>>>> ext4_dio_read_iter+0xcc/0x1a0 >>>>> [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task >>>>> fsstress/726 >>>>> [ 153.948182] Call Trace: >>>>> [ 153.948628] ? dump_stack_lvl+0x73/0x9f >>>>> [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>> [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>> [ 153.951693] ? kasan_report.cold+0x81/0x165 >>>>> [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>> [ 153.953190] ? __asan_load8+0x74/0x110 >>>>> [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>> [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 >>>>> [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 >>>>> [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 >>>>> [ 153.956916] ? ovl_read_iter+0x15f/0x270 >>>>> [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 >>>>> [ 153.958436] ? aio_setup_rw+0xbf/0xe0 >>>>> [ 153.959101] ? aio_read+0x190/0x2d0 >>>>> [ 153.959750] ? aio_write+0x3e0/0x3e0 >>>>> [ 153.960404] ? __kasan_check_read+0x1d/0x30 >>>>> [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 >>>>> [ 153.961978] ? __put_user_ns+0x40/0x40 >>>>> [ 153.962714] ? kasan_poison+0x40/0x90 >>>>> [ 153.963384] ? set_alloc_info+0x46/0x70 >>>>> [ 153.964102] ? __kasan_check_write+0x20/0x30 >>>>> [ 153.964856] ? __fget_files+0x106/0x180 >>>>> [ 153.965571] ? io_submit_one+0xaa7/0x1480 >>>>> [ 153.966325] ? aio_poll_complete_work+0x590/0x590 >>>>> [ 153.967713] ? ioctl_file_clone+0x110/0x110 >>>>> [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 >>>>> [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 >>>>> [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 >>>>> [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 >>>>> [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 >>>>> [ 153.975575] ? __kasan_check_read+0x1d/0x30 >>>>> [ 153.976749] ? do_syscall_64+0x35/0x80 >>>>> [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae >>>>> [ 153.979436] >>>>> [ 153.979738] Allocated by task 726: >>>>> [ 153.980352] kasan_save_stack+0x23/0x60 >>>>> [ 153.981054] set_alloc_info+0x46/0x70 >>>>> [ 153.981745] __kasan_slab_alloc+0x4c/0x90 >>>>> [ 153.982492] kmem_cache_alloc+0x153/0x750 >>>>> [ 153.983953] ovl_read_iter+0x13b/0x270 >>>>> [ 153.984689] aio_read+0x190/0x2d0 >>>>> [ 153.985332] io_submit_one+0xaa7/0x1480 >>>>> [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 >>>>> [ 153.986878] do_syscall_64+0x35/0x80 >>>>> [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae >>>>> [ 153.988516] >>>>> [ 153.988824] Freed by task 750: >>>>> [ 153.989417] kasan_save_stack+0x23/0x60 >>>>> [ 153.990140] kasan_set_track+0x24/0x40 >>>>> [ 153.990871] kasan_set_free_info+0x30/0x60 >>>>> [ 153.991639] __kasan_slab_free+0x137/0x210 >>>>> [ 153.992390] kmem_cache_free+0xf8/0x590 >>>>> [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 >>>>> [ 153.994205] ovl_aio_rw_complete+0x31/0x60 >>>>> [ 153.995043] iomap_dio_complete_work+0x4b/0x60 >>>>> [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 >>>>> [ 153.996742] bio_endio+0x40d/0x440 >>>>> [ 153.997376] blk_update_request+0x38f/0x820 >>>>> [ 153.998160] scsi_end_request+0x56/0x320 >>>>> [ 153.998872] scsi_io_completion+0x10a/0xb60 >>>>> [ 153.999655] scsi_finish_command+0x194/0x2b0 >>>>> [ 154.000464] scsi_complete+0xd7/0x1f0 >>>>> [ 154.001184] blk_complete_reqs+0x92/0xb0 >>>>> [ 154.001930] blk_done_softirq+0x29/0x40 >>>>> [ 154.002696] __do_softirq+0x133/0x57f >>>>> >>>>> ext4_dio_read_iter >>>>> ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) >>>>> file_accessed(iocb->ki_filp) <== this trigger UAF >>>>> >>>>> Ret can be -EIOCBQUEUED which means we will not wait for io >>>>> completion in >>>>> iomap_dio_rw. So the release for ovl_aio_req will be done when io >>>>> completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler >>>>> in ovl_read_iter will do this). But once io finish soon, we may already >>>>> release ovl_aio_req before we call file_access in ext4_dio_read_iter. >>>>> This can trigger the upper UAF. >>>>> >>>>> Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. >>>> >>>> Looks good at a glance. Will review more fully. >>> >>> Hi, >>> >>> Any comments for this patch? :) > > Thanks for the patch. I found one issue: the fdput() needs to be moved to the > destruction as well, since the iocb->ki_filp reference comes from aio_req->fd. Hi, it seems we will only use aio_req->iocb in ovl_aio_cleanup_handler before we call fdput(aio_req->fd): static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) { struct kiocb *iocb = &aio_req->iocb; struct kiocb *orig_iocb = aio_req->orig_iocb; 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); <=== use iocb ovl_copyattr(ovl_inode_real(inode), inode); } orig_iocb->ki_pos = iocb->ki_pos; fdput(aio_req->fd); <=== release aio_req->fd kmem_cache_free(ovl_aio_request_cachep, aio_req); } So call fdput(aio_req->fd) in ovl_aio_cleanup_handler or ovl_aio_put seems all ok for me. > > Other than that, it looks good. > > Minor things: > > - instead of setting refcount to 1 and incrementing it, just initialize it to 2 > > - code deduplication can come after the fix (if at all), this helps backporting Agree. > > - cleaned up header comment > > Here's the updated patch, can you please verify that I didn't break anything? Thanks! This patch can help pass the test! > > Thanks, > Miklos > --- > > From: yangerkun <yangerkun@huawei.com> > Subject: ovl: fix use after free in struct ovl_aio_req > Date: Thu, 30 Sep 2021 11:22:28 +0800 > > 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 <yangerkun@huawei.com> > Link: https://lore.kernel.org/r/20210930032228.3199690-3-yangerkun@huawei.com/ > Cc: <stable@vger.kernel.org> # v5.6 > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > fs/overlayfs/file.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > --- a/fs/overlayfs/file.c > +++ b/fs/overlayfs/file.c > @@ -17,6 +17,7 @@ > > struct ovl_aio_req { > struct kiocb iocb; > + refcount_t ref; > struct kiocb *orig_iocb; > struct fd fd; > }; > @@ -252,6 +253,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; > @@ -268,8 +277,7 @@ static void ovl_aio_cleanup_handler(stru > } > > 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) > @@ -319,7 +327,9 @@ static ssize_t ovl_read_iter(struct kioc > 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); > if (ret != -EIOCBQUEUED) > ovl_aio_cleanup_handler(aio_req); > } > @@ -390,7 +400,9 @@ static ssize_t ovl_write_iter(struct kio > 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); > } > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-11-01 4:02 ` yangerkun @ 2021-11-01 9:15 ` Miklos Szeredi 2021-11-01 10:52 ` yangerkun 0 siblings, 1 reply; 12+ messages in thread From: Miklos Szeredi @ 2021-11-01 9:15 UTC (permalink / raw) To: yangerkun; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 On Mon, 1 Nov 2021 at 05:02, yangerkun <yangerkun@huawei.com> wrote: > > > > On 2021/10/29 19:40, Miklos Szeredi wrote: > > On Thu, Oct 28, 2021 at 08:41:28PM +0800, yangerkun wrote: > >> > >> > >> 在 2021/10/18 22:26, yangerkun 写道: > >>> > >>> > >>> 在 2021/10/8 23:13, Miklos Szeredi 写道: > >>>> On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: > >>>>> > >>>>> Our testcase trigger follow UAF: > >>>>> > >>>>> [ 153.939147] > >>>>> ================================================================== > >>>>> [ 153.942199] BUG: KASAN: use-after-free in > >>>>> ext4_dio_read_iter+0xcc/0x1a0 > >>>>> [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task > >>>>> fsstress/726 > >>>>> [ 153.948182] Call Trace: > >>>>> [ 153.948628] ? dump_stack_lvl+0x73/0x9f > >>>>> [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 > >>>>> [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 > >>>>> [ 153.951693] ? kasan_report.cold+0x81/0x165 > >>>>> [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 > >>>>> [ 153.953190] ? __asan_load8+0x74/0x110 > >>>>> [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 > >>>>> [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 > >>>>> [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 > >>>>> [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 > >>>>> [ 153.956916] ? ovl_read_iter+0x15f/0x270 > >>>>> [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 > >>>>> [ 153.958436] ? aio_setup_rw+0xbf/0xe0 > >>>>> [ 153.959101] ? aio_read+0x190/0x2d0 > >>>>> [ 153.959750] ? aio_write+0x3e0/0x3e0 > >>>>> [ 153.960404] ? __kasan_check_read+0x1d/0x30 > >>>>> [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 > >>>>> [ 153.961978] ? __put_user_ns+0x40/0x40 > >>>>> [ 153.962714] ? kasan_poison+0x40/0x90 > >>>>> [ 153.963384] ? set_alloc_info+0x46/0x70 > >>>>> [ 153.964102] ? __kasan_check_write+0x20/0x30 > >>>>> [ 153.964856] ? __fget_files+0x106/0x180 > >>>>> [ 153.965571] ? io_submit_one+0xaa7/0x1480 > >>>>> [ 153.966325] ? aio_poll_complete_work+0x590/0x590 > >>>>> [ 153.967713] ? ioctl_file_clone+0x110/0x110 > >>>>> [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 > >>>>> [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 > >>>>> [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 > >>>>> [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 > >>>>> [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 > >>>>> [ 153.975575] ? __kasan_check_read+0x1d/0x30 > >>>>> [ 153.976749] ? do_syscall_64+0x35/0x80 > >>>>> [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae > >>>>> [ 153.979436] > >>>>> [ 153.979738] Allocated by task 726: > >>>>> [ 153.980352] kasan_save_stack+0x23/0x60 > >>>>> [ 153.981054] set_alloc_info+0x46/0x70 > >>>>> [ 153.981745] __kasan_slab_alloc+0x4c/0x90 > >>>>> [ 153.982492] kmem_cache_alloc+0x153/0x750 > >>>>> [ 153.983953] ovl_read_iter+0x13b/0x270 > >>>>> [ 153.984689] aio_read+0x190/0x2d0 > >>>>> [ 153.985332] io_submit_one+0xaa7/0x1480 > >>>>> [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 > >>>>> [ 153.986878] do_syscall_64+0x35/0x80 > >>>>> [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae > >>>>> [ 153.988516] > >>>>> [ 153.988824] Freed by task 750: > >>>>> [ 153.989417] kasan_save_stack+0x23/0x60 > >>>>> [ 153.990140] kasan_set_track+0x24/0x40 > >>>>> [ 153.990871] kasan_set_free_info+0x30/0x60 > >>>>> [ 153.991639] __kasan_slab_free+0x137/0x210 > >>>>> [ 153.992390] kmem_cache_free+0xf8/0x590 > >>>>> [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 > >>>>> [ 153.994205] ovl_aio_rw_complete+0x31/0x60 > >>>>> [ 153.995043] iomap_dio_complete_work+0x4b/0x60 > >>>>> [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 > >>>>> [ 153.996742] bio_endio+0x40d/0x440 > >>>>> [ 153.997376] blk_update_request+0x38f/0x820 > >>>>> [ 153.998160] scsi_end_request+0x56/0x320 > >>>>> [ 153.998872] scsi_io_completion+0x10a/0xb60 > >>>>> [ 153.999655] scsi_finish_command+0x194/0x2b0 > >>>>> [ 154.000464] scsi_complete+0xd7/0x1f0 > >>>>> [ 154.001184] blk_complete_reqs+0x92/0xb0 > >>>>> [ 154.001930] blk_done_softirq+0x29/0x40 > >>>>> [ 154.002696] __do_softirq+0x133/0x57f > >>>>> > >>>>> ext4_dio_read_iter > >>>>> ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) > >>>>> file_accessed(iocb->ki_filp) <== this trigger UAF > >>>>> > >>>>> Ret can be -EIOCBQUEUED which means we will not wait for io > >>>>> completion in > >>>>> iomap_dio_rw. So the release for ovl_aio_req will be done when io > >>>>> completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler > >>>>> in ovl_read_iter will do this). But once io finish soon, we may already > >>>>> release ovl_aio_req before we call file_access in ext4_dio_read_iter. > >>>>> This can trigger the upper UAF. > >>>>> > >>>>> Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. > >>>> > >>>> Looks good at a glance. Will review more fully. > >>> > >>> Hi, > >>> > >>> Any comments for this patch? :) > > > > Thanks for the patch. I found one issue: the fdput() needs to be moved to the > > destruction as well, since the iocb->ki_filp reference comes from aio_req->fd. > > Hi, it seems we will only use aio_req->iocb in ovl_aio_cleanup_handler > before we call fdput(aio_req->fd): Correct. However I'm taking about this call: file_accessed(iocb->ki_filp). If the fdput() is done from ovl_aio_cleanup_handler() which is called from ->ki_complete() then this would put the file, agains resulting in a UAF, only inside file_accessed() this time. The reason why you weren't seeing this, is probably that most of the time aio_req->fd is from file->private_data, which is the real file obtained at open time, and which isn't actually closed at fdput() (flags == 0). > > static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) > { > struct kiocb *iocb = &aio_req->iocb; > struct kiocb *orig_iocb = aio_req->orig_iocb; > > 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); > <=== use iocb > ovl_copyattr(ovl_inode_real(inode), inode); > } > > orig_iocb->ki_pos = iocb->ki_pos; > fdput(aio_req->fd); <=== release aio_req->fd > kmem_cache_free(ovl_aio_request_cachep, aio_req); > } > > So call fdput(aio_req->fd) in ovl_aio_cleanup_handler or ovl_aio_put > seems all ok for me. > > > > > Other than that, it looks good. > > > > Minor things: > > > > - instead of setting refcount to 1 and incrementing it, just initialize it to 2 > > > > - code deduplication can come after the fix (if at all), this helps backporting > > Agree. > > > > > - cleaned up header comment > > > > Here's the updated patch, can you please verify that I didn't break anything? > > Thanks! This patch can help pass the test! Great, thanks. Miklos ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] ovl: fix UAF for ovl_aio_req 2021-11-01 9:15 ` Miklos Szeredi @ 2021-11-01 10:52 ` yangerkun 0 siblings, 0 replies; 12+ messages in thread From: yangerkun @ 2021-11-01 10:52 UTC (permalink / raw) To: Miklos Szeredi; +Cc: Amir Goldstein, Jiufei Xue, overlayfs, yukuai3 On 2021/11/1 17:15, Miklos Szeredi wrote: > On Mon, 1 Nov 2021 at 05:02, yangerkun <yangerkun@huawei.com> wrote: >> >> >> >> On 2021/10/29 19:40, Miklos Szeredi wrote: >>> On Thu, Oct 28, 2021 at 08:41:28PM +0800, yangerkun wrote: >>>> >>>> >>>> 在 2021/10/18 22:26, yangerkun 写道: >>>>> >>>>> >>>>> 在 2021/10/8 23:13, Miklos Szeredi 写道: >>>>>> On Thu, 30 Sept 2021 at 05:12, yangerkun <yangerkun@huawei.com> wrote: >>>>>>> >>>>>>> Our testcase trigger follow UAF: >>>>>>> >>>>>>> [ 153.939147] >>>>>>> ================================================================== >>>>>>> [ 153.942199] BUG: KASAN: use-after-free in >>>>>>> ext4_dio_read_iter+0xcc/0x1a0 >>>>>>> [ 153.943331] Read of size 8 at addr ffff88803b7f1500 by task >>>>>>> fsstress/726 >>>>>>> [ 153.948182] Call Trace: >>>>>>> [ 153.948628] ? dump_stack_lvl+0x73/0x9f >>>>>>> [ 153.950255] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>>>> [ 153.950972] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>>>> [ 153.951693] ? kasan_report.cold+0x81/0x165 >>>>>>> [ 153.952429] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>>>> [ 153.953190] ? __asan_load8+0x74/0x110 >>>>>>> [ 153.953856] ? ext4_dio_read_iter+0xcc/0x1a0 >>>>>>> [ 153.954612] ? ext4_file_read_iter+0x1df/0x2a0 >>>>>>> [ 153.955394] ? ext4_dio_read_iter+0x1a0/0x1a0 >>>>>>> [ 153.956159] ? vfs_iocb_iter_read+0xd5/0x330 >>>>>>> [ 153.956916] ? ovl_read_iter+0x15f/0x270 >>>>>>> [ 153.957605] ? ovl_aio_cleanup_handler+0x2a0/0x2a0 >>>>>>> [ 153.958436] ? aio_setup_rw+0xbf/0xe0 >>>>>>> [ 153.959101] ? aio_read+0x190/0x2d0 >>>>>>> [ 153.959750] ? aio_write+0x3e0/0x3e0 >>>>>>> [ 153.960404] ? __kasan_check_read+0x1d/0x30 >>>>>>> [ 153.961167] ? ext4_file_getattr+0x116/0x1b0 >>>>>>> [ 153.961978] ? __put_user_ns+0x40/0x40 >>>>>>> [ 153.962714] ? kasan_poison+0x40/0x90 >>>>>>> [ 153.963384] ? set_alloc_info+0x46/0x70 >>>>>>> [ 153.964102] ? __kasan_check_write+0x20/0x30 >>>>>>> [ 153.964856] ? __fget_files+0x106/0x180 >>>>>>> [ 153.965571] ? io_submit_one+0xaa7/0x1480 >>>>>>> [ 153.966325] ? aio_poll_complete_work+0x590/0x590 >>>>>>> [ 153.967713] ? ioctl_file_clone+0x110/0x110 >>>>>>> [ 153.969008] ? vfs_getattr_nosec+0x14a/0x190 >>>>>>> [ 153.970539] ? __do_sys_newfstat+0xd6/0xf0 >>>>>>> [ 153.971713] ? __ia32_compat_sys_newfstat+0x40/0x40 >>>>>>> [ 153.973005] ? __x64_sys_io_submit+0x125/0x3b0 >>>>>>> [ 153.974282] ? __ia32_sys_io_submit+0x390/0x390 >>>>>>> [ 153.975575] ? __kasan_check_read+0x1d/0x30 >>>>>>> [ 153.976749] ? do_syscall_64+0x35/0x80 >>>>>>> [ 153.978034] ? entry_SYSCALL_64_after_hwframe+0x44/0xae >>>>>>> [ 153.979436] >>>>>>> [ 153.979738] Allocated by task 726: >>>>>>> [ 153.980352] kasan_save_stack+0x23/0x60 >>>>>>> [ 153.981054] set_alloc_info+0x46/0x70 >>>>>>> [ 153.981745] __kasan_slab_alloc+0x4c/0x90 >>>>>>> [ 153.982492] kmem_cache_alloc+0x153/0x750 >>>>>>> [ 153.983953] ovl_read_iter+0x13b/0x270 >>>>>>> [ 153.984689] aio_read+0x190/0x2d0 >>>>>>> [ 153.985332] io_submit_one+0xaa7/0x1480 >>>>>>> [ 153.986060] __x64_sys_io_submit+0x125/0x3b0 >>>>>>> [ 153.986878] do_syscall_64+0x35/0x80 >>>>>>> [ 153.987590] entry_SYSCALL_64_after_hwframe+0x44/0xae >>>>>>> [ 153.988516] >>>>>>> [ 153.988824] Freed by task 750: >>>>>>> [ 153.989417] kasan_save_stack+0x23/0x60 >>>>>>> [ 153.990140] kasan_set_track+0x24/0x40 >>>>>>> [ 153.990871] kasan_set_free_info+0x30/0x60 >>>>>>> [ 153.991639] __kasan_slab_free+0x137/0x210 >>>>>>> [ 153.992390] kmem_cache_free+0xf8/0x590 >>>>>>> [ 153.993303] ovl_aio_cleanup_handler+0x1ae/0x2a0 >>>>>>> [ 153.994205] ovl_aio_rw_complete+0x31/0x60 >>>>>>> [ 153.995043] iomap_dio_complete_work+0x4b/0x60 >>>>>>> [ 153.995898] iomap_dio_bio_end_io+0x23d/0x270 >>>>>>> [ 153.996742] bio_endio+0x40d/0x440 >>>>>>> [ 153.997376] blk_update_request+0x38f/0x820 >>>>>>> [ 153.998160] scsi_end_request+0x56/0x320 >>>>>>> [ 153.998872] scsi_io_completion+0x10a/0xb60 >>>>>>> [ 153.999655] scsi_finish_command+0x194/0x2b0 >>>>>>> [ 154.000464] scsi_complete+0xd7/0x1f0 >>>>>>> [ 154.001184] blk_complete_reqs+0x92/0xb0 >>>>>>> [ 154.001930] blk_done_softirq+0x29/0x40 >>>>>>> [ 154.002696] __do_softirq+0x133/0x57f >>>>>>> >>>>>>> ext4_dio_read_iter >>>>>>> ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0) >>>>>>> file_accessed(iocb->ki_filp) <== this trigger UAF >>>>>>> >>>>>>> Ret can be -EIOCBQUEUED which means we will not wait for io >>>>>>> completion in >>>>>>> iomap_dio_rw. So the release for ovl_aio_req will be done when io >>>>>>> completion routine call ovl_aio_rw_complete(or ovl_aio_cleanup_handler >>>>>>> in ovl_read_iter will do this). But once io finish soon, we may already >>>>>>> release ovl_aio_req before we call file_access in ext4_dio_read_iter. >>>>>>> This can trigger the upper UAF. >>>>>>> >>>>>>> Fix it by introduce refcount in ovl_aio_req like what aio_kiocb has did. >>>>>> >>>>>> Looks good at a glance. Will review more fully. >>>>> >>>>> Hi, >>>>> >>>>> Any comments for this patch? :) >>> >>> Thanks for the patch. I found one issue: the fdput() needs to be moved to the >>> destruction as well, since the iocb->ki_filp reference comes from aio_req->fd. >> >> Hi, it seems we will only use aio_req->iocb in ovl_aio_cleanup_handler >> before we call fdput(aio_req->fd): > > Correct. However I'm taking about this call: > file_accessed(iocb->ki_filp). If the fdput() is done from > ovl_aio_cleanup_handler() which is called from ->ki_complete() then > this would put the file, agains resulting in a UAF, only inside > file_accessed() this time. The reason why you weren't seeing this, > is probably that most of the time aio_req->fd is from > file->private_data, which is the real file obtained at open time, and > which isn't actually closed at fdput() (flags == 0). Thanks for your analysis. Got it now! > >> >> static void ovl_aio_cleanup_handler(struct ovl_aio_req *aio_req) >> { >> struct kiocb *iocb = &aio_req->iocb; >> struct kiocb *orig_iocb = aio_req->orig_iocb; >> >> 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); >> <=== use iocb >> ovl_copyattr(ovl_inode_real(inode), inode); >> } >> >> orig_iocb->ki_pos = iocb->ki_pos; >> fdput(aio_req->fd); <=== release aio_req->fd >> kmem_cache_free(ovl_aio_request_cachep, aio_req); >> } >> >> So call fdput(aio_req->fd) in ovl_aio_cleanup_handler or ovl_aio_put >> seems all ok for me. >> >>> >>> Other than that, it looks good. >>> >>> Minor things: >>> >>> - instead of setting refcount to 1 and incrementing it, just initialize it to 2 >>> >>> - code deduplication can come after the fix (if at all), this helps backporting >> >> Agree. >> >>> >>> - cleaned up header comment >>> >>> Here's the updated patch, can you please verify that I didn't break anything? >> >> Thanks! This patch can help pass the test! > > Great, thanks. > > Miklos > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-11-01 10:52 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-30 3:22 [PATCH 0/2] ovl: bugfix for ovl_aio_req yangerkun 2021-09-30 3:22 ` [PATCH 1/2] ovl: factor out ovl_get_aio_req yangerkun 2021-09-30 3:22 ` [PATCH 2/2] ovl: fix UAF for ovl_aio_req yangerkun 2021-10-08 1:25 ` yangerkun 2021-10-08 15:13 ` Miklos Szeredi 2021-10-18 14:26 ` yangerkun 2021-10-28 12:41 ` yangerkun 2021-10-29 8:35 ` Miklos Szeredi 2021-10-29 11:40 ` Miklos Szeredi 2021-11-01 4:02 ` yangerkun 2021-11-01 9:15 ` Miklos Szeredi 2021-11-01 10:52 ` yangerkun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).