[PATCH 0/8] fs: fix a range of UAFs during rcu pathwalk caused by non-rcu frees

[PATCH 0/8] fs: fix a range of UAFs during rcu pathwalk caused by non-rcu frees

[Christian Brauner]

While working on eliding the grace period in namespace_unlock() I went
through what filesystems free when they're shut down and what a pathwalk
in rcu mode can still read at that point. Since 66a97c2ec953's rcu
pathwalk fixes that's the filesystem's job. Whatever ->permission(),
->d_revalidate(), ->get_link(), ->d_hash() and ->d_compare() look at in
rcu mode has to be freed after a grace period. A few filesystems don't
do that.

Most of it is hidden by umount(2). namespace_unlock() waits for a grace
period before it drops the mounts, so a walker that was inside the
filesystem is gone by the time the filesystem is shut down.

But the last reference to a mount doesn't have to go away in umount(2).
A chdir() out of a lazily unmounted directory or the close() of the last
file on it drops it without a grace period. A walker can then still be
in the filesystem while ->put_super() and ->kill_sb() free what it
reads.

With KASAN on vfs.fixes:

- ext4_get_link() reads sb->s_fs_info and the extent status counters in
  rcu mode for a symlink that keeps its target in a data block.

  ext4_put_super() destroys the counters, clears s_fs_info and frees the
  sbi right away. That's a NULL dereference, a use-after-free of the sbi
  or a fault on a destroyed percpu counter, depending on where the
  walker sits.

- The unicode map of casefolded ext4, f2fs and tmpfs is freed from
  ->put_super() while generic_ci_d_hash() and generic_ci_d_compare() use
  it.

- ntfs3 with nocase frees its sbi and mount options under ntfs_d_hash()
  and ntfs_d_compare(). A remount frees the old mount options right away
  too, that one needs no unmount at all.

- eventfs frees an eventfs_inode after an SRCU grace period only, but
  tracefs_d_revalidate() reads it under rcu_read_lock(). No unmount
  needed either.

The ipc patch is a different kind of bug. create_ipc_ns() leaks the
mqueue mount and its superblock when it fails after mq_init_ns(). I ran
into it while looking at the kern_unmount() users.

afs and 9p look just as broken but nothing can reach it today. Both
have a private bdi, and generic_shutdown_super() calls bdi_unregister(),
which waits for a grace period before ->kill_sb() frees their private
data. That grace period is there for the rcu readers of bdi_list and
covers afs and 9p by accident. I don't want them to depend on that, so
they free their private data after a grace period of their own as well.

I appended the reproducers. The race reproducers hold a pathwalk inside
the filesystem's callback for two seconds with a kprobe while the other
side frees what it reads. For the ntfs3 remount the window is a single
load and its dereference, so that one needs an mdelay() in the kernel.
ext4 with a journal needs a kernel without lockdep since
jbd2_journal_destroy() calls lockdep_unregister_key(), which waits for a
grace period too.

Clean this all up.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (8):
      ext4: free the sbi after a grace period
      afs: free the super info after a grace period
      9p: free the session after a grace period
      eventfs: free the eventfs_inode after an rcu grace period too
      ipc: drop the mqueue mount with kern_unmount() on setup failure
      unicode: free the map after a grace period
      ntfs3: free the sbi after a grace period
      ntfs3: free replaced mount options after a grace period

 fs/9p/v9fs.h             |  1 +
 fs/9p/vfs_super.c        |  4 ++--
 fs/afs/internal.h        |  2 ++
 fs/afs/super.c           |  2 +-
 fs/ext4/ext4.h           |  2 ++
 fs/ext4/extents-test.c   |  2 ++
 fs/ext4/extents.c        |  1 +
 fs/ext4/extents_status.c |  7 ++++++-
 fs/ext4/extents_status.h |  1 +
 fs/ext4/super.c          | 13 +++++++++++--
 fs/ntfs3/dir.c           | 13 +++++++++++--
 fs/ntfs3/ntfs_fs.h       |  3 +++
 fs/ntfs3/super.c         | 49 +++++++++++++++++++++++++++++++++++-------------
 fs/tracefs/event_inode.c | 10 +++++++++-
 fs/unicode/utf8-core.c   | 21 +++++++++++++++++----
 include/linux/unicode.h  |  1 +
 ipc/namespace.c          |  2 +-
 17 files changed, 107 insertions(+), 27 deletions(-)
---
base-commit: 3d399224425573875b6f6f1181bd8cf9a28cc7d1
change-id: 20261009-work-fixes-rcu-cd9c1eec0769

[PATCH 1/8] ext4: free the sbi after a grace period

[Christian Brauner]

ext4_get_link() follows a symlink that keeps its target in a data block
in rcu pathwalk mode with ext4_getblk(EXT4_GET_BLOCKS_CACHED_NOWAIT).
That reads s_mount_state in the ASSERT() of ext4_getblk() and again in
ext4_es_lookup_extent(), and the lookup bumps the extent status hit or
miss counter, all through sb->s_fs_info.

ext4_put_super() destroys those counters, clears sb->s_fs_info and frees
the sbi without waiting for a grace period. A pathwalk that is still in
ext4_get_link() when the last reference to the mount goes away then
dereferences a NULL s_fs_info, the freed sbi or the destroyed percpu
counters.

With a pathwalk held up in ext4_get_link() while the last reference
goes away:

  KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
  RIP: 0010:ext4_getblk+0xf0/0x640
  Call Trace:
   <TASK>
   ext4_get_link+0x26c/0x410
   pick_link+0xf96/0x1780
   step_into_slowpath+0x9b8/0x1250
   path_lookupat+0x484/0xb00
   filename_lookup+0x1f1/0x5b0
   vfs_statx+0xd7/0x3b0
   vfs_fstatat+0x4d/0x90
   __do_sys_newfstatat+0xa0/0x100
   do_syscall_64+0x10e/0x5e0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

So far that has been hard to hit. umount(2) waits for a grace period in
namespace_unlock() before it drops the mount, so only a mount whose last
reference goes away elsewhere, like the close of a file or a chdir out of
a lazily unmounted directory, can race a walker.

Keep sb->s_fs_info and free the sbi together with the extent status
counters after a grace period. Split the counters out of
ext4_es_unregister_shrinker() for that. The shrinker still gets wasted
in ext4_put_super(), the counters go with the sbi.

Fixes: 6493792d3299 ("ext4: convert symlink external data block mapping to bdev")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Reproducer: delay.c holds a task named walker in ext4_get_link() in rcu
pathwalk mode for two seconds and repro.c drops the last reference to a
lazily unmounted ext4 meanwhile. Use an ext4 without a journal or a
kernel without lockdep: jbd2_journal_destroy() calls
lockdep_unregister_key(), which waits for a grace period on lockdep
kernels and hides this for a journaled ext4.

    mkfs.ext4 -O ^has_journal /dev/vdb
    mount -o noatime /dev/vdb /mnt
    ln -s /$(printf 'x%.0s' $(seq 100)) /mnt/link
    insmod delay.ko
    ./repro

The symlink target is too long for the inode, so ext4_get_link() reads it
from its block. Without this patch that gives the splat above, with it
it runs clean.

delay.c:

    #include <linux/delay.h>
    #include <linux/kprobes.h>
    #include <linux/module.h>

    /* keep a task named "walker" in ext4_get_link() in rcu pathwalk */
    static int pre(struct kprobe *p, struct pt_regs *regs)
    {
    	if (!regs_get_kernel_argument(regs, 0) && !strcmp(current->comm, "walker"))
    		mdelay(2000);
    	return 0;
    }

    static struct kprobe kp = {
    	.symbol_name	= "ext4_get_link",
    	.pre_handler	= pre,
    };

    static int __init delay_init(void)
    {
    	return register_kprobe(&kp);
    }

    static void __exit delay_exit(void)
    {
    	unregister_kprobe(&kp);
    }

    module_init(delay_init);
    module_exit(delay_exit);
    MODULE_LICENSE("GPL");

repro.c:

    #define _GNU_SOURCE
    #include <pthread.h>
    #include <sched.h>
    #include <string.h>
    #include <sys/mman.h>
    #include <sys/mount.h>
    #include <sys/prctl.h>
    #include <sys/stat.h>
    #include <unistd.h>

    static void pin(int cpu)
    {
    	cpu_set_t set;

    	CPU_ZERO(&set);
    	CPU_SET(cpu, &set);
    	sched_setaffinity(0, sizeof(set), &set);
    }

    static void *walker(void *arg)
    {
    	struct stat st;
    	char *p;

    	/*
    	 * The delay spins with preemption disabled. Empty this cpu's lru
    	 * batches so that lru_add_drain_all() from invalidate_bdev() has no
    	 * work to wait for here.
    	 */
    	pin(1);
    	p = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    	memset(p, 1, 4096);
    	madvise(p, 4096, MADV_COLD);

    	prctl(PR_SET_NAME, "walker");
    	stat("link", &st);
    	return NULL;
    }

    int main(void)
    {
    	struct stat st;
    	pthread_t t;

    	pin(0);
    	chdir("/mnt");
    	stat("link", &st);
    	/* the cwd now holds the last reference to the mount */
    	umount2("/mnt", MNT_DETACH);
    	pthread_create(&t, NULL, walker, NULL);
    	sleep(1);
    	/* drop it, ->kill_sb() runs before chdir() returns */
    	chdir("/");
    	pthread_join(t, NULL);
    	return 0;
    }
---
 fs/ext4/ext4.h           |  2 ++
 fs/ext4/extents-test.c   |  2 ++
 fs/ext4/extents.c        |  1 +
 fs/ext4/extents_status.c |  7 ++++++-
 fs/ext4/extents_status.h |  1 +
 fs/ext4/super.c          | 13 +++++++++++--
 6 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 724a27e8be61..af70bb26bb54 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1774,6 +1774,8 @@ struct ext4_sb_info {
 	struct list_head s_es_list;	/* List of inodes with reclaimable extents */
 	long s_es_nr_inode;
 	struct ext4_es_stats s_es_stats;
+	/* ext4_get_link() reads the sbi in rcu pathwalk */
+	struct rcu_head s_rcu;
 	struct mb_cache *s_ea_block_cache;
 	struct mb_cache *s_ea_inode_cache;
 
diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index c3836ecb89f9..2e997f228330 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -151,6 +151,7 @@ static void extents_kunit_exit(struct kunit *test)
 
 	sbi = k_ctx.k_ei->vfs_inode.i_sb->s_fs_info;
 	ext4_es_unregister_shrinker(sbi);
+	ext4_es_destroy_stats(sbi);
 	deactivate_super(sbi->s_sb);
 	kfree(sbi);
 	kfree(k_ctx.k_ei);
@@ -340,6 +341,7 @@ static int extents_kunit_init(struct kunit *test)
 	k_ctx.k_data = NULL;
 
 	ext4_es_unregister_shrinker(sbi);
+	ext4_es_destroy_stats(sbi);
 out_deactivate:
 	deactivate_locked_super(sb);
 	kfree(sbi);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 76038b6c3655..3bf0a6cdbec2 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -6409,6 +6409,7 @@ EXPORT_SYMBOL_FOR_EXT4_TEST(__ext4_ext_dirty);
 EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_ext_zeroout);
 EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_register_shrinker);
 EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_unregister_shrinker);
+EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_destroy_stats);
 EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_map_create_blocks);
 EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_init_tree);
 EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_lookup_extent);
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 6e4a191e8219..018b6146b9d8 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -1881,12 +1881,17 @@ int ext4_es_register_shrinker(struct ext4_sb_info *sbi)
 }
 
 void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi)
+{
+	shrinker_free(sbi->s_es_shrinker);
+}
+
+/* ext4_get_link() bumps the hit and miss counters in rcu pathwalk */
+void ext4_es_destroy_stats(struct ext4_sb_info *sbi)
 {
 	percpu_counter_destroy(&sbi->s_es_stats.es_stats_cache_hits);
 	percpu_counter_destroy(&sbi->s_es_stats.es_stats_cache_misses);
 	percpu_counter_destroy(&sbi->s_es_stats.es_stats_all_cnt);
 	percpu_counter_destroy(&sbi->s_es_stats.es_stats_shk_cnt);
-	shrinker_free(sbi->s_es_shrinker);
 }
 
 /*
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index f3396cf32b44..01cddcaaa68e 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -238,6 +238,7 @@ static inline void ext4_es_store_pblock_status(struct extent_status *es,
 
 extern int ext4_es_register_shrinker(struct ext4_sb_info *sbi);
 extern void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi);
+extern void ext4_es_destroy_stats(struct ext4_sb_info *sbi);
 
 extern int ext4_seq_es_shrinker_info_show(struct seq_file *seq, void *v);
 
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index bca0dc87d0b7..f028c5835c5d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1279,6 +1279,14 @@ static void ext4_flex_groups_free(struct ext4_sb_info *sbi)
 	}
 }
 
+static void ext4_put_sbi_rcu(struct rcu_head *head)
+{
+	struct ext4_sb_info *sbi = container_of(head, struct ext4_sb_info, s_rcu);
+
+	ext4_es_destroy_stats(sbi);
+	kfree(sbi);
+}
+
 static void ext4_put_super(struct super_block *sb)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -1374,7 +1382,6 @@ static void ext4_put_super(struct super_block *sb)
 	ext4_stop_mmpd(sbi);
 
 	brelse(sbi->s_sbh);
-	sb->s_fs_info = NULL;
 	/*
 	 * Now that we are completely done shutting down the
 	 * superblock, we need to actually destroy the kobject.
@@ -1387,7 +1394,8 @@ static void ext4_put_super(struct super_block *sb)
 #if IS_ENABLED(CONFIG_UNICODE)
 	utf8_unload(sb->s_encoding);
 #endif
-	kfree(sbi);
+	/* ext4_get_link() reads the sbi and the counters in rcu pathwalk */
+	call_rcu(&sbi->s_rcu, ext4_put_sbi_rcu);
 }
 
 static struct kmem_cache *ext4_inode_cachep;
@@ -5805,6 +5813,7 @@ failed_mount8: __maybe_unused
 	/* Drain deferred EA inode iputs from journal replay */
 	flush_delayed_work(&sbi->s_ea_inode_work);
 	ext4_es_unregister_shrinker(sbi);
+	ext4_es_destroy_stats(sbi);
 failed_mount3:
 	/* flush s_sb_upd_work before sbi destroy */
 	flush_work(&sbi->s_sb_upd_work);

-- 
2.53.0

Re: [PATCH 1/8] ext4: free the sbi after a grace period

[sashiko-bot]

> ext4_get_link() follows a symlink that keeps its target in a data block
> in rcu pathwalk mode with ext4_getblk(EXT4_GET_BLOCKS_CACHED_NOWAIT).
> That reads s_mount_state in the ASSERT() of ext4_getblk() and again in
> ext4_es_lookup_extent(), and the lookup bumps the extent status hit or
> miss counter, all through sb->s_fs_info.
> 
> ext4_put_super() destroys those counters, clears sb->s_fs_info and frees
> the sbi without waiting for a grace period. A pathwalk that is still in
> ext4_get_link() when the last reference to the mount goes away then
> dereferences a NULL s_fs_info, the freed sbi or the destroyed percpu
> counters.
> 
> With a pathwalk held up in ext4_get_link() while the last reference
> goes away:
> 
> [ ... ]
> in ext4_put_super(), the counters go with the sbi.
> 
> Fixes: 6493792d3299 ("ext4: convert symlink external data block mapping to bdev")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=1

Re: [PATCH 1/8] ext4: free the sbi after a grace period

[Jan Kara]

On Fri 09-10-26 23:51:51, Christian Brauner wrote:
> ext4_get_link() follows a symlink that keeps its target in a data block
> in rcu pathwalk mode with ext4_getblk(EXT4_GET_BLOCKS_CACHED_NOWAIT).
> That reads s_mount_state in the ASSERT() of ext4_getblk() and again in
> ext4_es_lookup_extent(), and the lookup bumps the extent status hit or
> miss counter, all through sb->s_fs_info.
> 
> ext4_put_super() destroys those counters, clears sb->s_fs_info and frees
> the sbi without waiting for a grace period. A pathwalk that is still in
> ext4_get_link() when the last reference to the mount goes away then
> dereferences a NULL s_fs_info, the freed sbi or the destroyed percpu
> counters.
> 
> With a pathwalk held up in ext4_get_link() while the last reference
> goes away:
> 
>   KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
>   RIP: 0010:ext4_getblk+0xf0/0x640
>   Call Trace:
>    <TASK>
>    ext4_get_link+0x26c/0x410
>    pick_link+0xf96/0x1780
>    step_into_slowpath+0x9b8/0x1250
>    path_lookupat+0x484/0xb00
>    filename_lookup+0x1f1/0x5b0
>    vfs_statx+0xd7/0x3b0
>    vfs_fstatat+0x4d/0x90
>    __do_sys_newfstatat+0xa0/0x100
>    do_syscall_64+0x10e/0x5e0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> So far that has been hard to hit. umount(2) waits for a grace period in
> namespace_unlock() before it drops the mount, so only a mount whose last
> reference goes away elsewhere, like the close of a file or a chdir out of
> a lazily unmounted directory, can race a walker.
> 
> Keep sb->s_fs_info and free the sbi together with the extent status
> counters after a grace period. Split the counters out of
> ext4_es_unregister_shrinker() for that. The shrinker still gets wasted
> in ext4_put_super(), the counters go with the sbi.
> 
> Fixes: 6493792d3299 ("ext4: convert symlink external data block mapping to bdev")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Reproducer: delay.c holds a task named walker in ext4_get_link() in rcu
> pathwalk mode for two seconds and repro.c drops the last reference to a
> lazily unmounted ext4 meanwhile. Use an ext4 without a journal or a
> kernel without lockdep: jbd2_journal_destroy() calls
> lockdep_unregister_key(), which waits for a grace period on lockdep
> kernels and hides this for a journaled ext4.
> 
>     mkfs.ext4 -O ^has_journal /dev/vdb
>     mount -o noatime /dev/vdb /mnt
>     ln -s /$(printf 'x%.0s' $(seq 100)) /mnt/link
>     insmod delay.ko
>     ./repro
> 
> The symlink target is too long for the inode, so ext4_get_link() reads it
> from its block. Without this patch that gives the splat above, with it
> it runs clean.
> 
> delay.c:
> 
>     #include <linux/delay.h>
>     #include <linux/kprobes.h>
>     #include <linux/module.h>
> 
>     /* keep a task named "walker" in ext4_get_link() in rcu pathwalk */
>     static int pre(struct kprobe *p, struct pt_regs *regs)
>     {
>     	if (!regs_get_kernel_argument(regs, 0) && !strcmp(current->comm, "walker"))
>     		mdelay(2000);
>     	return 0;
>     }
> 
>     static struct kprobe kp = {
>     	.symbol_name	= "ext4_get_link",
>     	.pre_handler	= pre,
>     };
> 
>     static int __init delay_init(void)
>     {
>     	return register_kprobe(&kp);
>     }
> 
>     static void __exit delay_exit(void)
>     {
>     	unregister_kprobe(&kp);
>     }
> 
>     module_init(delay_init);
>     module_exit(delay_exit);
>     MODULE_LICENSE("GPL");
> 
> repro.c:
> 
>     #define _GNU_SOURCE
>     #include <pthread.h>
>     #include <sched.h>
>     #include <string.h>
>     #include <sys/mman.h>
>     #include <sys/mount.h>
>     #include <sys/prctl.h>
>     #include <sys/stat.h>
>     #include <unistd.h>
> 
>     static void pin(int cpu)
>     {
>     	cpu_set_t set;
> 
>     	CPU_ZERO(&set);
>     	CPU_SET(cpu, &set);
>     	sched_setaffinity(0, sizeof(set), &set);
>     }
> 
>     static void *walker(void *arg)
>     {
>     	struct stat st;
>     	char *p;
> 
>     	/*
>     	 * The delay spins with preemption disabled. Empty this cpu's lru
>     	 * batches so that lru_add_drain_all() from invalidate_bdev() has no
>     	 * work to wait for here.
>     	 */
>     	pin(1);
>     	p = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>     	memset(p, 1, 4096);
>     	madvise(p, 4096, MADV_COLD);
> 
>     	prctl(PR_SET_NAME, "walker");
>     	stat("link", &st);
>     	return NULL;
>     }
> 
>     int main(void)
>     {
>     	struct stat st;
>     	pthread_t t;
> 
>     	pin(0);
>     	chdir("/mnt");
>     	stat("link", &st);
>     	/* the cwd now holds the last reference to the mount */
>     	umount2("/mnt", MNT_DETACH);
>     	pthread_create(&t, NULL, walker, NULL);
>     	sleep(1);
>     	/* drop it, ->kill_sb() runs before chdir() returns */
>     	chdir("/");
>     	pthread_join(t, NULL);
>     	return 0;
>     }
> ---
>  fs/ext4/ext4.h           |  2 ++
>  fs/ext4/extents-test.c   |  2 ++
>  fs/ext4/extents.c        |  1 +
>  fs/ext4/extents_status.c |  7 ++++++-
>  fs/ext4/extents_status.h |  1 +
>  fs/ext4/super.c          | 13 +++++++++++--
>  6 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 724a27e8be61..af70bb26bb54 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1774,6 +1774,8 @@ struct ext4_sb_info {
>  	struct list_head s_es_list;	/* List of inodes with reclaimable extents */
>  	long s_es_nr_inode;
>  	struct ext4_es_stats s_es_stats;
> +	/* ext4_get_link() reads the sbi in rcu pathwalk */
> +	struct rcu_head s_rcu;
>  	struct mb_cache *s_ea_block_cache;
>  	struct mb_cache *s_ea_inode_cache;
>  
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index c3836ecb89f9..2e997f228330 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -151,6 +151,7 @@ static void extents_kunit_exit(struct kunit *test)
>  
>  	sbi = k_ctx.k_ei->vfs_inode.i_sb->s_fs_info;
>  	ext4_es_unregister_shrinker(sbi);
> +	ext4_es_destroy_stats(sbi);
>  	deactivate_super(sbi->s_sb);
>  	kfree(sbi);
>  	kfree(k_ctx.k_ei);
> @@ -340,6 +341,7 @@ static int extents_kunit_init(struct kunit *test)
>  	k_ctx.k_data = NULL;
>  
>  	ext4_es_unregister_shrinker(sbi);
> +	ext4_es_destroy_stats(sbi);
>  out_deactivate:
>  	deactivate_locked_super(sb);
>  	kfree(sbi);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 76038b6c3655..3bf0a6cdbec2 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -6409,6 +6409,7 @@ EXPORT_SYMBOL_FOR_EXT4_TEST(__ext4_ext_dirty);
>  EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_ext_zeroout);
>  EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_register_shrinker);
>  EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_unregister_shrinker);
> +EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_destroy_stats);
>  EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_map_create_blocks);
>  EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_init_tree);
>  EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_es_lookup_extent);
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 6e4a191e8219..018b6146b9d8 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1881,12 +1881,17 @@ int ext4_es_register_shrinker(struct ext4_sb_info *sbi)
>  }
>  
>  void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi)
> +{
> +	shrinker_free(sbi->s_es_shrinker);
> +}
> +
> +/* ext4_get_link() bumps the hit and miss counters in rcu pathwalk */
> +void ext4_es_destroy_stats(struct ext4_sb_info *sbi)
>  {
>  	percpu_counter_destroy(&sbi->s_es_stats.es_stats_cache_hits);
>  	percpu_counter_destroy(&sbi->s_es_stats.es_stats_cache_misses);
>  	percpu_counter_destroy(&sbi->s_es_stats.es_stats_all_cnt);
>  	percpu_counter_destroy(&sbi->s_es_stats.es_stats_shk_cnt);
> -	shrinker_free(sbi->s_es_shrinker);
>  }
>  
>  /*
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index f3396cf32b44..01cddcaaa68e 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -238,6 +238,7 @@ static inline void ext4_es_store_pblock_status(struct extent_status *es,
>  
>  extern int ext4_es_register_shrinker(struct ext4_sb_info *sbi);
>  extern void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi);
> +extern void ext4_es_destroy_stats(struct ext4_sb_info *sbi);
>  
>  extern int ext4_seq_es_shrinker_info_show(struct seq_file *seq, void *v);
>  
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bca0dc87d0b7..f028c5835c5d 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1279,6 +1279,14 @@ static void ext4_flex_groups_free(struct ext4_sb_info *sbi)
>  	}
>  }
>  
> +static void ext4_put_sbi_rcu(struct rcu_head *head)
> +{
> +	struct ext4_sb_info *sbi = container_of(head, struct ext4_sb_info, s_rcu);
> +
> +	ext4_es_destroy_stats(sbi);
> +	kfree(sbi);
> +}
> +
>  static void ext4_put_super(struct super_block *sb)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
> @@ -1374,7 +1382,6 @@ static void ext4_put_super(struct super_block *sb)
>  	ext4_stop_mmpd(sbi);
>  
>  	brelse(sbi->s_sbh);
> -	sb->s_fs_info = NULL;
>  	/*
>  	 * Now that we are completely done shutting down the
>  	 * superblock, we need to actually destroy the kobject.
> @@ -1387,7 +1394,8 @@ static void ext4_put_super(struct super_block *sb)
>  #if IS_ENABLED(CONFIG_UNICODE)
>  	utf8_unload(sb->s_encoding);
>  #endif
> -	kfree(sbi);
> +	/* ext4_get_link() reads the sbi and the counters in rcu pathwalk */
> +	call_rcu(&sbi->s_rcu, ext4_put_sbi_rcu);
>  }
>  
>  static struct kmem_cache *ext4_inode_cachep;
> @@ -5805,6 +5813,7 @@ failed_mount8: __maybe_unused
>  	/* Drain deferred EA inode iputs from journal replay */
>  	flush_delayed_work(&sbi->s_ea_inode_work);
>  	ext4_es_unregister_shrinker(sbi);
> +	ext4_es_destroy_stats(sbi);
>  failed_mount3:
>  	/* flush s_sb_upd_work before sbi destroy */
>  	flush_work(&sbi->s_sb_upd_work);
> 
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 2/8] afs: free the super info after a grace period

[Christian Brauner]

afs_atcell_get_link() resolves the @cell and .@cell symlinks of a
dynamic root in rcu pathwalk mode. Before it looks at the dentry it gets
the afs_net through afs_i2net(), which reads ->net_ns from the
afs_super_info in sb->s_fs_info. The @cell dentries are kept around when
unused and the dynamic root has no ->d_revalidate(), so a lookup that
finds @cell in the dcache gets to ->get_link() in rcu mode.

afs_kill_super() frees the afs_super_info with a plain kfree() in
afs_destroy_sbi(). Right now a pathwalk can't still be in
afs_atcell_get_link() at that point, but only by accident. umount(2)
waits for a grace period in namespace_unlock() before it drops the
mount. And a mount whose last reference goes away elsewhere, like the
chdir out of a lazily unmounted directory, still gets a grace period
from bdi_unregister(). afs_fill_super() sets up a private bdi, so
generic_shutdown_super() unregisters it from kill_anon_super() and
bdi_remove_from_list() waits in synchronize_rcu_expedited() before
afs_destroy_sbi() runs. That grace period protects the rcu readers of
bdi_list in the writeback code. It has nothing to do with pathwalk.

Don't rely on it and free the afs_super_info after a grace period.

Fixes: 30bca65bbbae ("afs: Make /afs/@cell and /afs/.@cell symlinks")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/afs/internal.h | 2 ++
 fs/afs/super.c    | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 330654ed16ec..3282d5d6e1c0 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -251,6 +251,8 @@ struct afs_super_info {
 	struct afs_volume	*volume;	/* volume record */
 	enum afs_flock_mode	flock_mode:8;	/* File locking emulation mode */
 	bool			dyn_root;	/* True if dynamic root */
+	/* afs_atcell_get_link() reads ->net_ns in rcu pathwalk */
+	struct rcu_head		rcu;
 };
 
 static inline struct afs_super_info *AFS_FS_S(struct super_block *sb)
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 82bb713825a0..33b195821e36 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -523,7 +523,7 @@ static void afs_destroy_sbi(struct afs_super_info *as)
 		afs_put_volume(as->volume, afs_volume_trace_put_destroy_sbi);
 		afs_unuse_cell(as->cell, afs_cell_trace_unuse_sbi);
 		put_net(as->net_ns);
-		kfree(as);
+		kfree_rcu(as, rcu);
 	}
 }
 

-- 
2.53.0

Re: [PATCH 2/8] afs: free the super info after a grace period

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] afs: TOCTOU race in afs_atcell_get_link() causes NULL dereference
--

commit 7e87d75fc751930aad8ea9a45c609797c83f1e6b
Author: Christian Brauner <brauner@kernel.org>

afs: free the super info after a grace period

afs_atcell_get_link() resolves the @cell and .@cell symlinks of a
dynamic root in rcu pathwalk mode. Before it looks at the dentry it gets
the afs_net through afs_i2net(), which reads ->net_ns from the
afs_super_info in sb->s_fs_info. The @cell dentries are kept around when
unused and the dynamic root has no ->d_revalidate(), so a lookup that
finds @cell in the dcache gets to ->get_link() in rcu mode.

afs_kill_super() frees the afs_super_info with a plain kfree() in
afs_destroy_sbi(). Right now a pathwalk can't still be in
afs_atcell_get_link() at that point, but only by accident. umount(2)
waits for a grace period in namespace_unlock() before it drops the
mount. And a mount whose last reference goes away elsewhere, like the
chdir out of a lazily unmounted directory, still gets a grace period
from bdi_unregister(). afs_fill_super() sets up a private bdi, so
generic_shutdown_super() unregisters it from kill_anon_super() and
bdi_remove_from_list() waits in synchronize_rcu_expedited() before
afs_destroy_sbi() runs. That grace period protects the rcu readers of
bdi_list in the writeback code. It has nothing to do with pathwalk.

Don't rely on it and free the afs_super_info after a grace period.

Fixes: 30bca65bbbae ("afs: Make /afs/@cell and /afs/.@cell symlinks")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

[ ... ]

> diff --git a/fs/afs/super.c b/fs/afs/super.c
> index 82bb713825a0d..33b195821e36a 100644
> --- a/fs/afs/super.c
> +++ b/fs/afs/super.c
> @@ -523,7 +523,7 @@ static void afs_destroy_sbi(struct afs_super_info *as)
>  		afs_put_volume(as->volume, afs_volume_trace_put_destroy_sbi);
>  		afs_unuse_cell(as->cell, afs_cell_trace_unuse_sbi);
>  		put_net(as->net_ns);
> -		kfree(as);
> +		kfree_rcu(as, rcu);

[Severity: High]
Does this regression introduce a TOCTOU race in afs_atcell_get_link() that
can cause a NULL dereference?

By leaving put_net() executing before the RCU grace period, afs_net_exit() can
tear down the namespace while pathwalks are still active. If cleanup_net()
calls afs_cell_purge(), it replaces net->ws_cell with NULL.

Concurrently, a pathwalk in afs_atcell_get_link() can verify net->ws_cell is
non-NULL via rcu_access_pointer(), but the subsequent rcu_dereference() may
return NULL:

fs/afs/dynroot.c:afs_atcell_get_link() {
    ...
	if (!rcu_access_pointer(net->ws_cell))
		return ERR_PTR(-ENOENT);

	if (!dentry) {
		/* We're in RCU-pathwalk. */
		cell = rcu_dereference(net->ws_cell);
		if (dotted)
			name = cell->name - 1;
    ...
}

If the thread is preempted and afs_cell_purge() runs, could cell be NULL here?

The same appears to happen in the non-RCU pathwalk:

fs/afs/dynroot.c:afs_atcell_get_link() {
    ...
	down_read(&net->cells_lock);

	cell = rcu_dereference_protected(net->ws_cell,
					 lockdep_is_held(&net->cells_lock));
	if (dotted)
		name = cell->name - 1;
	else
		name = cell->name;
    ...
}

Can net->ws_cell be cleared to NULL before the lock is acquired, causing a
NULL dereference here as well?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=2

Re: [PATCH 2/8] afs: free the super info after a grace period

[Christian Brauner]

On Fri, Oct 09, 2026 at 10:07:58PM +0000, sashiko-bot@kernel.org wrote:
> Does this regression introduce a TOCTOU race in afs_atcell_get_link() that
> can cause a NULL dereference?
>
> By leaving put_net() executing before the RCU grace period, afs_net_exit() can
> tear down the namespace while pathwalks are still active. If cleanup_net()
> calls afs_cell_purge(), it replaces net->ws_cell with NULL.

No. This patch doesn't touch put_net(). It's called in the same place
as before.

The only place that sets net->ws_cell to NULL is afs_cell_purge() from
afs_net_exit(). The superblock pins the network namespace via
as->net_ns and once the last reference is gone cleanup_net() does a
synchronize_rcu() before it calls any ->exit() method. So
afs_cell_purge() can't run while a pathwalk is still in
afs_atcell_get_link() in rcu mode.

The double load of net->ws_cell is a bit sloppy but it's harmless.

> Can net->ws_cell be cleared to NULL before the lock is acquired, causing a
> NULL dereference here as well?

No. Without rcu the caller holds references to the dentry and the
mount. So the superblock and with it the network namespace can't go
away underneath it.

[PATCH 3/8] 9p: free the session after a grace period

[Christian Brauner]

v9fs_vfs_get_link_dotl() reads ->cache from the v9fs_session_info in
sb->s_fs_info to decide whether the symlink target sits in the page
cache. It does that before it looks at the dentry, so it also does it in
rcu pathwalk mode. Without loose or metadata caching the dentries have
no ->d_revalidate() that would make the walk leave rcu mode before it
gets there.

v9fs_kill_super() frees the session and clears sb->s_fs_info without
waiting for a grace period. Right now a pathwalk can't still be in
v9fs_vfs_get_link_dotl() at that point, but only by accident. umount(2)
waits for a grace period in namespace_unlock() before it drops the
mount. And a mount whose last reference goes away elsewhere, like the
chdir out of a lazily unmounted directory, still gets a grace period
from bdi_unregister(). v9fs_fill_super() sets up a private bdi, so
generic_shutdown_super() unregisters it from kill_anon_super() and
bdi_remove_from_list() waits in synchronize_rcu_expedited() before
v9fs_kill_super() frees the session. That grace period protects the rcu
readers of bdi_list in the writeback code. It has nothing to do with
pathwalk.

Don't rely on it. Keep sb->s_fs_info and free the session after a grace
period.

Fixes: 712da38d134b ("9p: Enable symlink caching in page cache")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/9p/v9fs.h      | 1 +
 fs/9p/vfs_super.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index a462bcbfc7da..2ac863dc8446 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -137,6 +137,7 @@ struct v9fs_session_info {
 	struct list_head slist; /* list of sessions registered with v9fs */
 	struct rw_semaphore rename_sem;
 	long session_lock_timeout; /* retry interval for blocking locks */
+	struct rcu_head rcu;	/* v9fs_vfs_get_link_dotl() reads ->cache in rcu pathwalk */
 };
 
 #define NDENTRY_TIMEOUT_NEVER (-1U)
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index 94d6b02c221b..79526044685c 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -173,8 +173,8 @@ static void v9fs_kill_super(struct super_block *s)
 
 	v9fs_session_cancel(v9ses);
 	v9fs_session_close(v9ses);
-	kfree(v9ses);
-	s->s_fs_info = NULL;
+	/* v9fs_vfs_get_link_dotl() may still read ->cache in rcu pathwalk */
+	kfree_rcu(v9ses, rcu);
 	p9_debug(P9_DEBUG_VFS, "exiting kill_super\n");
 }
 

-- 
2.53.0

Re: [PATCH 3/8] 9p: free the session after a grace period

[sashiko-bot]

