All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] configfs: fix use-after-free of symlink target racing with rmdir
@ 2026-07-30  9:30 Vasileios Almpanis
  2026-07-30  9:30 ` [PATCH 1/2] configfs: pin the symlink target's dirent instead of chasing ->ci_dentry Vasileios Almpanis
  2026-07-30  9:30 ` [PATCH 2/2] configfs: unhash the dentry before dropping the item in rmdir Vasileios Almpanis
  0 siblings, 2 replies; 5+ messages in thread
From: Vasileios Almpanis @ 2026-07-30  9:30 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Breno Leitao, Al Viro, linux-fsdevel, linux-kernel, stable,
	syzbot+6b16e3d085833cbf3e25

syzkaller reported a slab-use-after-free in config_item_get() when
symlink(2) races with rmdir(2) of the symlink target:

  BUG: KASAN: slab-use-after-free in config_item_get+0x26/0x90
   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

configfs_symlink() resolves the target with no locks, with the idea
that a hashed dentry means a live config_item. configfs_rmdir() drops
the last reference to the item before the dentry gets unhashed by
d_delete() in vfs_rmdir(), so get_target() could take a reference on
an already freed item.

Patch 2 fixes this by unhashing the dentry in configfs_remove_dir(),
before the item can be freed.  Patch 1 fixes a second lifetime bug in
the same path that the earlier unhashing makes easy to hit: an item
reference does not pin the item's dentry, so create_link() must not
reach the target's configfs_dirent through ->ci_dentry.  The patches
must be applied in this order.

Tested with the syzkaller reproducer, which no longer triggers either
the KASAN report or the s_count warning.

Vasileios Almpanis (2):
  configfs: pin the symlink target's dirent instead of chasing
    ->ci_dentry
  configfs: unhash the dentry before dropping the item in rmdir

 fs/configfs/dir.c     |  9 +++++++++
 fs/configfs/symlink.c | 24 ++++++++++++++++++++----
 2 files changed, 29 insertions(+), 4 deletions(-)

-- 
2.47.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] configfs: pin the symlink target's dirent instead of chasing ->ci_dentry
  2026-07-30  9:30 [PATCH 0/2] configfs: fix use-after-free of symlink target racing with rmdir Vasileios Almpanis
@ 2026-07-30  9:30 ` Vasileios Almpanis
  2026-07-30  9:30 ` [PATCH 2/2] configfs: unhash the dentry before dropping the item in rmdir Vasileios Almpanis
  1 sibling, 0 replies; 5+ messages in thread
From: Vasileios Almpanis @ 2026-07-30  9:30 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Breno Leitao, Al Viro, linux-fsdevel, linux-kernel, stable,
	syzbot+6b16e3d085833cbf3e25

create_link() reads the target's configfs_dirent from
item->ci_dentry->d_fsdata, relying on the item reference taken by
get_target().  That reference pins the item, not its dentry: the dentry is
pinned by DCACHE_PERSISTENT, which configfs_remove_dir() releases via
simple_rmdir() while the item is still alive.  A symlink racing with rmdir
of its target can therefore find ->ci_dentry freed and its dirent
released, triggering WARN_ON(!atomic_read(&sd->s_count)) in configfs_get().

Take the dirent in get_target() as well, under ->d_lock and atomically
with the item reference, and pass it down to create_link().  A hashed
dentry has not been killed yet, so its ->d_fsdata reference keeps the
dirent alive there.

Cc: stable@vger.kernel.org
Fixes: 7063fbf22611 ("[PATCH] configfs: User-driven configuration filesystem")
Signed-off-by: Vasileios Almpanis <vasilisalmpanis@gmail.com>
---
 fs/configfs/symlink.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
