[PATCH 1/2] configfs: handle unlinked target in configfs_depend_item_unlocked()

[PATCH 1/2] configfs: handle unlinked target in configfs_depend_item_unlocked()

[Slawomir Stepien]

configfs_depend_item_unlocked() starts by reading target->ci_group to
find the target's subsystem and the configfs root, and dereferences it
without checking for NULL. ci_group is only valid while the item is
linked into the tree: it is set by link_group() during mkdir and
cleared by unlink_group() during rmdir, both under the subsystem's
su_mutex, which callers of the unlocked variant do not hold.

A caller can therefore pass an item that is allocated but not linked.
f_tcm does this from usbg_make_tpg(): a TCM function instance becomes
visible to the target subsystem in tcm_alloc_inst()/tcm_set_name(),
before configfs_mkdir() links it, and stays visible after
configfs_rmdir() unlinks it until the final reference is dropped.
Creating a TPG in either window makes configfs_is_root(&parent->cg_item)
return false for a NULL parent, and the following parent->cg_subsys
dereference crashes.

An unlinked item is just a target that is not in the tree, which
configfs_depend_prep() already reports as -ENOENT. It skips items
marked CREATING or DROPPING, and rmdir marks an item DROPPING before
unlinking it. Read ci_group once with READ_ONCE() and return -ENOENT
if it is NULL.

If the target is unlinked right after the read, the snapshot still
points at the parent group: unlinking drops the child's reference on
it but does not free it. The depend then fails in the tree scan, since
the target is already marked DROPPING. The parent can only go away if
it is itself removed in that window, which is the same theoretical
race the root walk below already has for every caller of this
function.

