Commit 14fbbc82 authored by Daiyue Zhang's avatar Daiyue Zhang Committed by Christoph Hellwig

configfs: fix a use-after-free in __configfs_open_file

Commit b0841eef ("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: b0841eef ("configfs: provide exclusion between IO and removals")
Signed-off-by: default avatarDaiyue Zhang <zhangdaiyue1@huawei.com>
Signed-off-by: default avatarYi Chen <chenyi77@huawei.com>
Signed-off-by: default avatarGe Qiu <qiuge@huawei.com>
Reviewed-by: default avatarChao Yu <yuchao0@huawei.com>
Acked-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
parent a74e6a01
...@@ -378,7 +378,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type ...@@ -378,7 +378,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type
attr = to_attr(dentry); attr = to_attr(dentry);
if (!attr) if (!attr)
goto out_put_item; goto out_free_buffer;
if (type & CONFIGFS_ITEM_BIN_ATTR) { if (type & CONFIGFS_ITEM_BIN_ATTR) {
buffer->bin_attr = to_bin_attr(dentry); buffer->bin_attr = to_bin_attr(dentry);
...@@ -391,7 +391,7 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type ...@@ -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 */ /* Grab the module reference for this attribute if we have one */
error = -ENODEV; error = -ENODEV;
if (!try_module_get(buffer->owner)) if (!try_module_get(buffer->owner))
goto out_put_item; goto out_free_buffer;
error = -EACCES; error = -EACCES;
if (!buffer->item->ci_type) if (!buffer->item->ci_type)
...@@ -435,8 +435,6 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type ...@@ -435,8 +435,6 @@ static int __configfs_open_file(struct inode *inode, struct file *file, int type
out_put_module: out_put_module:
module_put(buffer->owner); module_put(buffer->owner);
out_put_item:
config_item_put(buffer->item);
out_free_buffer: out_free_buffer:
up_read(&frag->frag_sem); up_read(&frag->frag_sem);
kfree(buffer); kfree(buffer);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment