[PATCH v2 2/2] configfs: pin symlink target against rmdir before ->allow_link()

[Izaac Chong <izaacchong934@gmail.com>]

configfs_symlink() looks up the link target with get_target(), which
takes references on the target item and dirent but does not stop the
target from being removed. The target is only pinned (target_sd->s_links
incremented, refusing CONFIGFS_USET_DROPPING) later, in create_link(),
after ->allow_link() has run.

A concurrent rmdir of the target can therefore complete in between.
unlink_obj() then clears target->ci_parent and ->ci_group, and
->allow_link() implementations that walk the target's parents
dereference NULL. With the USB gadget UVC function:

  BUG: KASAN: null-ptr-deref in uvcg_streaming_header_allow_link+0xc3/0x270
  Read of size 8 at addr 0000000000000030
  Call Trace:
   uvcg_streaming_header_allow_link
   configfs_symlink
   vfs_symlink
   filename_symlinkat
   __x64_sys_symlink

target_fabric_mappedlun_link() also walks the target's ->ci_parent.

Take the s_links pin before calling ->allow_link() instead, under
configfs_dirent_lock and only if the target is not being dropped, as
create_link() did. configfs_detach_prep() sets CONFIGFS_USET_DROPPING
before checking s_links under the same lock, so the target either fails
to pin (-ENOENT) or rmdir sees the pin and returns -EBUSY. On success the
link keeps the count and configfs_unlink() drops it as before; on failure
it is dropped again.

Fixes: 4768e9b18dc6 ("[PATCH] configfs: Fix symlink() to a removing item")
Link: https://sashiko.dev/#/bug/linux-df6ae898-b1a6-4091-be3b-fbe4c177763f
Signed-off-by: Izaac Chong <izaacchong934@gmail.com>
---
 fs/configfs/symlink.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
index 1b2c9048a920..52f0f3a6cd7e 100644
--- a/fs/configfs/symlink.c
+++ b/fs/configfs/symlink.c
@@ -90,23 +90,11 @@ static int create_link(struct config_item *parent_item,
 		return -ENOMEM;
 
 	configfs_get(target_sd);
-	spin_lock(&configfs_dirent_lock);
-	if (target_sd->s_type & CONFIGFS_USET_DROPPING) {
-		spin_unlock(&configfs_dirent_lock);
-		configfs_put(target_sd);
-		kfree(body);
-		return -ENOENT;
-	}
-	target_sd->s_links++;
-	spin_unlock(&configfs_dirent_lock);
 	ret = configfs_get_target_path(parent_item, item, body);
 	if (!ret)
 		ret = configfs_create_link(target_sd, parent_item->ci_dentry,
 					   dentry, body);
 	if (ret) {
-		spin_lock(&configfs_dirent_lock);
-		target_sd->s_links--;
-		spin_unlock(&configfs_dirent_lock);
 		configfs_put(target_sd);
 		kfree(body);
 	}
@@ -220,8 +208,28 @@ 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_put_parent;
+
+	/*
+	 * Pin the target against rmdir before calling ->allow_link().
+	 * configfs_detach_prep() refuses to remove a dirent with s_links
+	 * set, and sets CONFIGFS_USET_DROPPING (under configfs_dirent_lock)
+	 * before it checks s_links.  Without the pin, a concurrent rmdir can
+	 * unlink the target (clearing ->ci_parent and ->ci_group) while
+	 * ->allow_link() looks at it.  On success the link owns the count,
+	 * which configfs_unlink() drops.
+	 */
+	spin_lock(&configfs_dirent_lock);
+	if (target_sd->s_type & CONFIGFS_USET_DROPPING)
+		ret = -ENOENT;
+	else
+		target_sd->s_links++;
+	spin_unlock(&configfs_dirent_lock);
+	if (ret)
+		goto out_put_parent;
+
+	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);
@@ -230,7 +238,13 @@ int configfs_symlink(const struct mnt_idmap *idmap, struct inode *dir,
 			type->ct_item_ops->drop_link(parent_item,
 						     target_item);
 	}
+	if (ret) {
+		spin_lock(&configfs_dirent_lock);
+		target_sd->s_links--;
+		spin_unlock(&configfs_dirent_lock);
+	}
 
+out_put_parent:
 	config_item_put(parent_item);
 out_put_target:
 	configfs_put(target_sd);

-- 
Git-143)