index 31eb28b27309..3b31c714400f 100644
--- a/fs/configfs/symlink.c
+++ b/fs/configfs/symlink.c
@@ -76,9 +76,9 @@ static int configfs_get_target_path(struct config_item *item,
 
 static int create_link(struct config_item *parent_item,
 		       struct config_item *item,
+		       struct configfs_dirent *target_sd,
 		       struct dentry *dentry)
 {
-	struct configfs_dirent *target_sd = item->ci_dentry->d_fsdata;
 	char *body;
 	int ret;
 
@@ -115,6 +115,7 @@ static int create_link(struct config_item *parent_item,
 
 
 static int get_target(const char *symname, struct config_item **target,
+		      struct configfs_dirent **target_sd,
 		      struct super_block *sb)
 {
 	struct path path __free(path_put) = {};
@@ -125,7 +126,20 @@ static int get_target(const char *symname, struct config_item **target,
 		return ret;
 	if (path.dentry->d_sb != sb)
 		return -EPERM;
-	*target = configfs_get_config_item(path.dentry);
+	/*
+	 * A hashed dentry guarantees that neither the item nor the dirent
+	 * have been released yet, as removals unhash before dropping.
+	 * Grab both references here. An item reference alone would not keep
+	 * ->ci_dentry alive.
+	 */
+	spin_lock(&path.dentry->d_lock);
+	if (!d_unhashed(path.dentry)) {
+		struct configfs_dirent *sd = path.dentry->d_fsdata;
+
+		*target = config_item_get(sd->s_element);
+		*target_sd = configfs_get(sd);
+	}
+	spin_unlock(&path.dentry->d_lock);
 	if (!*target)
 		return -ENOENT;
 	return 0;
@@ -139,6 +153,7 @@ int configfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	struct configfs_dirent *sd;
 	struct config_item *parent_item;
 	struct config_item *target_item = NULL;
+	struct configfs_dirent *target_sd = NULL;
 	const struct config_item_type *type;
 
 	sd = dentry->d_parent->d_fsdata;
@@ -182,7 +197,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, &target_item, &target_sd, dentry->d_sb);
 	inode_lock(dir);
 	if (ret)
 		goto out_put;
@@ -196,13 +211,14 @@ int configfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 		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, dentry);
+		ret = create_link(parent_item, target_item, target_sd, dentry);
 		mutex_unlock(&configfs_symlink_mutex);
 		if (ret && type->ct_item_ops->drop_link)
 			type->ct_item_ops->drop_link(parent_item,
 						     target_item);
 	}
 
+	configfs_put(target_sd);
 	config_item_put(target_item);
 
 out_put:
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] configfs: unhash the dentry before dropping the item in rmdir
  2026-07-30  9:30 [PATCH 0/2] configfs: fix use-after-free of symlink target racing with rmdir Vasileios Almpanis
  2026-07-30  9:30 ` [PATCH 1/2] configfs: pin the symlink target's dirent instead of chasing ->ci_dentry Vasileios Almpanis
@ 2026-07-30  9:30 ` Vasileios Almpanis
  2026-07-31  9:29   ` Breno Leitao
  1 sibling, 1 reply; 5+ messages in thread
From: Vasileios Almpanis @ 2026-07-30  9:30 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Breno Leitao, Al Viro, linux-fsdevel, linux-kernel, stable,
	syzbot+6b16e3d085833cbf3e25

configfs_get_config_item() treats a hashed dentry as proof that
sd->s_element is a live config_item.  configfs_rmdir() breaks that:
simple_rmdir() leaves the dentry hashed, the last reference to the item is
dropped right after, and the dentry is only unhashed by d_delete() once
->rmdir() has returned.  configfs_symlink() resolves its target holding no
lock on it, so get_target() can land in that window:

  BUG: KASAN: slab-use-after-free in config_item_get+0x26/0x90
   get_target fs/configfs/symlink.c:128 [inline]
   configfs_symlink+0x4ab/0x1030 fs/configfs/symlink.c:185

Unhash in configfs_remove_dir(), while the item is still guaranteed to be
there.  A reference obtained just before that stays harmless, as
create_link() rechecks CONFIGFS_USET_DROPPING, already set by
configfs_detach_prep().  Both configfs_unregister_subsystem() paths
d_drop() after detaching, so this only makes rmdir match them.

Reported-by: syzbot+6b16e3d085833cbf3e25@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6b16e3d085833cbf3e25
Fixes: 7063fbf22611 ("[PATCH] configfs: User-driven configuration filesystem")
Cc: stable@vger.kernel.org
Signed-off-by: Vasileios Almpanis <vasilisalmpanis@gmail.com>
---
 fs/configfs/dir.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 3c88f13f1ca2..eda80c2a2d38 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -416,6 +416,15 @@ static void configfs_remove_dir(struct dentry *d)
 	if (d_really_is_positive(d)) {
 		if (unlikely(simple_rmdir(d_inode(parent), d)))
 			pr_warn("remove_dir (%pd): attributes remain", d);
+		else
+			/*
+			 * configfs_get_config_item() takes a hashed dentry as
+			 * proof that ->s_element is still alive.  Our caller
+			 * is about to drop the last reference to the item and
+			 * the VFS will not unhash until after we return, so
+			 * unhash it here.
+			 */
+			d_drop(d);
 	}
 
 	pr_debug(" o %pd removing done (%d)\n", d, d_count(d));
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] configfs: unhash the dentry before dropping the item in rmdir
  2026-07-30  9:30 ` [PATCH 2/2] configfs: unhash the dentry before dropping the item in rmdir Vasileios Almpanis
@ 2026-07-31  9:29   ` Breno Leitao
  2026-07-31  9:55     ` Vasileios Almpanis
  0 siblings, 1 reply; 5+ messages in thread
From: Breno Leitao @ 2026-07-31  9:29 UTC (permalink / raw)
  To: Vasileios Almpanis
  Cc: Andreas Hindborg, Al Viro, linux-fsdevel, linux-kernel, stable,
	syzbot+6b16e3d085833cbf3e25

On Thu, Jul 30, 2026 at 11:30:25AM +0200, Vasileios Almpanis wrote:
> configfs_get_config_item() treats a hashed dentry as proof that
> sd->s_element is a live config_item.  configfs_rmdir() breaks that:
> simple_rmdir() leaves the dentry hashed, the last reference to the item is
> dropped right after, and the dentry is only unhashed by d_delete() once
> ->rmdir() has returned.  configfs_symlink() resolves its target holding no
> lock on it, so get_target() can land in that window:
> 
>   BUG: KASAN: slab-use-after-free in config_item_get+0x26/0x90
>    get_target fs/configfs/symlink.c:128 [inline]
>    configfs_symlink+0x4ab/0x1030 fs/configfs/symlink.c:185
> 
> Unhash in configfs_remove_dir(), while the item is still guaranteed to be
> there.  A reference obtained just before that stays harmless, as
> create_link() rechecks CONFIGFS_USET_DROPPING, already set by
> configfs_detach_prep().  Both configfs_unregister_subsystem() paths
> d_drop() after detaching, so this only makes rmdir match them.

Is the claim that "a reference obtained just before that stays harmless,
as create_link() rechecks CONFIGFS_USET_DROPPING" accurate?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] configfs: unhash the dentry before dropping the item in rmdir
  2026-07-31  9:29   ` Breno Leitao
@ 2026-07-31  9:55     ` Vasileios Almpanis
  0 siblings, 0 replies; 5+ messages in thread
From: Vasileios Almpanis @ 2026-07-31  9:55 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andreas Hindborg, Al Viro, linux-fsdevel, linux-kernel, stable,
	syzbot+6b16e3d085833cbf3e25


On 7/31/26 11:29 AM, Breno Leitao wrote:
> On Thu, Jul 30, 2026 at 11:30:25AM +0200, Vasileios Almpanis wrote:
>> configfs_get_config_item() treats a hashed dentry as proof that
>> sd->s_element is a live config_item.  configfs_rmdir() breaks that:
>> simple_rmdir() leaves the dentry hashed, the last reference to the item is
>> dropped right after, and the dentry is only unhashed by d_delete() once
>> ->rmdir() has returned.  configfs_symlink() resolves its target holding no
>> lock on it, so get_target() can land in that window:
>>
>>    BUG: KASAN: slab-use-after-free in config_item_get+0x26/0x90
>>     get_target fs/configfs/symlink.c:128 [inline]
>>     configfs_symlink+0x4ab/0x1030 fs/configfs/symlink.c:185
>>
>> Unhash in configfs_remove_dir(), while the item is still guaranteed to be
>> there.  A reference obtained just before that stays harmless, as
>> create_link() rechecks CONFIGFS_USET_DROPPING, already set by
>> configfs_detach_prep().  Both configfs_unregister_subsystem() paths
>> d_drop() after detaching, so this only makes rmdir match them.
> Is the claim that "a reference obtained just before that stays harmless,
> as create_link() rechecks CONFIGFS_USET_DROPPING" accurate?
I believe so.  Everything configfs_remove_dir() will now d_drop(), had
DROPPING set on its dirent beforehand by configfs_detach_prep(), and
once detach_prep() has succeeded the flag is never cleared again on
that subtree.

The race is closed by configfs_dirent_lock. create_link() checks DROPPING
and incs ->s_links in one locked section, and detach_prep() set DROPPING
and checks ->s_links in another. Either symlink gets ->s_links++ in 
first and
rmdir fails with -EBUSY before unhashing, or DROPPING is set first and
create_link() fails with -ENOENT.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-31  9:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30  9:30 [PATCH 0/2] configfs: fix use-after-free of symlink target racing with rmdir Vasileios Almpanis
2026-07-30  9:30 ` [PATCH 1/2] configfs: pin the symlink target's dirent instead of chasing ->ci_dentry Vasileios Almpanis
2026-07-30  9:30 ` [PATCH 2/2] configfs: unhash the dentry before dropping the item in rmdir Vasileios Almpanis
2026-07-31  9:29   ` Breno Leitao
2026-07-31  9:55     ` Vasileios Almpanis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.