> v9fs_vfs_get_link_dotl() reads ->cache from the v9fs_session_info in
> sb->s_fs_info to decide whether the symlink target sits in the page
> cache. It does that before it looks at the dentry, so it also does it in
> rcu pathwalk mode. Without loose or metadata caching the dentries have
> no ->d_revalidate() that would make the walk leave rcu mode before it
> gets there.
> 
> v9fs_kill_super() frees the session and clears sb->s_fs_info without
> waiting for a grace period. Right now a pathwalk can't still be in
> v9fs_vfs_get_link_dotl() at that point, but only by accident. umount(2)
> waits for a grace period in namespace_unlock() before it drops the
> mount. And a mount whose last reference goes away elsewhere, like the
> chdir out of a lazily unmounted directory, still gets a grace period
> from bdi_unregister(). v9fs_fill_super() sets up a private bdi, so
> generic_shutdown_super() unregisters it from kill_anon_super() and
> bdi_remove_from_list() waits in synchronize_rcu_expedited() before
> v9fs_kill_super() frees the session. That grace period protects the rcu
> readers of bdi_list in the writeback code. It has nothing to do with
> pathwalk.
> 
> Don't rely on it. Keep sb->s_fs_info and free the session after a grace
> period.
> 
> Fixes: 712da38d134b ("9p: Enable symlink caching in page cache")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=3

[PATCH 4/8] eventfs: free the eventfs_inode after an rcu grace period too

[Christian Brauner]

tracefs_d_revalidate() reads ->is_freed of the eventfs_inode that
dentry->d_fsdata points to, and ->d_revalidate() is called in rcu
pathwalk mode with nothing but rcu_read_lock() held. The dentry itself
stays around for such a walker, but tracefs_d_release() drops its
reference to the eventfs_inode when the dentry is killed, and
release_ei() then frees it after a grace period of eventfs_srcu. An
SRCU grace period doesn't wait for rcu_read_lock() sections, so the
walker can read an eventfs_inode that is already gone.

With a pathwalk held up in tracefs_d_revalidate() while the dentry goes
away:

  BUG: KASAN: slab-use-after-free in tracefs_d_revalidate+0x76/0x90
  Read of size 1 at addr ff1100010152d8d0 by task walker/97
  Call Trace:
   <TASK>
   tracefs_d_revalidate+0x76/0x90
   lookup_fast+0x320/0x640
   path_lookupat+0x160/0xb00
   filename_lookup+0x1f1/0x5b0
   vfs_statx+0xd7/0x3b0
   vfs_fstatat+0x4d/0x90
   __do_sys_newfstatat+0xa0/0x100
   do_syscall_64+0x10e/0x5e0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
  [...]
  Allocated by task 1:
   __kmalloc_cache_noprof+0x249/0x560
   eventfs_create_dir+0x7a/0x590
   event_create_dir+0x2fb/0xcf0
   trace_add_event_call+0xbf/0x150
  [...]
  Freed by task 11:
   kfree+0x1db/0x5b0
   srcu_invoke_callbacks+0x3a3/0x620
   process_one_work+0x937/0x1810
   worker_thread+0x4f1/0xd60

Before commit 8e556432477e ("eventfs: Use SRCU for freeing
eventfs_inodes") release_ei() used call_rcu(), which covered the walker,
while eventfs_iterate() needs the SRCU grace period. Wait for both and
chain call_rcu() from the SRCU callback.

Fixes: 8e556432477e ("eventfs: Use SRCU for freeing eventfs_inodes")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Reproducer: delay.c holds a task named walker in tracefs_d_revalidate()
in rcu pathwalk mode for two seconds and repro.c frees the eventfs_inode
of its dentry meanwhile. The keep event keeps the rcuwalk directory
around.

    echo 'p:rcuwalk/keep do_sys_openat2' >> /sys/kernel/tracing/kprobe_events
    echo 'p:rcuwalk/ev do_sys_openat2' >> /sys/kernel/tracing/kprobe_events
    insmod delay.ko
    ./repro

Without this patch that gives the splat above, with it it runs clean.

delay.c:

    #include <linux/delay.h>
    #include <linux/kprobes.h>
    #include <linux/module.h>
    #include <linux/namei.h>

    /* keep a task named "walker" in tracefs_d_revalidate() in rcu pathwalk */
    static int pre(struct kprobe *p, struct pt_regs *regs)
    {
    	if ((regs_get_kernel_argument(regs, 3) & LOOKUP_RCU) &&
    	    !strcmp(current->comm, "walker"))
    		mdelay(2000);
    	return 0;
    }

    static struct kprobe kp = {
    	.symbol_name	= "tracefs_d_revalidate",
    	.pre_handler	= pre,
    };

    static int __init delay_init(void)
    {
    	return register_kprobe(&kp);
    }

    static void __exit delay_exit(void)
    {
    	unregister_kprobe(&kp);
    }

    module_init(delay_init);
    module_exit(delay_exit);
    MODULE_LICENSE("GPL");

repro.c:

    #define _GNU_SOURCE
    #include <fcntl.h>
    #include <pthread.h>
    #include <sched.h>
    #include <sys/prctl.h>
    #include <sys/stat.h>
    #include <unistd.h>

    static void pin(int cpu)
    {
    	cpu_set_t set;

    	CPU_ZERO(&set);
    	CPU_SET(cpu, &set);
    	sched_setaffinity(0, sizeof(set), &set);
    }

    static void *walker(void *arg)
    {
    	struct stat st;

    	pin(1);
    	prctl(PR_SET_NAME, "walker");
    	stat("ev", &st);
    	return NULL;
    }

    int main(void)
    {
    	struct stat st;
    	pthread_t t;
    	int fd;

    	pin(0);
    	chdir("/sys/kernel/tracing/events/rcuwalk");
    	stat("ev", &st);
    	/* sets ->is_freed, the dentry stays hashed and holds the ei */
    	fd = open("/sys/kernel/tracing/kprobe_events", O_WRONLY | O_APPEND);
    	write(fd, "-:rcuwalk/ev\n", 13);
    	close(fd);
    	pthread_create(&t, NULL, walker, NULL);
    	sleep(1);
    	/* ->d_revalidate() says 0, the dentry is killed and drops the ei */
    	stat("ev", &st);
    	pthread_join(t, NULL);
    	return 0;
    }
---
 fs/tracefs/event_inode.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 6e3513b13cfa..090aca946717 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -91,6 +91,14 @@ static void free_ei_rcu(struct rcu_head *rcu)
 	}
 }
 
+/* read under eventfs_srcu and, by tracefs_d_revalidate(), in rcu pathwalk */
+static void free_ei_srcu(struct rcu_head *rcu)
+{
+	struct eventfs_inode *ei = container_of(rcu, struct eventfs_inode, rcu);
+
+	call_rcu(&ei->rcu, free_ei_rcu);
+}
+
 /*
  * eventfs_inode reference count management.
  *
@@ -112,7 +120,7 @@ static void release_ei(struct kref *ref)
 			entry->release(entry->name, ei->data);
 	}
 
-	call_srcu(&eventfs_srcu, &ei->rcu, free_ei_rcu);
+	call_srcu(&eventfs_srcu, &ei->rcu, free_ei_srcu);
 }
 
 static inline void put_ei(struct eventfs_inode *ei)

-- 
2.53.0

Re: [PATCH 4/8] eventfs: free the eventfs_inode after an rcu grace period too

[sashiko-bot]

> tracefs_d_revalidate() reads ->is_freed of the eventfs_inode that
> dentry->d_fsdata points to, and ->d_revalidate() is called in rcu
> pathwalk mode with nothing but rcu_read_lock() held. The dentry itself
> stays around for such a walker, but tracefs_d_release() drops its
> reference to the eventfs_inode when the dentry is killed, and
> release_ei() then frees it after a grace period of eventfs_srcu. An
> SRCU grace period doesn't wait for rcu_read_lock() sections, so the
> walker can read an eventfs_inode that is already gone.
> 
> With a pathwalk held up in tracefs_d_revalidate() while the dentry goes
> away:
> 
>   BUG: KASAN: slab-use-after-free in tracefs_d_revalidate+0x76/0x90
>   Read of size 1 at addr ff1100010152d8d0 by task walker/97
>   Call Trace:
> [ ... ]
> chain call_rcu() from the SRCU callback.
> 
> Fixes: 8e556432477e ("eventfs: Use SRCU for freeing eventfs_inodes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=4

[PATCH 5/8] ipc: drop the mqueue mount with kern_unmount() on setup failure

[Christian Brauner]

mq_init_ns() creates the mqueue mount with fc_mount_longterm(), so its
->mnt_ns is MNT_NS_INTERNAL. When setting up the mqueue or ipc sysctls
or msg_init_ns() fails afterwards, create_ipc_ns() drops it with a plain
mntput(). With ->mnt_ns set that takes the fast path of
mntput_no_expire(). The count drops to zero and nothing ever kills the
mount, so the mount and the mqueue superblock are leaked.

Commit dc55e35f9e81 ("ipc: Store mqueue sysctls in the ipc namespace")
added the first failure path after mq_init_ns() and never dropped the
mount. Commit 7229d74e5e8c ("ipc: create_ipc_ns: drop mqueue mount on
sysctl setup failure") tried to fix that with a mntput() but that
doesn't help for a long-term mount.

Use kern_unmount(). free_ipc() open-codes the same thing for a namespace
that made it.

Fixes: dc55e35f9e81 ("ipc: Store mqueue sysctls in the ipc namespace")
Fixes: 7229d74e5e8c ("ipc: create_ipc_ns: drop mqueue mount on sysctl setup failure")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Reproducer: repro.c fails each slab allocation of unshare(CLONE_NEWIPC)
in turn with fail-nth, a hundred rounds. It needs CONFIG_FAILSLAB and
CONFIG_FAULT_INJECTION_DEBUG_FS and runs as root.

    echo N > /sys/kernel/debug/failslab/ignore-gfp-wait
    echo 0 > /sys/kernel/debug/failslab/verbose
    echo 2 > /proc/sys/vm/drop_caches; cat /proc/sys/fs/inode-nr
    ./repro 100
    sleep 2
    echo 2 > /proc/sys/vm/drop_caches; cat /proc/sys/fs/inode-nr

Every failure in setup_mq_sysctls() and setup_ipc_sysctls() leaves an
mqueue superblock behind with its root inode, about seven per round.
Without this patch inode-nr goes from 967 to 1678, with it it stays at
967.

repro.c:

    #define _GNU_SOURCE
    #include <fcntl.h>
    #include <sched.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <sys/wait.h>
    #include <unistd.h>

    /*
     * Fail the nth slab allocation of unshare(CLONE_NEWIPC). Returns 1 once
     * nth is past the last one: nothing was injected and unshare() worked.
     */
    static int last(int nth)
    {
    	int status;

    	if (!fork()) {
    		char buf[16] = "";
    		int fd, ret;

    		fd = open("/proc/thread-self/fail-nth", O_RDWR);
    		snprintf(buf, sizeof(buf), "%d", nth);
    		write(fd, buf, strlen(buf));
    		ret = unshare(CLONE_NEWIPC);
    		pread(fd, buf, sizeof(buf) - 1, 0);
    		_exit(!ret && atoi(buf));
    	}
    	wait(&status);
    	return WEXITSTATUS(status);
    }

    int main(int argc, char *argv[])
    {
    	for (int i = 0; i < atoi(argv[1]); i++)
    		for (int nth = 1; !last(nth); nth++)
    			;
    	return 0;
    }
---
 ipc/namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipc/namespace.c b/ipc/namespace.c
