[PATCH] configfs: fix race between symlink and rmdir

[Jeffin Philip <jeffinphilip14@gmail.com>]

From: syzbot <syzbot@kernel.org>

A race condition between configfs_rmdir() and configfs_symlink() can lead
to a use-after-free of a config_item.

When a directory is removed, configfs_rmdir() frees the associated
config_item but leaves the dentry hashed until vfs_rmdir() calls
d_delete(). This creates a window where a concurrent symlinkat() can find
the hashed dentry, see that !d_unhashed(dentry) is true, and attempt to
increment the refcount of the already freed config_item, triggering a
warning:

  refcount_t: addition on 0; use-after-free.
  WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x9f/0x110
  ...
  Call Trace:
   __refcount_add include/linux/refcount.h:-1 [inline]
   __refcount_inc include/linux/refcount.h:366 [inline]
   refcount_inc include/linux/refcount.h:383 [inline]
   kref_get include/linux/kref.h:45 [inline]
   config_item_get+0x88/0x90 fs/configfs/item.c:104
   configfs_get_config_item fs/configfs/configfs_internal.h:127 [inline]
   get_target fs/configfs/symlink.c:128 [inline]
   configfs_symlink+0x4ab/0x1030 fs/configfs/symlink.c:185
   vfs_symlink+0x18b/0x330 fs/namei.c:5660
   filename_symlinkat+0x1cd/0x420 fs/namei.c:5685
   __do_sys_symlinkat fs/namei.c:5705 [inline]
   __se_sys_symlinkat+0x4e/0x2b0 fs/namei.c:5700

To fix this, add d_drop(dentry) in configfs_rmdir() immediately after
detaching the group/item and before unlinking the object. This ensures that
any concurrent kern_path() will either not find the dentry, or
configfs_get_config_item() will see d_unhashed(dentry) == true and safely
return NULL.

However, adding d_drop(dentry) exposes a secondary use-after-free bug in
create_link(). configfs_symlink() calls get_target(), which resolves the
path, gets the config_item, and then immediately drops the path reference.
If configfs_rmdir() runs concurrently, it drops its config_item reference,
and vfs_rmdir() calls d_delete_notify(). Since the dentry has only 1
reference left, dentry_unlink_inode() is called, freeing the
configfs_dirent. Later, create_link() accesses the freed configfs_dirent.

To fix this secondary issue, modify get_target() to return the path, and
hold the path reference in configfs_symlink() until after create_link()
finishes. By holding the path reference, the dentry's refcount remains >=
2, preventing d_delete() from calling dentry_unlink_inode(), which keeps
the configfs_dirent alive. The path is safely cleaned up using the
__free(path_put) attribute in configfs_symlink().

Fixes: 98702467f829 ("configfs: remove unnecessary dentry_unhash on rmdir, dir rename")
Assisted-by: Gemini:gemini-3.5-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+8358d1f3d9c15bdf1c9a@syzkaller.appspotmail.com
Tested-by: Jeffin Philip <jeffinphilip14@gmail.com>
Reviewed-by: Jeffin Philip <jeffinphilip14@gmail.com>
Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=8358d1f3d9c15bdf1c9a
Link: https://syzkaller.appspot.com/ai_job?id=13fec684-23f7-4f06-99db-ed37ae92a405
Cc: Breno Leitao <leitao@debian.org>
---
 fs/configfs/dir.c     |  2 ++
 fs/configfs/symlink.c | 14 +++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 3c88f13f1ca2..b0812432f6c5 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1552,12 +1552,14 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry)
 
 	if (sd->s_type & CONFIGFS_USET_DIR) {
 		configfs_detach_group(dentry);
+		d_drop(dentry);
 
 		mutex_lock(&subsys->su_mutex);
 		client_disconnect_notify(parent_item, item);
 		unlink_group(to_config_group(item));
 	} else {
 		configfs_detach_item(dentry);
+		d_drop(dentry);
 
 		mutex_lock(&subsys->su_mutex);
 		client_disconnect_notify(parent_item, item);
diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
index 31eb28b27309..c8a2d9ad6cac 100644
--- a/fs/configfs/symlink.c
+++ b/fs/configfs/symlink.c
@@ -114,18 +114,17 @@ static int create_link(struct config_item *parent_item,
 }
 
 
-static int get_target(const char *symname, struct config_item **target,
-		      struct super_block *sb)
+static int get_target(const char *symname, struct path *path,
+		      struct config_item **target, struct super_block *sb)
 {
-	struct path path __free(path_put) = {};
 	int ret;
 
-	ret = kern_path(symname, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, &path);
+	ret = kern_path(symname, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, path);
 	if (ret)
 		return ret;
-	if (path.dentry->d_sb != sb)
+	if (path->dentry->d_sb != sb)
 		return -EPERM;
-	*target = configfs_get_config_item(path.dentry);
+	*target = configfs_get_config_item(path->dentry);
 	if (!*target)
 		return -ENOENT;
 	return 0;
@@ -136,6 +135,7 @@ int configfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 		     struct dentry *dentry, const char *symname)
 {
 	int ret;
+	struct path path __free(path_put) = {};
 	struct configfs_dirent *sd;
 	struct config_item *parent_item;
 	struct config_item *target_item = NULL;
@@ -182,7 +182,7 @@ int configfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	 *  AV, a thoroughly annoyed bastard.
 	 */
 	inode_unlock(dir);
-	ret = get_target(symname, &target_item, dentry->d_sb);
+	ret = get_target(symname, &path, &target_item, dentry->d_sb);
 	inode_lock(dir);
 	if (ret)
 		goto out_put;
-- 
2.55.0