[PATCH] cifs: fix use-after-free on the link name
From: ChenXiaoSong
Date: Wed Nov 02 2022 - 01:14:09 EST
xfstests generic/011 reported use-after-free bug as follows:
BUG: KASAN: use-after-free in __d_alloc+0x269/0x859
Read of size 15 at addr ffff8880078933a0 by task dirstress/952
CPU: 1 PID: 952 Comm: dirstress Not tainted 6.1.0-rc3+ #77
Call Trace:
__dump_stack+0x23/0x29
dump_stack_lvl+0x51/0x73
print_address_description+0x67/0x27f
print_report+0x3e/0x5c
kasan_report+0x7b/0xa8
kasan_check_range+0x1b2/0x1c1
memcpy+0x22/0x5d
__d_alloc+0x269/0x859
d_alloc+0x45/0x20c
d_alloc_parallel+0xb2/0x8b2
lookup_open+0x3b8/0x9f9
open_last_lookups+0x63d/0xc26
path_openat+0x11a/0x261
do_filp_open+0xcc/0x168
do_sys_openat2+0x13b/0x3f7
do_sys_open+0x10f/0x146
__se_sys_creat+0x27/0x2e
__x64_sys_creat+0x55/0x6a
do_syscall_64+0x40/0x96
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Allocated by task 952:
kasan_save_stack+0x1f/0x42
kasan_set_track+0x21/0x2a
kasan_save_alloc_info+0x17/0x1d
__kasan_kmalloc+0x7e/0x87
__kmalloc_node_track_caller+0x59/0x155
kstrndup+0x60/0xe6
parse_mf_symlink+0x215/0x30b
check_mf_symlink+0x260/0x36a
cifs_get_inode_info+0x14e1/0x1690
cifs_revalidate_dentry_attr+0x70d/0x964
cifs_revalidate_dentry+0x36/0x62
cifs_d_revalidate+0x162/0x446
lookup_open+0x36f/0x9f9
open_last_lookups+0x63d/0xc26
path_openat+0x11a/0x261
do_filp_open+0xcc/0x168
do_sys_openat2+0x13b/0x3f7
do_sys_open+0x10f/0x146
__se_sys_creat+0x27/0x2e
__x64_sys_creat+0x55/0x6a
do_syscall_64+0x40/0x96
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Freed by task 950:
kasan_save_stack+0x1f/0x42
kasan_set_track+0x21/0x2a
kasan_save_free_info+0x1c/0x34
____kasan_slab_free+0x1c1/0x1d5
__kasan_slab_free+0xe/0x13
__kmem_cache_free+0x29a/0x387
kfree+0xd3/0x10e
cifs_fattr_to_inode+0xb6a/0xc8c
cifs_get_inode_info+0x3cb/0x1690
cifs_revalidate_dentry_attr+0x70d/0x964
cifs_revalidate_dentry+0x36/0x62
cifs_d_revalidate+0x162/0x446
lookup_open+0x36f/0x9f9
open_last_lookups+0x63d/0xc26
path_openat+0x11a/0x261
do_filp_open+0xcc/0x168
do_sys_openat2+0x13b/0x3f7
do_sys_open+0x10f/0x146
__se_sys_creat+0x27/0x2e
__x64_sys_creat+0x55/0x6a
do_syscall_64+0x40/0x96
entry_SYSCALL_64_after_hwframe+0x63/0xcd
When opened a symlink, link name is from 'inode->i_link', but it may be
reset to a new value when revalidate the dentry. If some processes get the
link name on the race scenario, then UAF will happen on link name.
Fix this by implementing 'get_link' interface to duplicate the link name.
Fixes: 76894f3e2f71 ("cifs: improve symlink handling for smb2+")
Signed-off-by: ChenXiaoSong <chenxiaosong2@xxxxxxxxxx>
---
fs/cifs/cifsfs.c | 21 ++++++++++++++++++++-
fs/cifs/inode.c | 5 -----
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index d0b9fec111aa..bb9592594fcc 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -1143,8 +1143,27 @@ const struct inode_operations cifs_file_inode_ops = {
.fiemap = cifs_fiemap,
};
+const char *cifs_get_link(struct dentry *dentry, struct inode *inode,
+ struct delayed_call *done)
+{
+ char *target_path = NULL;
+
+ spin_lock(&inode->i_lock);
+ if (likely(CIFS_I(inode)->symlink_target))
+ target_path = kstrdup(CIFS_I(inode)->symlink_target,
+ GFP_ATOMIC);
+ spin_unlock(&inode->i_lock);
+
+ if (target_path)
+ set_delayed_call(done, kfree_link, target_path);
+ else
+ target_path = ERR_PTR(-EOPNOTSUPP);
+
+ return target_path;
+}
+
const struct inode_operations cifs_symlink_inode_ops = {
- .get_link = simple_get_link,
+ .get_link = cifs_get_link,
.permission = cifs_permission,
.listxattr = cifs_listxattr,
};
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 9bde08d44617..4e2ca3c6e5c0 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -215,11 +215,6 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
kfree(cifs_i->symlink_target);
cifs_i->symlink_target = fattr->cf_symlink_target;
fattr->cf_symlink_target = NULL;
-
- if (unlikely(!cifs_i->symlink_target))
- inode->i_link = ERR_PTR(-EOPNOTSUPP);
- else
- inode->i_link = cifs_i->symlink_target;
}
spin_unlock(&inode->i_lock);
--
2.31.1