index 1e71353bdb4a..75aa7b469348 100644
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -96,7 +96,7 @@ static struct ipc_namespace *create_ipc_ns(struct user_namespace *user_ns,
 fail_mq_sysctls:
 	retire_mq_sysctls(ns);
 fail_mq_mount:
-	mntput(ns->mq_mnt);
+	kern_unmount(ns->mq_mnt);
 fail_put:
 	put_user_ns(ns->user_ns);
 	ns_common_free(ns);

-- 
2.53.0

Re: [PATCH 5/8] ipc: drop the mqueue mount with kern_unmount() on setup failure

[sashiko-bot]

> mq_init_ns() creates the mqueue mount with fc_mount_longterm(), so its
> ->mnt_ns is MNT_NS_INTERNAL. When setting up the mqueue or ipc sysctls
> or msg_init_ns() fails afterwards, create_ipc_ns() drops it with a plain
> mntput(). With ->mnt_ns set that takes the fast path of
> mntput_no_expire(). The count drops to zero and nothing ever kills the
> mount, so the mount and the mqueue superblock are leaked.
> 
> Commit dc55e35f9e81 ("ipc: Store mqueue sysctls in the ipc namespace")
> added the first failure path after mq_init_ns() and never dropped the
> mount. Commit 7229d74e5e8c ("ipc: create_ipc_ns: drop mqueue mount on
> sysctl setup failure") tried to fix that with a mntput() but that
> doesn't help for a long-term mount.
> 
> Use kern_unmount(). free_ipc() open-codes the same thing for a namespace
> that made it.
> 
> Fixes: dc55e35f9e81 ("ipc: Store mqueue sysctls in the ipc namespace")
> Fixes: 7229d74e5e8c ("ipc: create_ipc_ns: drop mqueue mount on sysctl setup failure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=5

[PATCH 6/8] unicode: free the map after a grace period

[Christian Brauner]

generic_ci_d_hash() and generic_ci_d_compare() read sb->s_encoding and
dereference the unicode map while a pathwalk in rcu mode holds nothing
but rcu_read_lock(). The filesystems that use them, ext4, f2fs and
tmpfs, drop the map with utf8_unload() from ->put_super(), which runs
when the last reference to the mount goes away. A lookup that is still
inside such a filesystem at that point reads a freed map.

With a pathwalk held up in utf8_casefold_hash() while the last reference
to a casefolded tmpfs goes away:

  BUG: KASAN: slab-use-after-free in utf8nlookup+0x74c/0x950
  Read of size 8 at addr ff110001030fe658 by task walker/98
  Call Trace:
   <TASK>
   utf8nlookup+0x74c/0x950
   utf8byte+0x160/0x1560
   utf8_casefold_hash+0x167/0x230
   generic_ci_d_hash+0xdd/0x1c0
   link_path_walk+0x11ad/0x1d30
   path_lookupat+0x63/0xb00
   filename_lookup+0x1f1/0x5b0
   vfs_statx+0xd7/0x3b0
  [...]
  Allocated by task 94:
   __kmalloc_cache_noprof+0x249/0x560
   utf8_load+0x4e/0x570
   shmem_parse_opt_casefold.isra.0+0x92/0x270
  [...]
  Freed by task 97:
   kfree+0x1db/0x5b0
   shmem_put_super+0x72/0x170
   generic_shutdown_super+0x14c/0x530
   kill_anon_super+0x3a/0x60
   deactivate_locked_super+0x9c/0xf0
   cleanup_mnt+0x9f/0x130

So far that has been hard to hit. umount(2) waits for a grace period
in namespace_unlock() before it drops the mount, so only a mount whose
last reference goes away elsewhere, like the close of a file or a chdir
out of a lazily unmounted directory, can race a walker. So it is
reachable.

Delay the free until a grace period has passed. The map and the data
table it holds a reference to then outlive any pathwalk that found the
superblock before the mount went away. The reference on the data table
module is dropped from the rcu callback as well. utf8-core is built in
whenever CONFIG_UNICODE is set, so there is no module exit that would
have to wait for the callbacks.

Overlayfs points the s_encoding of an overlay at the map of its first
layer without taking a reference of its own. That stays correct. The
layer is held by the overlay, so its map is dropped only after the
overlay is gone.

Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Reproducer: delay.c holds a task named walker in utf8_casefold_hash() in
rcu pathwalk mode for two seconds and repro.c drops the last reference
to a lazily unmounted casefolded tmpfs meanwhile.

    mount -t tmpfs -o casefold tmpfs /mnt
    chattr +F /mnt
    echo hello > /mnt/Foo
    insmod delay.ko
    ./repro

Without this patch that gives the splat above, with it it runs clean.

delay.c:

    #include <linux/delay.h>
    #include <linux/kprobes.h>
    #include <linux/module.h>

    /* keep a task named "walker" in utf8_casefold_hash() in rcu pathwalk */
    static int pre(struct kprobe *p, struct pt_regs *regs)
    {
    	if (!strcmp(current->comm, "walker"))
    		mdelay(2000);
    	return 0;
    }

    static struct kprobe kp = {
    	.symbol_name	= "utf8_casefold_hash",
    	.pre_handler	= pre,
    };

    static int __init delay_init(void)
    {
    	return register_kprobe(&kp);
    }

    static void __exit delay_exit(void)
    {
    	unregister_kprobe(&kp);
    }

    module_init(delay_init);
    module_exit(delay_exit);
    MODULE_LICENSE("GPL");

repro.c:

    #define _GNU_SOURCE
    #include <pthread.h>
    #include <sched.h>
    #include <string.h>
    #include <sys/mman.h>
    #include <sys/mount.h>
    #include <sys/prctl.h>
    #include <sys/stat.h>
    #include <unistd.h>

    static void pin(int cpu)
    {
    	cpu_set_t set;

    	CPU_ZERO(&set);
    	CPU_SET(cpu, &set);
    	sched_setaffinity(0, sizeof(set), &set);
    }

    static void *walker(void *arg)
    {
    	struct stat st;
    	char *p;

    	/*
    	 * The delay spins. Empty this cpu's lru batches so that an
    	 * lru_add_drain_all() on the other side has no work to wait for here.
    	 */
    	pin(1);
    	p = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    	memset(p, 1, 4096);
    	madvise(p, 4096, MADV_COLD);

    	prctl(PR_SET_NAME, "walker");
    	stat("FOO", &st);
    	return NULL;
    }

    int main(void)
    {
    	struct stat st;
    	pthread_t t;

    	pin(0);
    	chdir("/mnt");
    	stat("FOO", &st);
    	/* the cwd now holds the last reference to the mount */
    	umount2("/mnt", MNT_DETACH);
    	pthread_create(&t, NULL, walker, NULL);
    	sleep(1);
    	/* drop it, ->kill_sb() runs before chdir() returns */
    	chdir("/");
    	pthread_join(t, NULL);
    	return 0;
    }
---
 fs/unicode/utf8-core.c  | 21 +++++++++++++++++----
 include/linux/unicode.h |  1 +
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/unicode/utf8-core.c b/fs/unicode/utf8-core.c
index f313532f5e80..74a74a48dd8b 100644
--- a/fs/unicode/utf8-core.c
+++ b/fs/unicode/utf8-core.c
@@ -3,6 +3,7 @@
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/slab.h>
+#include <linux/rcupdate.h>
 #include <linux/parser.h>
 #include <linux/errno.h>
 #include <linux/stringhash.h>
@@ -183,12 +184,24 @@ struct unicode_map *utf8_load(unsigned int version)
 }
 EXPORT_SYMBOL(utf8_load);
 
+static void utf8_unload_rcu(struct rcu_head *head)
+{
+	struct unicode_map *um = container_of(head, struct unicode_map, rcu);
+
+	symbol_put(utf8_data_table);
+	kfree(um);
+}
+
+/*
+ * A pathwalk in rcu mode may still be in ->d_hash() or ->d_compare() of a
+ * filesystem that is being torn down and read the map through its
+ * superblock, so the map and the tables it points to have to outlive the
+ * grace period.
+ */
 void utf8_unload(struct unicode_map *um)
 {
-	if (um) {
-		symbol_put(utf8_data_table);
-		kfree(um);
-	}
+	if (um)
+		call_rcu(&um->rcu, utf8_unload_rcu);
 }
 EXPORT_SYMBOL(utf8_unload);
 
diff --git a/include/linux/unicode.h b/include/linux/unicode.h
index 64fa44fe180c..3a8cb603eb93 100644
--- a/include/linux/unicode.h
+++ b/include/linux/unicode.h
@@ -53,6 +53,7 @@ struct unicode_map {
 	unsigned int version;
 	const struct utf8data *ntab[UTF8_NMAX];
 	const struct utf8data_table *tables;
+	struct rcu_head rcu;
 };
 
 int utf8_validate(const struct unicode_map *um, const struct qstr *str);

-- 
2.53.0

Re: [PATCH 6/8] unicode: free the map after a grace period

[sashiko-bot]

> generic_ci_d_hash() and generic_ci_d_compare() read sb->s_encoding and
> dereference the unicode map while a pathwalk in rcu mode holds nothing
> but rcu_read_lock(). The filesystems that use them, ext4, f2fs and
> tmpfs, drop the map with utf8_unload() from ->put_super(), which runs
> when the last reference to the mount goes away. A lookup that is still
> inside such a filesystem at that point reads a freed map.
> 
> With a pathwalk held up in utf8_casefold_hash() while the last reference
> to a casefolded tmpfs goes away:
> 
>   BUG: KASAN: slab-use-after-free in utf8nlookup+0x74c/0x950
>   Read of size 8 at addr ff110001030fe658 by task walker/98
>   Call Trace:
>    <TASK>
>    utf8nlookup+0x74c/0x950
> [ ... ]
> overlay is gone.
> 
> Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=6

Re: [PATCH 6/8] unicode: free the map after a grace period

[Jan Kara]

On Fri 09-10-26 23:51:56, Christian Brauner wrote:
> generic_ci_d_hash() and generic_ci_d_compare() read sb->s_encoding and
> dereference the unicode map while a pathwalk in rcu mode holds nothing
> but rcu_read_lock(). The filesystems that use them, ext4, f2fs and
> tmpfs, drop the map with utf8_unload() from ->put_super(), which runs
> when the last reference to the mount goes away. A lookup that is still
> inside such a filesystem at that point reads a freed map.
> 
> With a pathwalk held up in utf8_casefold_hash() while the last reference
> to a casefolded tmpfs goes away:
> 
>   BUG: KASAN: slab-use-after-free in utf8nlookup+0x74c/0x950
>   Read of size 8 at addr ff110001030fe658 by task walker/98
>   Call Trace:
>    <TASK>
>    utf8nlookup+0x74c/0x950
>    utf8byte+0x160/0x1560
>    utf8_casefold_hash+0x167/0x230
>    generic_ci_d_hash+0xdd/0x1c0
>    link_path_walk+0x11ad/0x1d30
>    path_lookupat+0x63/0xb00
>    filename_lookup+0x1f1/0x5b0
>    vfs_statx+0xd7/0x3b0
>   [...]
>   Allocated by task 94:
>    __kmalloc_cache_noprof+0x249/0x560
>    utf8_load+0x4e/0x570
>    shmem_parse_opt_casefold.isra.0+0x92/0x270
>   [...]
>   Freed by task 97:
>    kfree+0x1db/0x5b0
>    shmem_put_super+0x72/0x170
>    generic_shutdown_super+0x14c/0x530
>    kill_anon_super+0x3a/0x60
>    deactivate_locked_super+0x9c/0xf0
>    cleanup_mnt+0x9f/0x130
> 
> So far that has been hard to hit. umount(2) waits for a grace period
> in namespace_unlock() before it drops the mount, so only a mount whose
> last reference goes away elsewhere, like the close of a file or a chdir
> out of a lazily unmounted directory, can race a walker. So it is
> reachable.
> 
> Delay the free until a grace period has passed. The map and the data
> table it holds a reference to then outlive any pathwalk that found the
> superblock before the mount went away. The reference on the data table
> module is dropped from the rcu callback as well. utf8-core is built in
> whenever CONFIG_UNICODE is set, so there is no module exit that would
> have to wait for the callbacks.
> 
> Overlayfs points the s_encoding of an overlay at the map of its first
> layer without taking a reference of its own. That stays correct. The
> layer is held by the overlay, so its map is dropped only after the
> overlay is gone.
> 
> Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Reproducer: delay.c holds a task named walker in utf8_casefold_hash() in
> rcu pathwalk mode for two seconds and repro.c drops the last reference
> to a lazily unmounted casefolded tmpfs meanwhile.
> 
>     mount -t tmpfs -o casefold tmpfs /mnt
>     chattr +F /mnt
>     echo hello > /mnt/Foo
>     insmod delay.ko
>     ./repro
> 
> Without this patch that gives the splat above, with it it runs clean.
> 
> delay.c:
> 
>     #include <linux/delay.h>
>     #include <linux/kprobes.h>
>     #include <linux/module.h>
> 
>     /* keep a task named "walker" in utf8_casefold_hash() in rcu pathwalk */
>     static int pre(struct kprobe *p, struct pt_regs *regs)
>     {
>     	if (!strcmp(current->comm, "walker"))
>     		mdelay(2000);
>     	return 0;
>     }
> 
>     static struct kprobe kp = {
>     	.symbol_name	= "utf8_casefold_hash",
>     	.pre_handler	= pre,
>     };
> 
>     static int __init delay_init(void)
>     {
>     	return register_kprobe(&kp);
>     }
> 
>     static void __exit delay_exit(void)
>     {
>     	unregister_kprobe(&kp);
>     }
> 
>     module_init(delay_init);
>     module_exit(delay_exit);
>     MODULE_LICENSE("GPL");
> 
> repro.c:
> 
>     #define _GNU_SOURCE
>     #include <pthread.h>
>     #include <sched.h>
>     #include <string.h>
>     #include <sys/mman.h>
>     #include <sys/mount.h>
>     #include <sys/prctl.h>
>     #include <sys/stat.h>
>     #include <unistd.h>
> 
>     static void pin(int cpu)
>     {
>     	cpu_set_t set;
> 
>     	CPU_ZERO(&set);
>     	CPU_SET(cpu, &set);
>     	sched_setaffinity(0, sizeof(set), &set);
>     }
> 
>     static void *walker(void *arg)
>     {
>     	struct stat st;
>     	char *p;
> 
>     	/*
>     	 * The delay spins. Empty this cpu's lru batches so that an
>     	 * lru_add_drain_all() on the other side has no work to wait for here.
>     	 */
>     	pin(1);
>     	p = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>     	memset(p, 1, 4096);
>     	madvise(p, 4096, MADV_COLD);
> 
>     	prctl(PR_SET_NAME, "walker");
>     	stat("FOO", &st);
>     	return NULL;
>     }
> 
>     int main(void)
>     {
>     	struct stat st;
>     	pthread_t t;
> 
>     	pin(0);
>     	chdir("/mnt");
>     	stat("FOO", &st);
>     	/* the cwd now holds the last reference to the mount */
>     	umount2("/mnt", MNT_DETACH);
>     	pthread_create(&t, NULL, walker, NULL);
>     	sleep(1);
>     	/* drop it, ->kill_sb() runs before chdir() returns */
>     	chdir("/");
>     	pthread_join(t, NULL);
>     	return 0;
>     }
> ---
>  fs/unicode/utf8-core.c  | 21 +++++++++++++++++----
>  include/linux/unicode.h |  1 +
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/unicode/utf8-core.c b/fs/unicode/utf8-core.c
> index f313532f5e80..74a74a48dd8b 100644
> --- a/fs/unicode/utf8-core.c
> +++ b/fs/unicode/utf8-core.c
> @@ -3,6 +3,7 @@
>  #include <linux/kernel.h>
>  #include <linux/string.h>
>  #include <linux/slab.h>
> +#include <linux/rcupdate.h>
>  #include <linux/parser.h>
>  #include <linux/errno.h>
>  #include <linux/stringhash.h>
> @@ -183,12 +184,24 @@ struct unicode_map *utf8_load(unsigned int version)
>  }
>  EXPORT_SYMBOL(utf8_load);
>  
> +static void utf8_unload_rcu(struct rcu_head *head)
> +{
> +	struct unicode_map *um = container_of(head, struct unicode_map, rcu);
> +
> +	symbol_put(utf8_data_table);
> +	kfree(um);
> +}
> +
> +/*
> + * A pathwalk in rcu mode may still be in ->d_hash() or ->d_compare() of a
> + * filesystem that is being torn down and read the map through its
> + * superblock, so the map and the tables it points to have to outlive the
> + * grace period.
> + */
>  void utf8_unload(struct unicode_map *um)
>  {
> -	if (um) {
> -		symbol_put(utf8_data_table);
> -		kfree(um);
> -	}
> +	if (um)
> +		call_rcu(&um->rcu, utf8_unload_rcu);
>  }
>  EXPORT_SYMBOL(utf8_unload);
>  
> diff --git a/include/linux/unicode.h b/include/linux/unicode.h
> index 64fa44fe180c..3a8cb603eb93 100644
> --- a/include/linux/unicode.h
> +++ b/include/linux/unicode.h
> @@ -53,6 +53,7 @@ struct unicode_map {
>  	unsigned int version;
>  	const struct utf8data *ntab[UTF8_NMAX];
>  	const struct utf8data_table *tables;
> +	struct rcu_head rcu;
>  };
>  
>  int utf8_validate(const struct unicode_map *um, const struct qstr *str);
> 
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 7/8] ntfs3: free the sbi after a grace period

