From: Yingjie Wang <1075151112@qq.com> configfs_symlink() drops the source directory inode lock while resolving its target. A concurrent rmdir() of the source item can take that lock, detach the item and clear its ci_group in unlink_obj(). configfs_symlink() then reacquires the lock and calls the source ->allow_link() callback on the detached item. Its config_item reference keeps the object allocated, but not attached to the hierarchy. The UVC gadget format callback reads src->ci_group->cg_subsys to take the subsystem mutex and faults on the cleared pointer. Racing symlink() into a UVC format directory with rmdir() of that directory produces: BUG: KASAN: null-ptr-deref in uvcg_format_allow_link+0x2a/0x140 Read of size 8 at addr 0000000000000060 by task configfs_race/291 Call Trace: uvcg_format_allow_link+0x2a/0x140 configfs_symlink+0x3f2/0x870 vfs_symlink+0xc0/0x2a0 filename_symlinkat+0x118/0x2b0 __x64_sys_symlink+0x50/0x70 Creating the items needs the privileges required to manage configfs. Rechecking ci_group after relocking would cover rmdir(), which holds the source inode lock until unlink_obj() returns. It would not cover configfs_unregister_group() and configfs_unregister_subsystem(): both clear ci_group in unlink_group() while the source inode lock is not held, so the check could become stale before the callback runs. Take the source fragment read lock after reacquiring the inode lock and hold it through allow_link(), create_link() and drop_link(). rmdir() and both unregister paths mark the fragment dead under the write lock before they unlink the item, so a concurrent removal either waits for the callback to finish or makes the symlink fail with -ENOENT. Built from the same configuration, the kernel without this change faulted within 1,000 iterations, while the kernel with it completed 10,000 iterations with no report. The reproducer covers the rmdir() path; the unregister paths were checked by code inspection. Fixes: 351e5d869e5a ("configfs: fix a deadlock in configfs_symlink()") Cc: stable@vger.kernel.org Signed-off-by: Yingjie Wang <1075151112@qq.com> --- The target-side races were fixed by commits a7c1290eef60 ("configfs: pin the symlink target's dirent instead of chasing ->ci_dentry") and f06c2d26d199 ("configfs: unhash the dentry before dropping the item in rmdir"). Those changes protect removal of the link target. This patch protects the item that receives the link and is independent. fs/configfs/symlink.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c index 89178f5d371a..a2a69801b14d 100644 --- a/fs/configfs/symlink.c +++ b/fs/configfs/symlink.c @@ -151,6 +151,7 @@ int configfs_symlink(const struct mnt_idmap *idmap, struct inode *dir, { int ret; struct configfs_dirent *sd; + struct configfs_fragment *frag; struct config_item *parent_item; struct config_item *target_item = NULL; struct configfs_dirent *target_sd = NULL; @@ -207,8 +208,17 @@ int configfs_symlink(const struct mnt_idmap *idmap, struct inode *dir, else ret = inode_permission(&nop_mnt_idmap, dir, MAY_WRITE | MAY_EXEC); - if (!ret) - ret = type->ct_item_ops->allow_link(parent_item, target_item); + if (ret) + goto out_target; + + frag = sd->s_frag; + down_read(&frag->frag_sem); + if (frag->frag_dead) { + ret = -ENOENT; + goto out_unlock; + } + + ret = type->ct_item_ops->allow_link(parent_item, target_item); if (!ret) { mutex_lock(&configfs_symlink_mutex); ret = create_link(parent_item, target_item, target_sd, dentry); @@ -218,6 +228,10 @@ int configfs_symlink(const struct mnt_idmap *idmap, struct inode *dir, target_item); } +out_unlock: + up_read(&frag->frag_sem); + +out_target: configfs_put(target_sd); config_item_put(target_item); base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d -- 2.43.0