[PATCH] super: make iterate_supers_type() deletion-safe

[PATCH] super: make iterate_supers_type() deletion-safe

[Karl Mehltretter]

iterate_supers_type() drops sb_lock while invoking the callback and keeps
only a passive reference to the current superblock. That reference keeps
the object allocated, but does not keep its s_instances node linked.

After the callback releases s_umount, final teardown can unlink the current
s_instances node. The iterator then advances through a reinitialized node.
With the current hlist it stops without visiting the remaining superblocks.
The unlink moved from generic_shutdown_super() to kill_super_notify(), but
the cursor lifetime has been unsafe since the helper was introduced.

The CIFS DFS lookup can consequently miss a matching superblock and return
-EINVAL.

Walk the global superblock list in reverse and filter it by filesystem
type. A passive reference keeps its s_list node linked, and reverse
traversal preserves newest-first visitation. Superblocks removed from
fs_supers remain on the global list, but teardown marks them SB_DYING
before unlinking them, so the existing filter excludes them.

This broadens the scan from superblocks of one type to all superblocks.
The only in-tree caller is the CIFS DFS lookup, so the broader scan is
limited to that path.

Fixes: 43e15cdbefea ("new helper: iterate_supers_type()")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Testing: x86_64 QEMU deterministic KUnit A/B using the actual CIFS lookup
callback over temporary VFS superblocks. Baseline skipped the surviving
match after teardown of the preceding nonmatch and returned -EINVAL. With
only this patch applied, the same test passed.

A real Samba DFS server and the kernel CIFS client were then run in the
same QEMU guest over loopback. Baseline and patch-only kernels both followed
the referrals and completed 20 reconnects while two workers repeatedly
mounted and unmounted independent CIFS superblocks. The patch-only run
successfully mounted, read from, and unmounted CIFS shares 95 times. Kprobes
recorded 110 DFS lookup calls and 60 iterate_supers_type() calls. There were
no unmount failures, kernel warnings, oopses, or sanitizer reports. The
natural network stress did not hit the narrow race on baseline.

Stable is requested because concurrent DFS automount teardown can make a
reconnect lookup miss a live matching superblock.

Backport note: before dc3216b14160 ("super: ensure valid info"), the
s_instances unlink is in generic_shutdown_super(). Before 3ec9800c2d33
("super: convert s_count to refcount_t s_passive"), the passive reference is
named s_count.

 fs/super.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index 05e4431730387..c8accd144bad2 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1026,11 +1026,18 @@ void iterate_supers_type(struct file_system_type *type,
 	struct super_block *sb, *p = NULL;
 
 	spin_lock(&sb_lock);
-	hlist_for_each_entry(sb, &type->fs_supers, s_instances) {
+	/*
+	 * The passive reference keeps the s_list cursor valid while sb_lock
+	 * is dropped. Entries are added at the tail. Walk backwards to retain
+	 * newest-first visitation.
+	 */
+	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
 		bool locked;
 
 		if (super_flags(sb, SB_DYING))
 			continue;
+		if (sb->s_type != type)
+			continue;
 
 		if (!refcount_inc_not_zero(&sb->s_passive))
 			continue;
-- 
2.53.0

Re: [PATCH] super: make iterate_supers_type() deletion-safe

[Jan Kara]

On Thu 03-09-26 03:33:36, Karl Mehltretter wrote:
> iterate_supers_type() drops sb_lock while invoking the callback and keeps
> only a passive reference to the current superblock. That reference keeps
> the object allocated, but does not keep its s_instances node linked.
> 
> After the callback releases s_umount, final teardown can unlink the current
> s_instances node. The iterator then advances through a reinitialized node.
> With the current hlist it stops without visiting the remaining superblocks.
> The unlink moved from generic_shutdown_super() to kill_super_notify(), but
> the cursor lifetime has been unsafe since the helper was introduced.
> 
> The CIFS DFS lookup can consequently miss a matching superblock and return
> -EINVAL.
> 
> Walk the global superblock list in reverse and filter it by filesystem
> type. A passive reference keeps its s_list node linked, and reverse
> traversal preserves newest-first visitation. Superblocks removed from
> fs_supers remain on the global list, but teardown marks them SB_DYING
> before unlinking them, so the existing filter excludes them.
> 
> This broadens the scan from superblocks of one type to all superblocks.
> The only in-tree caller is the CIFS DFS lookup, so the broader scan is
> limited to that path.
> 
> Fixes: 43e15cdbefea ("new helper: iterate_supers_type()")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>

Good spotting! But what I'm wondering about is whether we just shouldn't
move where we delete sb from fs_supers. Currently we do that in
kill_super_notify() to hide the sb from sget_fc() (which would otherwise
permanently retry and call test() for S_DEAD sb which can cause problems).
But if we just skipped S_DEAD superblocks in sget_fc(), we could move
removal from fs_supers list to put_super() (make it symmetric with the
handling of super_blocks list) and that would also fix
iterate_supers_type(). Christian, what do you think?

								Honza

> ---
> Testing: x86_64 QEMU deterministic KUnit A/B using the actual CIFS lookup
> callback over temporary VFS superblocks. Baseline skipped the surviving
> match after teardown of the preceding nonmatch and returned -EINVAL. With
> only this patch applied, the same test passed.
> 
> A real Samba DFS server and the kernel CIFS client were then run in the
> same QEMU guest over loopback. Baseline and patch-only kernels both followed
> the referrals and completed 20 reconnects while two workers repeatedly
> mounted and unmounted independent CIFS superblocks. The patch-only run
> successfully mounted, read from, and unmounted CIFS shares 95 times. Kprobes
> recorded 110 DFS lookup calls and 60 iterate_supers_type() calls. There were
> no unmount failures, kernel warnings, oopses, or sanitizer reports. The
> natural network stress did not hit the narrow race on baseline.
> 
> Stable is requested because concurrent DFS automount teardown can make a
> reconnect lookup miss a live matching superblock.
> 
> Backport note: before dc3216b14160 ("super: ensure valid info"), the
> s_instances unlink is in generic_shutdown_super(). Before 3ec9800c2d33
> ("super: convert s_count to refcount_t s_passive"), the passive reference is
> named s_count.
> 
>  fs/super.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 05e4431730387..c8accd144bad2 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1026,11 +1026,18 @@ void iterate_supers_type(struct file_system_type *type,
>  	struct super_block *sb, *p = NULL;
>  
>  	spin_lock(&sb_lock);
> -	hlist_for_each_entry(sb, &type->fs_supers, s_instances) {
> +	/*
> +	 * The passive reference keeps the s_list cursor valid while sb_lock
> +	 * is dropped. Entries are added at the tail. Walk backwards to retain
> +	 * newest-first visitation.
> +	 */
> +	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
>  		bool locked;
>  
>  		if (super_flags(sb, SB_DYING))
>  			continue;
> +		if (sb->s_type != type)
> +			continue;
>  
>  		if (!refcount_inc_not_zero(&sb->s_passive))
>  			continue;
> -- 
> 2.53.0
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] super: make iterate_supers_type() deletion-safe

[Christian Brauner]

On Thu, Sep 03, 2026 at 12:16:14PM +0200, Jan Kara wrote:
> On Thu 03-09-26 03:33:36, Karl Mehltretter wrote:
> > iterate_supers_type() drops sb_lock while invoking the callback and keeps
> > only a passive reference to the current superblock. That reference keeps
> > the object allocated, but does not keep its s_instances node linked.
> > 
> > After the callback releases s_umount, final teardown can unlink the current
> > s_instances node. The iterator then advances through a reinitialized node.
> > With the current hlist it stops without visiting the remaining superblocks.
> > The unlink moved from generic_shutdown_super() to kill_super_notify(), but
> > the cursor lifetime has been unsafe since the helper was introduced.
> > 
> > The CIFS DFS lookup can consequently miss a matching superblock and return
> > -EINVAL.
> > 
> > Walk the global superblock list in reverse and filter it by filesystem
> > type. A passive reference keeps its s_list node linked, and reverse
> > traversal preserves newest-first visitation. Superblocks removed from
> > fs_supers remain on the global list, but teardown marks them SB_DYING
> > before unlinking them, so the existing filter excludes them.
> > 
> > This broadens the scan from superblocks of one type to all superblocks.
> > The only in-tree caller is the CIFS DFS lookup, so the broader scan is
> > limited to that path.
> > 
> > Fixes: 43e15cdbefea ("new helper: iterate_supers_type()")
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> 
> Good spotting! But what I'm wondering about is whether we just shouldn't
> move where we delete sb from fs_supers. Currently we do that in
> kill_super_notify() to hide the sb from sget_fc() (which would otherwise
> permanently retry and call test() for S_DEAD sb which can cause problems).
> But if we just skipped S_DEAD superblocks in sget_fc(), we could move
> removal from fs_supers list to put_super() (make it symmetric with the
> handling of super_blocks list) and that would also fix
> iterate_supers_type(). Christian, what do you think?

Yes, that might work work.

Re: [PATCH] super: make iterate_supers_type() deletion-safe

[Christian Brauner]

On Fri, Sep 04, 2026 at 12:29:39PM +0200, Christian Brauner wrote:
> On Thu, Sep 03, 2026 at 12:16:14PM +0200, Jan Kara wrote:
> > On Thu 03-09-26 03:33:36, Karl Mehltretter wrote:
> > > iterate_supers_type() drops sb_lock while invoking the callback and keeps
> > > only a passive reference to the current superblock. That reference keeps
> > > the object allocated, but does not keep its s_instances node linked.
> > > 
> > > After the callback releases s_umount, final teardown can unlink the current
> > > s_instances node. The iterator then advances through a reinitialized node.
> > > With the current hlist it stops without visiting the remaining superblocks.
> > > The unlink moved from generic_shutdown_super() to kill_super_notify(), but
> > > the cursor lifetime has been unsafe since the helper was introduced.
> > > 
> > > The CIFS DFS lookup can consequently miss a matching superblock and return
> > > -EINVAL.
> > > 
> > > Walk the global superblock list in reverse and filter it by filesystem
> > > type. A passive reference keeps its s_list node linked, and reverse
> > > traversal preserves newest-first visitation. Superblocks removed from
> > > fs_supers remain on the global list, but teardown marks them SB_DYING
> > > before unlinking them, so the existing filter excludes them.
> > > 
> > > This broadens the scan from superblocks of one type to all superblocks.
> > > The only in-tree caller is the CIFS DFS lookup, so the broader scan is
> > > limited to that path.
> > > 
> > > Fixes: 43e15cdbefea ("new helper: iterate_supers_type()")
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: LLM
> > > Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> > 
> > Good spotting! But what I'm wondering about is whether we just shouldn't
> > move where we delete sb from fs_supers. Currently we do that in
> > kill_super_notify() to hide the sb from sget_fc() (which would otherwise
> > permanently retry and call test() for S_DEAD sb which can cause problems).
> > But if we just skipped S_DEAD superblocks in sget_fc(), we could move
> > removal from fs_supers list to put_super() (make it symmetric with the
> > handling of super_blocks list) and that would also fix
> > iterate_supers_type(). Christian, what do you think?
> 
> Yes, that might work work.

Draft, feel free to grab, Jan or Karl.

---
 fs/kernfs/mount.c |  4 ++--
 fs/super.c        | 39 +++++++++++++++++++--------------------
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index f183a96778b9..a57399021c8b 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -434,8 +434,8 @@ void kernfs_kill_sb(struct super_block *sb)
 	up_write(&root->kernfs_supers_rwsem);
 
 	/*
-	 * Remove the superblock from fs_supers/s_instances
-	 * so we can't find it, before freeing kernfs_super_info.
+	 * Mark the superblock dead so sget_fc() can't find it,
+	 * before freeing kernfs_super_info.
 	 */
 	kill_anon_super(sb);
 	kfree(info);
diff --git a/fs/super.c b/fs/super.c
index 05e443173038..0f9e13eedb4f 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -433,15 +433,19 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
 void put_super(struct super_block *s)
 {
 	if (refcount_dec_and_test(&s->s_passive)) {
+		struct file_system_type *type = s->s_type;
 
 		spin_lock(&sb_lock);
 		list_del_init(&s->s_list);
+		hlist_del_init(&s->s_instances);
 		spin_unlock(&sb_lock);
 
 		WARN_ON(s->s_dentry_lru.node);
 		WARN_ON(s->s_inode_lru.node);
 		WARN_ON(s->s_mounts);
 		call_rcu(&s->rcu, destroy_super_rcu);
+		/* The unlink above may touch type->fs_supers, so drop it last. */
+		put_filesystem(type);
 	}
 }
 
@@ -558,17 +562,6 @@ static void kill_super_notify(struct super_block *sb)
 	if (sb->s_flags & SB_DEAD)
 		return;
 
-	/*
-	 * Remove it from @fs_supers so it isn't found by new
-	 * sget_fc() walkers anymore. Any concurrent mounter still
-	 * managing to grab a temporary reference is guaranteed to
-	 * already see SB_DYING and will wait until we notify them about
-	 * SB_DEAD.
-	 */
-	spin_lock(&sb_lock);
-	hlist_del_init(&sb->s_instances);
-	spin_unlock(&sb_lock);
-
 	/* Drop sget_fc()'s claim; a never-registered entry stays with the sb. */
 	if (sb->s_super_dev->sd_dev) {
 		super_dev_put(sb->s_super_dev);
@@ -577,11 +570,15 @@ static void kill_super_notify(struct super_block *sb)
 
 	/*
 	 * Let concurrent mounts know that this thing is really dead.
-	 * We don't need @sb->s_umount here as every concurrent caller
-	 * will see SB_DYING and either discard the superblock or wait
-	 * for SB_DEAD.
+	 * sget_fc() skips SB_DEAD superblocks and calls test() under
+	 * sb_lock, so set it under sb_lock: once we return no test()
+	 * runs on this superblock anymore and none will start. Everyone
+	 * else already saw SB_DYING and either discarded the superblock
+	 * or waits for SB_DEAD.
 	 */
+	spin_lock(&sb_lock);
 	super_wake(sb, SB_DEAD);
+	spin_unlock(&sb_lock);
 }
 
 /**
@@ -608,7 +605,6 @@ void deactivate_locked_super(struct super_block *s)
 		list_lru_destroy(&s->s_dentry_lru);
 		list_lru_destroy(&s->s_inode_lru);
 
-		put_filesystem(fs);
 		put_super(s);
 	} else {
 		super_unlock_excl(s);
@@ -795,12 +791,12 @@ void generic_shutdown_super(struct super_block *sb)
 	}
 	/*
 	 * Broadcast to everyone that grabbed a temporary reference to this
-	 * superblock before we removed it from @fs_supers that the superblock
-	 * is dying. Every walker of @fs_supers outside of sget_fc() will now
-	 * discard this superblock and treat it as dead.
+	 * superblock that it is dying. Every walker of @fs_supers outside
+	 * of sget_fc() will now discard this superblock and treat it as
+	 * dead.
 	 *
-	 * We leave the superblock on @fs_supers so it can be found by
-	 * sget_fc() until we passed sb->kill_sb().
+	 * sget_fc() keeps finding the superblock until SB_DEAD is set, so
+	 * a concurrent mounter waits until we passed sb->kill_sb().
 	 */
 	super_wake(sb, SB_DYING);
 	super_unlock_excl(sb);
@@ -879,6 +875,9 @@ struct super_block *sget_fc(struct fs_context *fc,
 	spin_lock(&sb_lock);
 	if (test) {
 		hlist_for_each_entry(old, &fc->fs_type->fs_supers, s_instances) {
+			/* Only unlinked at the last passive reference. */
+			if (super_flags(old, SB_DEAD))
+				continue;
 			if (test(old, fc))
 				goto share_extant_sb;
 		}
-- 
2.53.0

Re: [PATCH] super: make iterate_supers_type() deletion-safe

[Karl Mehltretter]

On Fri, Sep 04, 2026 at 01:11:53PM +0100, Christian Brauner wrote:
> 
> Draft, feel free to grab, Jan or Karl.
> 
> ---
>  fs/kernfs/mount.c |  4 ++--
>  fs/super.c        | 39 +++++++++++++++++++--------------------
>  2 files changed, 21 insertions(+), 22 deletions(-)
> 

Thanks, I found no issues with your draft.
All the tests I ran on my patch also pass with your draft.

Below is my proposed changelog. If it looks good, I'll send v2 with you as
author. Could you provide your Signed-off-by?

super: make iterate_supers_type() deletion-safe

iterate_supers_type() drops sb_lock while invoking the callback and keeps
only a passive reference to the current superblock. That reference keeps
the object allocated, but does not keep its s_instances node linked.

After the iterator releases s_umount, final teardown can unlink the current
s_instances node. The iterator then advances through a reinitialized node.
With the current hlist it stops without visiting the remaining superblocks.
The unlink moved from generic_shutdown_super() to kill_super_notify(), but
the cursor lifetime has been unsafe since the helper was introduced.

The CIFS DFS lookup can consequently miss a matching superblock and return
-EINVAL.

Move removal from fs_supers to put_super(), alongside removal from
super_blocks, so a passive reference keeps both list nodes linked. Keep
the filesystem module reference until then, since unlinking s_instances
may touch type->fs_supers.

Make sget_fc() skip SB_DEAD superblocks before invoking test(), and set
SB_DEAD under sb_lock to serialize with those callbacks. This allows
kernfs to free its private information after kill_anon_super() returns.
Keep matching SB_DYING superblocks until SB_DEAD is set so concurrent
mounts still wait for teardown before retrying.

Fixes: 43e15cdbefea ("new helper: iterate_supers_type()")
Reported-by: Karl Mehltretter <kmehltretter@gmail.com>
Suggested-by: Jan Kara <jack@suse.cz>
Cc: stable@vger.kernel.org
<your Signed-off-by>
Tested-by: Karl Mehltretter <kmehltretter@gmail.com>
Assisted-by: LLM
<my Signed-off-by>

Thanks,
Karl