[Christian Brauner]

If the "nocase" mount option ntfs3 uses custom hash and dentry compare
functions. Both access ntfs3 superblock private information in rcu
pathwalk. ntfs_put_super() and ntfs3_kill_sb() drop the info without any
rcu delay. A lookup that's still inside the filesystem when the last
reference to the mount is dropped causes a UAF. With a pathwalk held up
in ntfs_d_hash() while the last reference to the mount goes away:

  BUG: KASAN: slab-use-after-free in ntfs_nls_to_utf16+0x752/0x940
  Read of size 8 at addr ff1100010453c850 by task walker/99
  Call Trace:
   <TASK>
   ntfs_nls_to_utf16+0x752/0x940
   ntfs_d_hash+0x22b/0x500
   link_path_walk+0x11ad/0x1d30
   path_lookupat+0x63/0xb00
   filename_lookup+0x1f1/0x5b0
   vfs_statx+0xd7/0x3b0
  [...]
  Allocated by task 95:
   __kmalloc_cache_noprof+0x249/0x560
   ntfs_init_fs_context+0x2db/0x5a0
   alloc_fs_context+0x52e/0xc70
  [...]
  Freed by task 98:
   kfree+0x1db/0x5b0
   deactivate_locked_super+0x9c/0xf0
   cleanup_mnt+0x9f/0x130
   task_work_run+0x131/0x250

The bug is currently masked by the grace period in namespace_unlock()
for umount(2). Keep the necessary information until the filesystem
private info is freed and free it with an rcu delay.

ntfs3_free_sbi() now runs from an rcu callback and drops the upcase
table via ntfs_put_shared(). So switch s_shared_lock over to
spin_lock_bh(). Otherwise the callback can deadlock against a task
holding the lock on the same cpu.

Fixes: a3a956c78efa ("fs/ntfs3: Add option "nocase"")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Reproducer: delay.c holds a task named walker in ntfs_d_hash() in rcu
pathwalk mode for two seconds and repro.c drops the last reference to a
lazily unmounted ntfs3 meanwhile. The name is not ascii, so
ntfs_d_hash() goes through the sbi.

    mkntfs -F -f -Q /dev/vdb
    mount -t ntfs3 -o nocase,noatime /dev/vdb /mnt
    echo hello > /mnt/$(printf '\303\244')
    insmod delay.ko
    ./repro

Without this patch that gives the splat above, with it it runs clean.

delay.c:

    #include <linux/delay.h>
    #include <linux/kprobes.h>
    #include <linux/module.h>

    /* keep a task named "walker" in ntfs_d_hash() in rcu pathwalk */
    static int pre(struct kprobe *p, struct pt_regs *regs)
    {
    	if (!strcmp(current->comm, "walker"))
    		mdelay(2000);
    	return 0;
    }

    static struct kprobe kp = {
    	.symbol_name	= "ntfs_d_hash",
    	.pre_handler	= pre,
    };

    static int __init delay_init(void)
    {
    	return register_kprobe(&kp);
    }

    static void __exit delay_exit(void)
    {
    	unregister_kprobe(&kp);
    }

    module_init(delay_init);
    module_exit(delay_exit);
    MODULE_LICENSE("GPL");

repro.c:

    #define _GNU_SOURCE
    #include <pthread.h>
    #include <sched.h>
    #include <string.h>
    #include <sys/mman.h>
    #include <sys/mount.h>
    #include <sys/prctl.h>
    #include <sys/stat.h>
    #include <unistd.h>

    static void pin(int cpu)
    {
    	cpu_set_t set;

    	CPU_ZERO(&set);
    	CPU_SET(cpu, &set);
    	sched_setaffinity(0, sizeof(set), &set);
    }

    static void *walker(void *arg)
    {
    	struct stat st;
    	char *p;

    	/*
    	 * The delay spins. Empty this cpu's lru batches so that an
    	 * lru_add_drain_all() on the other side has no work to wait for here.
    	 */
    	pin(1);
    	p = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    	memset(p, 1, 4096);
    	madvise(p, 4096, MADV_COLD);

    	prctl(PR_SET_NAME, "walker");
    	stat("\xc3\xa4", &st);
    	return NULL;
    }

    int main(void)
    {
    	struct stat st;
    	pthread_t t;

    	pin(0);
    	chdir("/mnt");
    	stat("\xc3\xa4", &st);
    	/* the cwd now holds the last reference to the mount */
    	umount2("/mnt", MNT_DETACH);
    	pthread_create(&t, NULL, walker, NULL);
    	sleep(1);
    	/* drop it, ->kill_sb() runs before chdir() returns */
    	chdir("/");
    	pthread_join(t, NULL);
    	return 0;
    }
---
 fs/ntfs3/ntfs_fs.h |  1 +
 fs/ntfs3/super.c   | 33 +++++++++++++++++++++------------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 5811d89d67b3..bbcd2e02bf63 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -211,6 +211,7 @@ struct ntfs_index {
 /* Ntfs file system in-core superblock data. */
 struct ntfs_sb_info {
 	struct super_block *sb;
+	struct rcu_head rcu;
 
 	u32 discard_granularity;
 	u64 discard_granularity_mask_inv; // ~(discard_granularity_mask_inv-1)
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index f4a42a0c73a4..841f9068d82f 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -196,7 +196,7 @@ void *ntfs_set_shared(void *ptr, u32 bytes)
 	void *ret = NULL;
 	int i, j = -1;
 
-	spin_lock(&s_shared_lock);
+	spin_lock_bh(&s_shared_lock);
 	for (i = 0; i < ARRAY_SIZE(s_shared); i++) {
 		if (!s_shared[i].cnt) {
 			j = i;
@@ -214,7 +214,7 @@ void *ntfs_set_shared(void *ptr, u32 bytes)
 		s_shared[j].cnt = 1;
 		ret = ptr;
 	}
-	spin_unlock(&s_shared_lock);
+	spin_unlock_bh(&s_shared_lock);
 
 	return ret;
 }
@@ -231,7 +231,7 @@ void *ntfs_put_shared(void *ptr)
 	void *ret = ptr;
 	int i;
 
-	spin_lock(&s_shared_lock);
+	spin_lock_bh(&s_shared_lock);
 	for (i = 0; i < ARRAY_SIZE(s_shared); i++) {
 		if (s_shared[i].cnt && s_shared[i].ptr == ptr) {
 			if (--s_shared[i].cnt)
@@ -239,7 +239,7 @@ void *ntfs_put_shared(void *ptr)
 			break;
 		}
 	}
-	spin_unlock(&s_shared_lock);
+	spin_unlock_bh(&s_shared_lock);
 
 	return ret;
 }
@@ -708,6 +708,8 @@ static noinline void ntfs3_put_sbi(struct ntfs_sb_info *sbi)
 
 static void ntfs3_free_sbi(struct ntfs_sb_info *sbi)
 {
+	if (sbi->options)
+		put_mount_options(sbi->options);
 	kfree(sbi->new_rec);
 	kvfree(ntfs_put_shared(sbi->upcase));
 	kvfree(sbi->def_table);
@@ -719,6 +721,11 @@ static void ntfs3_free_sbi(struct ntfs_sb_info *sbi)
 	kfree(sbi);
 }
 
+static void ntfs3_free_sbi_rcu(struct rcu_head *head)
+{
+	ntfs3_free_sbi(container_of(head, struct ntfs_sb_info, rcu));
+}
+
 static void ntfs_put_super(struct super_block *sb)
 {
 	struct ntfs_sb_info *sbi = sb->s_fs_info;
@@ -728,11 +735,11 @@ static void ntfs_put_super(struct super_block *sb)
 	/* Mark rw ntfs as clear, if possible. */
 	ntfs_set_state(sbi, NTFS_DIRTY_CLEAR);
 
-	if (sbi->options) {
-		put_mount_options(sbi->options);
-		sbi->options = NULL;
-	}
-
+	/*
+	 * The mount options stay until the sbi is freed: ->d_hash() and
+	 * ->d_compare() of a pathwalk in rcu mode read the nls table through
+	 * them.
+	 */
 	ntfs3_put_sbi(sbi);
 }
 
@@ -1939,9 +1946,11 @@ static void ntfs3_kill_sb(struct super_block *sb)
 
 	kill_block_super(sb);
 
-	if (sbi->options)
-		put_mount_options(sbi->options);
-	ntfs3_free_sbi(sbi);
+	/*
+	 * A pathwalk in rcu mode may still be in ->d_hash() or ->d_compare()
+	 * and read the upcase table and the nls table through the sbi.
+	 */
+	call_rcu(&sbi->rcu, ntfs3_free_sbi_rcu);
 }
 
 // clang-format off

