[PATCH] configfs: protect symlink source callbacks from removal

[Yingjie Wang <x-professor@outlook.com>]

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