* Re: [PATCH] configfs: Fix use-after-free issue in __configfs_open_file [not found] <20210301061053.105377-1-yuchao0@huawei.com> @ 2021-03-05 3:29 ` Chao Yu 2021-03-11 2:16 ` Chao Yu 0 siblings, 1 reply; 3+ messages in thread From: Chao Yu @ 2021-03-05 3:29 UTC (permalink / raw) To: linux-kernel Cc: chao, Daiyue Zhang, Al Viro, Joel Becker, Christoph Hellwig, Yi Chen, Ge Qiu, linux-fsdevel +Cc fsdevel Ping, Any comments one this patch? On 2021/3/1 14:10, Chao Yu wrote: > From: Daiyue Zhang <zhangdaiyue1@huawei.com> > > Commit b0841eefd969 ("configfs: provide exclusion between IO and removals") > uses ->frag_dead to mark the fragment state, thus no bothering with extra > refcount on config_item when opening a file. The configfs_get_config_item > was removed in __configfs_open_file, but not with config_item_put. So the > refcount on config_item will lost its balance, causing use-after-free > issues in some occasions like this: > > Test: > 1. Mount configfs on /config with read-only items: > drwxrwx--- 289 root root 0 2021-04-01 11:55 /config > drwxr-xr-x 2 root root 0 2021-04-01 11:54 /config/a > --w--w--w- 1 root root 4096 2021-04-01 11:53 /config/a/1.txt > ...... > > 2. Then run: > for file in /config > do > echo $file > grep -R 'key' $file > done > > 3. __configfs_open_file will be called in parallel, the first one > got called will do: > if (file->f_mode & FMODE_READ) { > if (!(inode->i_mode & S_IRUGO)) > goto out_put_module; > config_item_put(buffer->item); > kref_put() > package_details_release() > kfree() > > the other one will run into use-after-free issues like this: > BUG: KASAN: use-after-free in __configfs_open_file+0x1bc/0x3b0 > Read of size 8 at addr fffffff155f02480 by task grep/13096 > CPU: 0 PID: 13096 Comm: grep VIP: 00 Tainted: G W 4.14.116-kasan #1 > TGID: 13096 Comm: grep > Call trace: > dump_stack+0x118/0x160 > kasan_report+0x22c/0x294 > __asan_load8+0x80/0x88 > __configfs_open_file+0x1bc/0x3b0 > configfs_open_file+0x28/0x34 > do_dentry_open+0x2cc/0x5c0 > vfs_open+0x80/0xe0 > path_openat+0xd8c/0x2988 > do_filp_open+0x1c4/0x2fc > do_sys_open+0x23c/0x404 > SyS_openat+0x38/0x48 > > Allocated by task 2138: > kasan_kmalloc+0xe0/0x1ac > kmem_cache_alloc_trace+0x334/0x394 > packages_make_item+0x4c/0x180 > configfs_mkdir+0x358/0x740 > vfs_mkdir2+0x1bc/0x2e8 > SyS_mkdirat+0x154/0x23c > el0_svc_naked+0x34/0x38 > > Freed by task 13096: > kasan_slab_free+0xb8/0x194 > kfree+0x13c/0x910 > package_details_release+0x524/0x56c > kref_put+0xc4/0x104 > config_item_put+0x24/0x34 > __configfs_open_file+0x35c/0x3b0 > configfs_open_file+0x28/0x34 > do_dentry_open+0x2cc/0x5c0 > vfs_open+0x80/0xe0 > path_openat+0xd8c/0x2988 > do_filp_open+0x1c4/0x2fc > do_sys_open+0x23c/0x404 > SyS_openat+0x38/0x48 > el0_svc_naked+0x34/0x38 > > To fix this issue, remove the config_item_put in > __configfs_open_file to balance the refcount of config_item. > > Fixes: b0841eefd969 ("configfs: provide exclusion between IO and removals") > Cc: Al Viro <viro@zeniv.linux.org.uk> > Cc: Joel Becker <jlbec@evilplan.org> > Cc: Christoph Hellwig <hch@lst.de> > Signed-off-by: Daiyue Zhang <zhangdaiyue1@huawei.com> > Signed-off-by: Yi Chen <chenyi77@huawei.com> > Signed-off-by: Ge Qiu <qiuge@huawei.com> > Reviewed-by: Chao Yu <yuchao0@huawei.com> > --- > fs/configfs/file.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/fs/configfs/file.c b/fs/configfs/file.c > index 1f0270229d7b..da8351d1e455 100644 > --- a/fs/configfs/file.c > +++ b/fs/configfs/file.c > @@ -378,7 +378,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type > > attr = to_attr(dentry); > if (!attr) > - goto out_put_item; > + goto out_free_buffer; > > if (type & CONFIGFS_ITEM_BIN_ATTR) { > buffer->bin_attr = to_bin_attr(dentry); > @@ -391,7 +391,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type > /* Grab the module reference for this attribute if we have one */ > error = -ENODEV; > if (!try_module_get(buffer->owner)) > - goto out_put_item; > + goto out_free_buffer; > > error = -EACCES; > if (!buffer->item->ci_type) > @@ -435,8 +435,6 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type > > out_put_module: > module_put(buffer->owner); > -out_put_item: > - config_item_put(buffer->item); > out_free_buffer: > up_read(&frag->frag_sem); > kfree(buffer); > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] configfs: Fix use-after-free issue in __configfs_open_file 2021-03-05 3:29 ` [PATCH] configfs: Fix use-after-free issue in __configfs_open_file Chao Yu @ 2021-03-11 2:16 ` Chao Yu 2021-03-11 3:42 ` Al Viro 0 siblings, 1 reply; 3+ messages in thread From: Chao Yu @ 2021-03-11 2:16 UTC (permalink / raw) To: Al Viro, Joel Becker, Christoph Hellwig Cc: linux-kernel, chao, Daiyue Zhang, Yi Chen, Ge Qiu, linux-fsdevel Hi Joel, Christoph, Al Does any one have time to review this patch, ten days past... Thanks, On 2021/3/5 11:29, Chao Yu wrote: > +Cc fsdevel > > Ping, > > Any comments one this patch? > > On 2021/3/1 14:10, Chao Yu wrote: >> From: Daiyue Zhang <zhangdaiyue1@huawei.com> >> >> Commit b0841eefd969 ("configfs: provide exclusion between IO and removals") >> uses ->frag_dead to mark the fragment state, thus no bothering with extra >> refcount on config_item when opening a file. The configfs_get_config_item >> was removed in __configfs_open_file, but not with config_item_put. So the >> refcount on config_item will lost its balance, causing use-after-free >> issues in some occasions like this: >> >> Test: >> 1. Mount configfs on /config with read-only items: >> drwxrwx--- 289 root root 0 2021-04-01 11:55 /config >> drwxr-xr-x 2 root root 0 2021-04-01 11:54 /config/a >> --w--w--w- 1 root root 4096 2021-04-01 11:53 /config/a/1.txt >> ...... >> >> 2. Then run: >> for file in /config >> do >> echo $file >> grep -R 'key' $file >> done >> >> 3. __configfs_open_file will be called in parallel, the first one >> got called will do: >> if (file->f_mode & FMODE_READ) { >> if (!(inode->i_mode & S_IRUGO)) >> goto out_put_module; >> config_item_put(buffer->item); >> kref_put() >> package_details_release() >> kfree() >> >> the other one will run into use-after-free issues like this: >> BUG: KASAN: use-after-free in __configfs_open_file+0x1bc/0x3b0 >> Read of size 8 at addr fffffff155f02480 by task grep/13096 >> CPU: 0 PID: 13096 Comm: grep VIP: 00 Tainted: G W 4.14.116-kasan #1 >> TGID: 13096 Comm: grep >> Call trace: >> dump_stack+0x118/0x160 >> kasan_report+0x22c/0x294 >> __asan_load8+0x80/0x88 >> __configfs_open_file+0x1bc/0x3b0 >> configfs_open_file+0x28/0x34 >> do_dentry_open+0x2cc/0x5c0 >> vfs_open+0x80/0xe0 >> path_openat+0xd8c/0x2988 >> do_filp_open+0x1c4/0x2fc >> do_sys_open+0x23c/0x404 >> SyS_openat+0x38/0x48 >> >> Allocated by task 2138: >> kasan_kmalloc+0xe0/0x1ac >> kmem_cache_alloc_trace+0x334/0x394 >> packages_make_item+0x4c/0x180 >> configfs_mkdir+0x358/0x740 >> vfs_mkdir2+0x1bc/0x2e8 >> SyS_mkdirat+0x154/0x23c >> el0_svc_naked+0x34/0x38 >> >> Freed by task 13096: >> kasan_slab_free+0xb8/0x194 >> kfree+0x13c/0x910 >> package_details_release+0x524/0x56c >> kref_put+0xc4/0x104 >> config_item_put+0x24/0x34 >> __configfs_open_file+0x35c/0x3b0 >> configfs_open_file+0x28/0x34 >> do_dentry_open+0x2cc/0x5c0 >> vfs_open+0x80/0xe0 >> path_openat+0xd8c/0x2988 >> do_filp_open+0x1c4/0x2fc >> do_sys_open+0x23c/0x404 >> SyS_openat+0x38/0x48 >> el0_svc_naked+0x34/0x38 >> >> To fix this issue, remove the config_item_put in >> __configfs_open_file to balance the refcount of config_item. >> >> Fixes: b0841eefd969 ("configfs: provide exclusion between IO and removals") >> Cc: Al Viro <viro@zeniv.linux.org.uk> >> Cc: Joel Becker <jlbec@evilplan.org> >> Cc: Christoph Hellwig <hch@lst.de> >> Signed-off-by: Daiyue Zhang <zhangdaiyue1@huawei.com> >> Signed-off-by: Yi Chen <chenyi77@huawei.com> >> Signed-off-by: Ge Qiu <qiuge@huawei.com> >> Reviewed-by: Chao Yu <yuchao0@huawei.com> >> --- >> fs/configfs/file.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/fs/configfs/file.c b/fs/configfs/file.c >> index 1f0270229d7b..da8351d1e455 100644 >> --- a/fs/configfs/file.c >> +++ b/fs/configfs/file.c >> @@ -378,7 +378,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type >> >> attr = to_attr(dentry); >> if (!attr) >> - goto out_put_item; >> + goto out_free_buffer; >> >> if (type & CONFIGFS_ITEM_BIN_ATTR) { >> buffer->bin_attr = to_bin_attr(dentry); >> @@ -391,7 +391,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type >> /* Grab the module reference for this attribute if we have one */ >> error = -ENODEV; >> if (!try_module_get(buffer->owner)) >> - goto out_put_item; >> + goto out_free_buffer; >> >> error = -EACCES; >> if (!buffer->item->ci_type) >> @@ -435,8 +435,6 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type >> >> out_put_module: >> module_put(buffer->owner); >> -out_put_item: >> - config_item_put(buffer->item); >> out_free_buffer: >> up_read(&frag->frag_sem); >> kfree(buffer); >> > . > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] configfs: Fix use-after-free issue in __configfs_open_file 2021-03-11 2:16 ` Chao Yu @ 2021-03-11 3:42 ` Al Viro 0 siblings, 0 replies; 3+ messages in thread From: Al Viro @ 2021-03-11 3:42 UTC (permalink / raw) To: Chao Yu Cc: Joel Becker, Christoph Hellwig, linux-kernel, chao, Daiyue Zhang, Yi Chen, Ge Qiu, linux-fsdevel On Thu, Mar 11, 2021 at 10:16:20AM +0800, Chao Yu wrote: > Hi Joel, Christoph, Al > > Does any one have time to review this patch, ten days past... Acked-by: Al Viro <viro@zeniv.linux.org.uk> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-11 3:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210301061053.105377-1-yuchao0@huawei.com>
2021-03-05 3:29 ` [PATCH] configfs: Fix use-after-free issue in __configfs_open_file Chao Yu
2021-03-11 2:16 ` Chao Yu
2021-03-11 3:42 ` Al Viro
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).