-- 
2.53.0

Re: [PATCH 7/8] ntfs3: free the sbi after a grace period

[sashiko-bot]

> If the "nocase" mount option ntfs3 uses custom hash and dentry compare
> functions. Both access ntfs3 superblock private information in rcu
> pathwalk. ntfs_put_super() and ntfs3_kill_sb() drop the info without any
> rcu delay. A lookup that's still inside the filesystem when the last
> reference to the mount is dropped causes a UAF. With a pathwalk held up
> in ntfs_d_hash() while the last reference to the mount goes away:
> 
>   BUG: KASAN: slab-use-after-free in ntfs_nls_to_utf16+0x752/0x940
>   Read of size 8 at addr ff1100010453c850 by task walker/99
>   Call Trace:
>    <TASK>
>    ntfs_nls_to_utf16+0x752/0x940
>    ntfs_d_hash+0x22b/0x500
>    link_path_walk+0x11ad/0x1d30
>    path_lookupat+0x63/0xb00
> [ ... ]
> holding the lock on the same cpu.
> 
> Fixes: a3a956c78efa ("fs/ntfs3: Add option "nocase"")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=7

[PATCH 8/8] ntfs3: free replaced mount options after a grace period

[Christian Brauner]

ntfs_fs_reconfigure() replaces the old mount options with new mount
options in sbi->options. The old mount options land in fs_context and
are freed as soon as remount returns. With a pathwalk held up in
ntfs_nls_to_utf16() between loading sbi->options and dereferencing it
while a remount replaces them:

  BUG: KASAN: slab-use-after-free in ntfs_nls_to_utf16+0x7ae/0x7c0
  Read of size 8 at addr ff110001044fd448 by task walker/98
  Call Trace:
   <TASK>
   ntfs_nls_to_utf16+0x7ae/0x7c0
   ntfs_d_hash+0x22b/0x500
   link_path_walk+0x11ad/0x1d30
   path_lookupat+0x63/0xb00
   filename_lookup+0x1f1/0x5b0
   vfs_statx+0xd7/0x3b0
  [...]
  Allocated by task 95:
   __kmalloc_node_track_caller_noprof+0x2a2/0x730
   kmemdup_noprof+0x25/0x50
   ntfs_fill_super+0x1e5/0x4550
  [...]
  Freed by task 97:
   kfree+0x1db/0x5b0
   put_fs_context+0x13c/0x7d0
   path_mount+0x54a/0x2130
   __x64_sys_mount+0x20c/0x270

That info is needed in rcu lookup in the dentry hash and dentry compary
methods for ntfs3 and causes a UAF today. Delay freeing with an rcu
grace period.

ntfs_nls_to_utf16() reads ->nls and ->ads once under rcu_read_lock() so
the hooks pair with the release in ntfs_fs_reconfigure(). That also
covers callers outside of rcu pathwalk such as ntfs_lookup(). They don't
hold rcu_read_lock() themselves and wouldn't be protected by the grace
period otherwise. Remount refuses to change the iocharset so ->nls stays
valid after rcu_read_unlock().

Fixes: a3a956c78efa ("fs/ntfs3: Add option "nocase"")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Reproducer: the window is the load of sbi->options in
ntfs_nls_to_utf16() and the dereference right after it, so widen it in
the kernel: split

    struct nls_table *nls = sbi->options->nls;

into a load of sbi->options, an mdelay(2000) when current->comm is
"walker", and the load of ->nls from the options read before the delay.
repro.c then remounts while a task named walker sits in that delay. The
name is not ascii, so ntfs_d_hash() goes through the options.

    mkntfs -F -f -Q /dev/vdb
    mount -t ntfs3 -o nocase,noatime /dev/vdb /mnt
    echo hello > /mnt/$(printf '\303\244')
    ./repro

Without this patch that gives the splat above, with it (and the same
delay between the READ_ONCE() and ->nls) it runs clean.

repro.c:

    #define _GNU_SOURCE
    #include <pthread.h>
    #include <sched.h>
    #include <string.h>
    #include <sys/mman.h>
    #include <sys/mount.h>
    #include <sys/prctl.h>
    #include <sys/stat.h>
    #include <unistd.h>

    static void pin(int cpu)
    {
    	cpu_set_t set;

    	CPU_ZERO(&set);
    	CPU_SET(cpu, &set);
    	sched_setaffinity(0, sizeof(set), &set);
    }

    static void *walker(void *arg)
    {
    	struct stat st;
    	char *p;

    	/*
    	 * The delay spins. Empty this cpu's lru batches so that an
    	 * lru_add_drain_all() on the other side has no work to wait for here.
    	 */
    	pin(1);
    	p = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    	memset(p, 1, 4096);
    	madvise(p, 4096, MADV_COLD);

    	prctl(PR_SET_NAME, "walker");
    	stat("\xc3\xa4", &st);
    	return NULL;
    }

    int main(void)
    {
    	struct stat st;
    	pthread_t t;

    	pin(0);
    	chdir("/mnt");
    	stat("\xc3\xa4", &st);
    	pthread_create(&t, NULL, walker, NULL);
    	sleep(1);
    	/* replaces sbi->options and frees the old ones */
    	mount("none", "/mnt", NULL, MS_REMOUNT, "nocase");
    	pthread_join(t, NULL);
    	return 0;
    }
