* [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 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.