Fixes: d79d75b5c518 ("fs: configfs: Add unlocked version of configfs_depend_item()")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 fs/configfs/dir.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 0c80feec5926..7f4f448a6c66 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1243,7 +1243,14 @@ int configfs_depend_item_unlocked(struct configfs_subsystem *caller_subsys,
 	if (configfs_is_root(target))
 		return -EINVAL;
 
-	parent = target->ci_group;
+	/*
+	 * The target may not be linked into the tree yet (mkdir in progress)
+	 * or may already have been unlinked (rmdir in progress).
+	 */
+	parent = READ_ONCE(target->ci_group);
+	if (!parent)
+		return -ENOENT;
+
 	/*
 	 * This may happen when someone is trying to depend root
 	 * directory of some subsystem
-- 
2.56.0

[PATCH 2/2] configfs: give the root inode its own lockdep class

[Slawomir Stepien]

configfs_depend_item_unlocked() is meant to be called from configfs
callbacks, where VFS already holds directory inode locks and configfs
holds the subsystem's su_mutex. When the target item lives in another
subsystem, it additionally locks the configfs root inode so that the
target subsystem cannot be unregistered underneath it.

The root inode shares the &sb->s_type->i_mutex_key lockdep class with
every user-created configfs directory, so lockdep cannot tell it apart
from the directory inode locks taken by mkdir, rmdir and symlink. Any
lock held by the depend caller therefore looks like it nests both
inside and outside "the" configfs inode lock. f_tcm's usbg_make_tpg(),
which depends on a gadget function item from the target subsystem's
mkdir callback, triggers this in two ways:

  target su_mutex -> root inode (depend), while
  rmdir under target/: directory inode -> target su_mutex

  tpg_instances_lock -> root inode (depend), while
  mkdir functions/tcm.N: directory inode -> gadget su_mutex ->
    func_lock -> tpg_instances_lock (tcm_alloc_inst())

Neither is a real deadlock, because no holder of the root inode lock
waits for su_mutex, tpg_instances_lock or a directory inode lock held
by a depend caller:

 - configfs_depend_item() and configfs_depend_item_unlocked() only look
   up the subsystem and scan the tree under configfs_dirent_lock, a
   spinlock.
 - Lookup and readdir in the configfs root only take
   configfs_dirent_lock. mkdir and symlink in the root fail with
   -EPERM before any other lock is taken, and configfs_rmdir() of a
   subsystem directory never gets as far as taking su_mutex.
 - configfs_register_subsystem() only locks the inodes of the new
   subsystem directory it is creating, which nobody else can hold yet.
 - configfs_unregister_subsystem() does wait for the subsystem
   directory's inode lock, but it only runs from the subsystem module's
   exit path. Every mkdir and rmdir in that subsystem holds a reference
   on the module, so none of them can be in progress at that point.

Put the root inode's i_rwsem into its own lock class so that lockdep
stops aliasing it with ordinary directory inodes. Splitting a class
cannot create new lockdep cycles, and real dependencies on the root
inode are still tracked under the new class. This is an annotation
only: without CONFIG_LOCKDEP the key is empty and lockdep_set_class()
compiles away.

Fixes: d79d75b5c518 ("fs: configfs: Add unlocked version of configfs_depend_item()")
Reported-by: syzbot+b3410eb576b568216ae5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b3410eb576b568216ae5
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 fs/configfs/mount.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index d8cac1cbf3bd..f5e9b98c1315 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -15,11 +15,19 @@
 #include <linux/fs_context.h>
 #include <linux/pagemap.h>
 #include <linux/init.h>
+#include <linux/lockdep.h>
 #include <linux/slab.h>
 
 #include <linux/configfs.h>
 #include "configfs_internal.h"
 
+/*
+ * configfs_depend_item_unlocked() locks the root inode while the caller holds
+ * su_mutex and other locks from a configfs callback; nothing taken under the
+ * root inode lock depends on those, so keep it out of the directory class.
+ */
+static struct lock_class_key configfs_root_inode_class;
+
 static struct vfsmount *configfs_mount = NULL;
 struct kmem_cache *configfs_dir_cachep;
 static int configfs_mnt_count = 0;
@@ -74,6 +82,7 @@ static int configfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (inode) {
 		inode->i_op = &configfs_root_inode_operations;
 		inode->i_fop = &configfs_dir_operations;
+		lockdep_set_class(&inode->i_rwsem, &configfs_root_inode_class);
 		/* directory inodes start off with i_nlink == 2 (for "." entry) */
 		inc_nlink(inode);
 	} else {
-- 
2.56.0

Re: [PATCH 1/2] configfs: handle unlinked target in configfs_depend_item_unlocked()

[Breno Leitao]

On Thu, Oct 22, 2026 at 03:44:42PM +0200, Slawomir Stepien wrote:
> configfs_depend_item_unlocked() starts by reading target->ci_group to
> find the target's subsystem and the configfs root, and dereferences it
> without checking for NULL. ci_group is only valid while the item is
> linked into the tree: it is set by link_group() during mkdir and
> cleared by unlink_group() during rmdir, both under the subsystem's
> su_mutex, which callers of the unlocked variant do not hold.
> 
> A caller can therefore pass an item that is allocated but not linked.
> f_tcm does this from usbg_make_tpg(): a TCM function instance becomes
> visible to the target subsystem in tcm_alloc_inst()/tcm_set_name(),
> before configfs_mkdir() links it, and stays visible after
> configfs_rmdir() unlinks it until the final reference is dropped.
> Creating a TPG in either window makes configfs_is_root(&parent->cg_item)
> return false for a NULL parent, and the following parent->cg_subsys
> dereference crashes.
> 
> An unlinked item is just a target that is not in the tree, which
> configfs_depend_prep() already reports as -ENOENT. It skips items
> marked CREATING or DROPPING, and rmdir marks an item DROPPING before
> unlinking it. Read ci_group once with READ_ONCE() and return -ENOENT
> if it is NULL.
> 
> If the target is unlinked right after the read, the snapshot still
> points at the parent group: unlinking drops the child's reference on
> it but does not free it. The depend then fails in the tree scan, since
> the target is already marked DROPPING. The parent can only go away if
> it is itself removed in that window, which is the same theoretical
> race the root walk below already has for every caller of this
> function.
> 
> Fixes: d79d75b5c518 ("fs: configfs: Add unlocked version of configfs_depend_item()")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>

Reviewed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH 2/2] configfs: give the root inode its own lockdep class

[Breno Leitao]

On Thu, Oct 22, 2026 at 03:44:43PM +0200, Slawomir Stepien wrote:
> configfs_depend_item_unlocked() is meant to be called from configfs
> callbacks, where VFS already holds directory inode locks and configfs
> holds the subsystem's su_mutex. When the target item lives in another
> subsystem, it additionally locks the configfs root inode so that the
> target subsystem cannot be unregistered underneath it.
> 
> The root inode shares the &sb->s_type->i_mutex_key lockdep class with
> every user-created configfs directory, so lockdep cannot tell it apart
> from the directory inode locks taken by mkdir, rmdir and symlink. Any
> lock held by the depend caller therefore looks like it nests both
> inside and outside "the" configfs inode lock. f_tcm's usbg_make_tpg(),
> which depends on a gadget function item from the target subsystem's
> mkdir callback, triggers this in two ways:
> 
>   target su_mutex -> root inode (depend), while
>   rmdir under target/: directory inode -> target su_mutex
> 
>   tpg_instances_lock -> root inode (depend), while
>   mkdir functions/tcm.N: directory inode -> gadget su_mutex ->
>     func_lock -> tpg_instances_lock (tcm_alloc_inst())
> 
> Neither is a real deadlock, because no holder of the root inode lock
> waits for su_mutex, tpg_instances_lock or a directory inode lock held
> by a depend caller:
> 
>  - configfs_depend_item() and configfs_depend_item_unlocked() only look
>    up the subsystem and scan the tree under configfs_dirent_lock, a
>    spinlock.
>  - Lookup and readdir in the configfs root only take
>    configfs_dirent_lock. mkdir and symlink in the root fail with
>    -EPERM before any other lock is taken, and configfs_rmdir() of a
>    subsystem directory never gets as far as taking su_mutex.
>  - configfs_register_subsystem() only locks the inodes of the new
>    subsystem directory it is creating, which nobody else can hold yet.
>  - configfs_unregister_subsystem() does wait for the subsystem
>    directory's inode lock, but it only runs from the subsystem module's
>    exit path. Every mkdir and rmdir in that subsystem holds a reference
>    on the module, so none of them can be in progress at that point.
> 
> Put the root inode's i_rwsem into its own lock class so that lockdep
> stops aliasing it with ordinary directory inodes. Splitting a class
> cannot create new lockdep cycles, and real dependencies on the root
> inode are still tracked under the new class. This is an annotation
> only: without CONFIG_LOCKDEP the key is empty and lockdep_set_class()
> compiles away.
> 
> Fixes: d79d75b5c518 ("fs: configfs: Add unlocked version of configfs_depend_item()")
> Reported-by: syzbot+b3410eb576b568216ae5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b3410eb576b568216ae5
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>

Reviewed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH 1/2] configfs: handle unlinked target in configfs_depend_item_unlocked()

[Breno Leitao]

On Thu, 22 Oct 2026 15:44:42 +0200, Slawomir Stepien wrote:
> configfs_depend_item_unlocked() starts by reading target->ci_group to
> find the target's subsystem and the configfs root, and dereferences it
> without checking for NULL. ci_group is only valid while the item is
> linked into the tree: it is set by link_group() during mkdir and
> cleared by unlink_group() during rmdir, both under the subsystem's
> su_mutex, which callers of the unlocked variant do not hold.
> 
> [...]

Applied, thanks!

[1/2] configfs: handle unlinked target in configfs_depend_item_unlocked()
      https://git.kernel.org/pub/scm/linux/kernel/git/leitao/linux.git/commit/?id=d678ac1c74dc
[2/2] configfs: give the root inode its own lockdep class
      https://git.kernel.org/pub/scm/linux/kernel/git/leitao/linux.git/commit/?id=e6f2f0945f5e

Best regards,
-- 
Breno Leitao <leitao@debian.org>