---
 fs/ntfs3/dir.c     | 13 +++++++++++--
 fs/ntfs3/ntfs_fs.h |  2 ++
 fs/ntfs3/super.c   | 16 +++++++++++++++-
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index eb9152e9fa22..14ca54e54578 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -186,11 +186,20 @@ int ntfs_nls_to_utf16(struct ntfs_sb_info *sbi, const u8 *name, u32 name_len,
 {
 	int ret, slen, i;
 	const u8 *end;
-	struct nls_table *nls = sbi->options->nls;
+	struct ntfs_mount_options *opts;
+	struct nls_table *nls;
+	bool ads;
 	u16 *uname = uni->name;
 
 	static_assert(sizeof(wchar_t) == sizeof(u16));
 
+	/* ntfs_fs_reconfigure() may replace the options, but not ->nls */
+	rcu_read_lock();
+	opts = READ_ONCE(sbi->options);
+	nls = opts->nls;
+	ads = opts->ads;
+	rcu_read_unlock();
+
 	if (!nls) {
 		/* utf8 -> utf16 */
 		ret = _utf8s_to_utf16s(name, name_len, endian, uname, max_ulen);
@@ -230,7 +239,7 @@ int ntfs_nls_to_utf16(struct ntfs_sb_info *sbi, const u8 *name, u32 name_len,
 
 	uni->len = ret;
 	uni->ads_len = 0;
-	if (ret > 0 && sbi->options->ads) {
+	if (ret > 0 && ads) {
 		uname = uni->name;
 		/* Find delimiter in range [1 : ret-2). */
 		for (i = 1; i + 1 < ret; i++) {
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index bbcd2e02bf63..44d59b444648 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -91,6 +91,8 @@ enum utf16_endian;
 struct ntfs_mount_options {
 	char *nls_name;
 	struct nls_table *nls;
+	/* ->d_hash() and ->d_compare() read the options in rcu pathwalk */
+	struct rcu_head rcu;
 
 	kuid_t fs_uid;
 	kgid_t fs_gid;
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 841f9068d82f..58ef6be3286b 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -251,6 +251,11 @@ static inline void put_mount_options(struct ntfs_mount_options *options)
 	kfree(options);
 }
 
+static void put_mount_options_rcu(struct rcu_head *head)
+{
+	put_mount_options(container_of(head, struct ntfs_mount_options, rcu));
+}
+
 enum Opt {
 	Opt_uid,
 	Opt_gid,
@@ -444,6 +449,7 @@ static int ntfs_fs_reconfigure(struct fs_context *fc)
 	struct super_block *sb = fc->root->d_sb;
 	struct ntfs_sb_info *sbi = sb->s_fs_info;
 	struct ntfs_mount_options *new_opts = fc->fs_private;
+	struct ntfs_mount_options *old_opts;
 	int ro_rw;
 
 	ro_rw = sb_rdonly(sb) && !(fc->sb_flags & SB_RDONLY);
@@ -473,7 +479,15 @@ static int ntfs_fs_reconfigure(struct fs_context *fc)
 	}
 
 	sync_filesystem(sb);
-	swap(sbi->options, fc->fs_private);
+	old_opts = sbi->options;
+	/* pairs with the READ_ONCE() in ntfs_nls_to_utf16() */
+	smp_store_release(&sbi->options, new_opts);
+	fc->fs_private = NULL;
+	/*
+	 * ntfs_d_hash() and ntfs_d_compare() of a pathwalk in rcu mode may
+	 * still read the old options through the sbi.
+	 */
+	call_rcu(&old_opts->rcu, put_mount_options_rcu);
 
 	return 0;
 }

-- 
2.53.0

Re: [PATCH 8/8] ntfs3: free replaced mount options after a grace period

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ntfs3: use-after-free of sbi->options during concurrent remount
- [Medium] vfs: ntfs3 silent mount option loss on remount
--

commit c093269bafa15fa705cf9dfc3164cc884b583d68
Author: Christian Brauner <brauner@kernel.org>
ntfs3: free replaced mount options after a grace period

This commit introduces an RCU grace period before freeing replaced mount
options in ntfs_fs_reconfigure() to resolve a use-after-free issue in
ntfs_nls_to_utf16() during pathwalks. It also introduces struct rcu_head
into struct ntfs_mount_options and updates readers like
ntfs_nls_to_utf16() to use RCU synchronization.

> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index 841f9068d82f9..58ef6be3286b1 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -473,7 +479,15 @@ static int ntfs_fs_reconfigure(struct fs_context *fc)
>  	}
>  
>  	sync_filesystem(sb);
> -	swap(sbi->options, fc->fs_private);
> +	old_opts = sbi->options;
> +	/* pairs with the READ_ONCE() in ntfs_nls_to_utf16() */
> +	smp_store_release(&sbi->options, new_opts);
> +	fc->fs_private = NULL;
> +	/*
> +	 * ntfs_d_hash() and ntfs_d_compare() of a pathwalk in rcu mode may
> +	 * still read the old options through the sbi.
> +	 */
> +	call_rcu(&old_opts->rcu, put_mount_options_rcu);

[Severity: High]
Does this change leave other readers of sbi->options vulnerable to a
use-after-free?

While this patch protects ntfs_nls_to_utf16(), other VFS operations appear
to access sbi->options without holding rcu_read_lock() or s_umount. If a
thread is preempted immediately after loading the sbi->options pointer, and
a concurrent mount -o remount calls ntfs_fs_reconfigure() which replaces
sbi->options and schedules its destruction via call_rcu(), the grace period
can complete before the first thread resumes.

For example, during inode initialization:

fs/ntfs3/inode.c:ntfs_read_mft() {
    ...
    inode->i_uid = sbi->options->fs_uid;
    inode->i_gid = sbi->options->fs_gid;
    ...
}

Or during getattr operations:

fs/ntfs3/file.c:ntfs_fileattr_get() {
    ...
    if (sbi->options->nocase)
        flags |= FS_CASEFOLD_FL;
    ...
}

Or during directory traversal:

fs/ntfs3/dir.c:ntfs_dir_emit() {
    ...
    if (!sbi->options->showmeta && ntfs_is_meta_file(sbi, ino))
    ...
}

Will these unprotected accesses cause memory corruption when the options
are freed by the concurrent remount?

[Severity: Medium]
This problem wasn't introduced by this patch, but doesn't replacing
sbi->options with new_opts cause a silent loss of mount options during
remount?

When a user executes a remount without providing the full set of original
mount options, fc->fs_private is populated with a fresh, default-initialized
options struct in ntfs_init_fs_context():

fs/ntfs3/super.c:ntfs_init_fs_context() {
    ...
    if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
        goto ok;
    ...
}

Then in ntfs_fs_reconfigure(), the old options are entirely overwritten by
the new defaults, discarding any unspecified previous settings like uid, gid,
or umask:

    smp_store_release(&sbi->options, new_opts);

Can this lead to unintended permission changes or feature toggles by
resetting unspecified options to their default values?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-work-fixes-rcu-v1-0-7446d2d335e6@kernel.org?part=8

Re: [PATCH 8/8] ntfs3: free replaced mount options after a grace period

[Christian Brauner]

On Fri, Oct 09, 2026 at 10:02:47PM +0000, sashiko-bot@kernel.org wrote:
> Does this change leave other readers of sbi->options vulnerable to a
> use-after-free?
>
> While this patch protects ntfs_nls_to_utf16(), other VFS operations appear
> to access sbi->options without holding rcu_read_lock() or s_umount. If a
> thread is preempted immediately after loading the sbi->options pointer, and
> a concurrent mount -o remount calls ntfs_fs_reconfigure() which replaces
> sbi->options and schedules its destruction via call_rcu(), the grace period
> can complete before the first thread resumes.

This is pre-existing. Before this patch ntfs_fs_reconfigure() swapped
the options and put_fs_context() freed the old ones right when the
remount returned. So all of these readers could already see freed
options and now they at least have to stay preempted for a whole grace
period.

Making every sbi->options reader safe against a concurrent remount is a
separate ntfs3 problem. This patch only cares about ntfs_nls_to_utf16()
because that's what rcu pathwalk calls.

> This problem wasn't introduced by this patch, but doesn't replacing
> sbi->options with new_opts cause a silent loss of mount options during
> remount?

That's how ntfs3 remount has worked since it switched to the new mount
api. Unrelated to this series.

Re: [PATCH 0/8] fs: fix a range of UAFs during rcu pathwalk caused by non-rcu frees

[Jan Kara]

On Fri 09-10-26 23:51:50, Christian Brauner wrote:
> While working on eliding the grace period in namespace_unlock() I went
> through what filesystems free when they're shut down and what a pathwalk
> in rcu mode can still read at that point. Since 66a97c2ec953's rcu
> pathwalk fixes that's the filesystem's job. Whatever ->permission(),
> ->d_revalidate(), ->get_link(), ->d_hash() and ->d_compare() look at in
> rcu mode has to be freed after a grace period. A few filesystems don't
> do that.
> 
> Most of it is hidden by umount(2). namespace_unlock() waits for a grace
> period before it drops the mounts, so a walker that was inside the
> filesystem is gone by the time the filesystem is shut down.
> 
> But the last reference to a mount doesn't have to go away in umount(2).
> A chdir() out of a lazily unmounted directory or the close() of the last
> file on it drops it without a grace period. A walker can then still be
> in the filesystem while ->put_super() and ->kill_sb() free what it
> reads.

Good catches! But I find it very counter-intuitive that pathname lookups
can still be running against a filesystem while its ->put_super is called.
People will keep forgetting about that and as a result these UAF bugs will
be introduced again. Isn't it feasible we'd guarantee to the filesystem
->put_super() gets called only after all lookups are done (i.e., after RCU
grace period expires)?

I understand this is a hairy bussiness because this has userspace visible
implications (like when filesystem's sysfs files disappear, when the bdev
really gets unused, etc.) but keeping all filesystems safe to run RCU
lookup after ->put_super() will be IMHO a headache.

								Honza


> 
> With KASAN on vfs.fixes:
> 
> - ext4_get_link() reads sb->s_fs_info and the extent status counters in
>   rcu mode for a symlink that keeps its target in a data block.
> 
>   ext4_put_super() destroys the counters, clears s_fs_info and frees the
>   sbi right away. That's a NULL dereference, a use-after-free of the sbi
>   or a fault on a destroyed percpu counter, depending on where the
>   walker sits.
> 
> - The unicode map of casefolded ext4, f2fs and tmpfs is freed from
>   ->put_super() while generic_ci_d_hash() and generic_ci_d_compare() use
>   it.
> 
> - ntfs3 with nocase frees its sbi and mount options under ntfs_d_hash()
>   and ntfs_d_compare(). A remount frees the old mount options right away
>   too, that one needs no unmount at all.
> 
> - eventfs frees an eventfs_inode after an SRCU grace period only, but
>   tracefs_d_revalidate() reads it under rcu_read_lock(). No unmount
>   needed either.
> 
> The ipc patch is a different kind of bug. create_ipc_ns() leaks the
> mqueue mount and its superblock when it fails after mq_init_ns(). I ran
> into it while looking at the kern_unmount() users.
> 
> afs and 9p look just as broken but nothing can reach it today. Both
> have a private bdi, and generic_shutdown_super() calls bdi_unregister(),
> which waits for a grace period before ->kill_sb() frees their private
> data. That grace period is there for the rcu readers of bdi_list and
> covers afs and 9p by accident. I don't want them to depend on that, so
> they free their private data after a grace period of their own as well.
> 
> I appended the reproducers. The race reproducers hold a pathwalk inside
> the filesystem's callback for two seconds with a kprobe while the other
> side frees what it reads. For the ntfs3 remount the window is a single
> load and its dereference, so that one needs an mdelay() in the kernel.
> ext4 with a journal needs a kernel without lockdep since
> jbd2_journal_destroy() calls lockdep_unregister_key(), which waits for a
> grace period too.
> 
> Clean this all up.
> 
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
> Christian Brauner (8):
>       ext4: free the sbi after a grace period
>       afs: free the super info after a grace period
>       9p: free the session after a grace period
>       eventfs: free the eventfs_inode after an rcu grace period too
>       ipc: drop the mqueue mount with kern_unmount() on setup failure
>       unicode: free the map after a grace period
>       ntfs3: free the sbi after a grace period
>       ntfs3: free replaced mount options after a grace period
> 
>  fs/9p/v9fs.h             |  1 +
>  fs/9p/vfs_super.c        |  4 ++--
>  fs/afs/internal.h        |  2 ++
>  fs/afs/super.c           |  2 +-
>  fs/ext4/ext4.h           |  2 ++
>  fs/ext4/extents-test.c   |  2 ++
>  fs/ext4/extents.c        |  1 +
>  fs/ext4/extents_status.c |  7 ++++++-
>  fs/ext4/extents_status.h |  1 +
>  fs/ext4/super.c          | 13 +++++++++++--
>  fs/ntfs3/dir.c           | 13 +++++++++++--
>  fs/ntfs3/ntfs_fs.h       |  3 +++
>  fs/ntfs3/super.c         | 49 +++++++++++++++++++++++++++++++++++-------------
>  fs/tracefs/event_inode.c | 10 +++++++++-
>  fs/unicode/utf8-core.c   | 21 +++++++++++++++++----
>  include/linux/unicode.h  |  1 +
>  ipc/namespace.c          |  2 +-
>  17 files changed, 107 insertions(+), 27 deletions(-)
> ---
> base-commit: 3d399224425573875b6f6f1181bd8cf9a28cc7d1
> change-id: 20261009-work-fixes-rcu-cd9c1eec0769
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/8] fs: fix a range of UAFs during rcu pathwalk caused by non-rcu frees

[Christian Brauner]

On Mon, Oct 12, 2026 at 12:36:15PM +0200, Jan Kara wrote:
> On Fri 09-10-26 23:51:50, Christian Brauner wrote:
> > While working on eliding the grace period in namespace_unlock() I went
> > through what filesystems free when they're shut down and what a pathwalk
> > in rcu mode can still read at that point. Since 66a97c2ec953's rcu
> > pathwalk fixes that's the filesystem's job. Whatever ->permission(),
> > ->d_revalidate(), ->get_link(), ->d_hash() and ->d_compare() look at in
> > rcu mode has to be freed after a grace period. A few filesystems don't
> > do that.
> > 
> > Most of it is hidden by umount(2). namespace_unlock() waits for a grace
> > period before it drops the mounts, so a walker that was inside the
> > filesystem is gone by the time the filesystem is shut down.
> > 
> > But the last reference to a mount doesn't have to go away in umount(2).
> > A chdir() out of a lazily unmounted directory or the close() of the last
> > file on it drops it without a grace period. A walker can then still be
> > in the filesystem while ->put_super() and ->kill_sb() free what it
> > reads.
> 
> Good catches! But I find it very counter-intuitive that pathname lookups
> can still be running against a filesystem while its ->put_super is called.
> People will keep forgetting about that and as a result these UAF bugs will
> be introduced again. Isn't it feasible we'd guarantee to the filesystem
> ->put_super() gets called only after all lookups are done (i.e., after RCU
> grace period expires)?
> 
> I understand this is a hairy bussiness because this has userspace visible
> implications (like when filesystem's sysfs files disappear, when the bdev
> really gets unused, etc.) but keeping all filesystems safe to run RCU
> lookup after ->put_super() will be IMHO a headache.

I don't think delaying put_super() or freeing the superblock helps.
That's not where the issue is. The issue is with sb->s_fs_info being
available after a grace period and that can go away in different
locations ->put_super() or ->kill_sb(). Bringing put_super() into the
argument was maybe not my best idea. We also need to backport this
stuff.

Re: [PATCH 0/8] fs: fix a range of UAFs during rcu pathwalk caused by non-rcu frees

[Jan Kara]

On Mon 12-10-26 12:42:06, Christian Brauner wrote:
> On Mon, Oct 12, 2026 at 12:36:15PM +0200, Jan Kara wrote:
> > On Fri 09-10-26 23:51:50, Christian Brauner wrote:
> > > While working on eliding the grace period in namespace_unlock() I went
> > > through what filesystems free when they're shut down and what a pathwalk
> > > in rcu mode can still read at that point. Since 66a97c2ec953's rcu
> > > pathwalk fixes that's the filesystem's job. Whatever ->permission(),
> > > ->d_revalidate(), ->get_link(), ->d_hash() and ->d_compare() look at in
> > > rcu mode has to be freed after a grace period. A few filesystems don't
> > > do that.
> > > 
> > > Most of it is hidden by umount(2). namespace_unlock() waits for a grace
> > > period before it drops the mounts, so a walker that was inside the
> > > filesystem is gone by the time the filesystem is shut down.
> > > 
> > > But the last reference to a mount doesn't have to go away in umount(2).
> > > A chdir() out of a lazily unmounted directory or the close() of the last
> > > file on it drops it without a grace period. A walker can then still be
> > > in the filesystem while ->put_super() and ->kill_sb() free what it
> > > reads.
> > 
> > Good catches! But I find it very counter-intuitive that pathname lookups
> > can still be running against a filesystem while its ->put_super is called.
> > People will keep forgetting about that and as a result these UAF bugs will
> > be introduced again. Isn't it feasible we'd guarantee to the filesystem
> > ->put_super() gets called only after all lookups are done (i.e., after RCU
> > grace period expires)?
> > 
> > I understand this is a hairy bussiness because this has userspace visible
> > implications (like when filesystem's sysfs files disappear, when the bdev
> > really gets unused, etc.) but keeping all filesystems safe to run RCU
> > lookup after ->put_super() will be IMHO a headache.
> 
> I don't think delaying put_super() or freeing the superblock helps.
> That's not where the issue is. The issue is with sb->s_fs_info being
> available after a grace period and that can go away in different
> locations ->put_super() or ->kill_sb().

So my thinking was: Cannot we separate the teardown of mounts and the
teardown of the sb by an RCU grace period? Then by the time the filesystem
gets to any freeing of filesystem related structures all RCU lookups are
guaranteed to be done so there's no problem with them accessing possibly
freed structures pointed to by the sb.

But then I've read more of the changelogs and AFAIU getting rid of the RCU
grace period waiting in the unmount path is actually your point to speedup
container teardown. I still think in-progress RCU lookups on a sb being
unmounted will be a source of bugs in the future (dependencies during fs
unmount are complex as is and adding RCU lookups to the game adds even more
complexity). Anyway, if we're going to do that then eventually I think
we'll need to add something like lockdep context tracking to verify each
structure accessed from RCU lookup is properly RCU-freed but that's for
later.

> Bringing put_super() into the argument was maybe not my best idea. We
> also need to backport this stuff.

OK, let's go with these fixes for now.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR