[PATCH v2] xfs: tear down zoned sysfs before freeing zone info

[PATCH v2] xfs: tear down zoned sysfs before freeing zone info

[Cen Zhang]

The zoned sysfs directory is currently registered as part of the generic
per-mount sysfs setup, but the data it exposes has a narrower lifetime.
mp->m_zone_info is allocated later by xfs_mount_zones() and freed by
xfs_unmount_zones(), while the zoned sysfs kobject remains registered
until xfs_mount_sysfs_del() runs near the end of xfs_unmountfs().

A read of nr_open_zones can therefore enter through the still-live sysfs
kobject after xfs_unmount_zones() has freed mp->m_zone_info, leading to a
use-after-free in nr_open_zones_show().

Make the zoned sysfs lifetime match the zone allocator lifetime.  Create
the zoned sysfs directory only after xfs_mount_zones() succeeds, and
remove it before xfs_unmount_zones() frees m_zone_info during unmount and
mount failure cleanup.

Validation reproduced this kernel report:
BUG: KASAN: slab-use-after-free in nr_open_zones_show+0x86/0x90
The buggy address belongs to the object at ffff88810b177800 which belongs
to the cache kmalloc-1k of size 1024
The buggy address is located 160 bytes inside of freed 1024-byte region
[ffff88810b177800, ffff88810b177c00)
Read of size 4
Call trace:
  print_report+0xcd/0x620
  nr_open_zones_show+0x86/0x90 (fs/xfs/xfs_sysfs.c:724)
  srso_alias_return_thunk+0x5/0xfbef5
  __virt_addr_valid+0x20c/0x410
  kasan_report+0xdd/0x110
  sysfs_kf_seq_show+0x1bd/0x380
  seq_read_iter+0x40f/0x11b0
  lock_release+0xba/0x260
  mark_held_locks+0x40/0x70
  vfs_read+0x717/0xce0
  __up_read+0x319/0x900
  ksys_read+0xf8/0x1c0
  do_user_addr_fault+0x3d0/0xbc0
  trace_hardirqs_on_prepare+0x23/0xf0
  do_syscall_64+0xc8/0x530 (arch/x86/entry/syscall_64.c:87)
  entry_SYSCALL_64_after_hwframe+0x74/0x7c
Allocated by task stack:
  kasan_save_stack+0x33/0x60
  kasan_save_track+0x14/0x30
  __kasan_kmalloc+0xaa/0xb0
  __kmalloc_cache_noprof+0x205/0x460
  xfs_mount_zones+0x34c/0x2650
  xfs_mountfs+0x1b97/0x1eb0
  xfs_fs_fill_super+0xf2b/0x18a0
  get_tree_bdev_flags+0x310/0x590
  vfs_get_tree+0x8d/0x2e0
  __x64_sys_fsconfig+0x61c/0xbc0
  do_syscall_64+0xc8/0x530 (arch/x86/entry/syscall_64.c:87)
  entry_SYSCALL_64_after_hwframe+0x74/0x7c
Freed by task stack:
  kasan_save_stack+0x33/0x60
  kasan_save_track+0x14/0x30
  kasan_save_free_info+0x3b/0x60
  __kasan_slab_free+0x5f/0x80
  kfree+0x20e/0x4c0
  xfs_unmountfs+0x2fd/0x390
  xfs_fs_put_super+0x60/0x110
  generic_shutdown_super+0x143/0x4b0
  kill_block_super+0x3b/0x90
  xfs_kill_sb+0x12/0x50
  deactivate_locked_super+0xa7/0x160
  cleanup_mnt+0x218/0x420
  task_work_run+0x11a/0x1f0
  exit_to_user_mode_loop+0x13c/0x4f0
  do_syscall_64+0x4a9/0x530 (arch/x86/entry/syscall_64.c:87)
  entry_SYSCALL_64_after_hwframe+0x74/0x7c

Fixes: 62c89988dc19 ("xfs: expose the number of open zones in sysfs")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
v2:
Tear down the zoned sysfs directory before freeing m_zone_info instead of
serializing nr_open_zones_show() with s_umount.

 fs/xfs/xfs_mount.c | 10 +++++++++-
 fs/xfs/xfs_sysfs.c | 28 +++++++++++++++++-----------
 fs/xfs/xfs_sysfs.h |  2 ++
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index be90c7b03994..2dcb10bc6187 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1178,6 +1178,9 @@ xfs_mountfs(
 		error = xfs_mount_zones(mp);
 		if (error)
 			goto out_rtunmount;
+		error = xfs_zoned_sysfs_init(mp);
+		if (error)
+			goto out_unmount_zones;
 	}
 
 	/*
@@ -1233,6 +1236,9 @@ xfs_mountfs(
  out_agresv:
 	xfs_fs_unreserve_ag_blocks(mp);
 	xfs_qm_unmount_quotas(mp);
+	if (xfs_has_zoned(mp))
+		xfs_zoned_sysfs_del(mp);
+ out_unmount_zones:
 	if (xfs_has_zoned(mp))
 		xfs_unmount_zones(mp);
  out_rtunmount:
@@ -1323,8 +1329,10 @@ xfs_unmountfs(
 		xfs_zone_gc_stop(mp);
 	xfs_fs_unreserve_ag_blocks(mp);
 	xfs_qm_unmount_quotas(mp);
-	if (xfs_has_zoned(mp))
+	if (xfs_has_zoned(mp)) {
+		xfs_zoned_sysfs_del(mp);
 		xfs_unmount_zones(mp);
+	}
 	xfs_rtunmount_inodes(mp);
 	xfs_irele(mp->m_rootip);
 	if (mp->m_metadirip)
diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 676777064c2d..b62712187324 100644
--- a/fs/xfs/xfs_sysfs.c
+++ b/fs/xfs/xfs_sysfs.c
@@ -780,6 +780,23 @@ static const struct kobj_type xfs_zoned_ktype = {
 	.default_groups = xfs_zoned_groups,
 };
 
+int
+xfs_zoned_sysfs_init(struct xfs_mount *mp)
+{
+	if (!IS_ENABLED(CONFIG_XFS_RT) || !xfs_has_zoned(mp))
+		return 0;
+
+	return xfs_sysfs_init(&mp->m_zoned_kobj, &xfs_zoned_ktype,
+			&mp->m_kobj, "zoned");
+}
+
+void
+xfs_zoned_sysfs_del(struct xfs_mount *mp)
+{
+	if (IS_ENABLED(CONFIG_XFS_RT) && xfs_has_zoned(mp))
+		xfs_sysfs_del(&mp->m_zoned_kobj);
+}
+
 int
 xfs_mount_sysfs_init(
 	struct xfs_mount	*mp)
@@ -820,14 +837,6 @@ xfs_mount_sysfs_init(
 	if (error)
 		goto out_remove_error_dir;
 
-	if (IS_ENABLED(CONFIG_XFS_RT) && xfs_has_zoned(mp)) {
-		/* .../xfs/<dev>/zoned/ */
-		error = xfs_sysfs_init(&mp->m_zoned_kobj, &xfs_zoned_ktype,
-					&mp->m_kobj, "zoned");
-		if (error)
-			goto out_remove_error_dir;
-	}
-
 	return 0;
 
 out_remove_error_dir:
@@ -846,9 +855,6 @@ xfs_mount_sysfs_del(
 	struct xfs_error_cfg	*cfg;
 	int			i, j;
 
-	if (IS_ENABLED(CONFIG_XFS_RT) && xfs_has_zoned(mp))
-		xfs_sysfs_del(&mp->m_zoned_kobj);
-
 	for (i = 0; i < XFS_ERR_CLASS_MAX; i++) {
 		for (j = 0; j < XFS_ERR_ERRNO_MAX; j++) {
 			cfg = &mp->m_error_cfg[i][j];
diff --git a/fs/xfs/xfs_sysfs.h b/fs/xfs/xfs_sysfs.h
index 1622fe80ad3e..25e5f8fae2f3 100644
--- a/fs/xfs/xfs_sysfs.h
+++ b/fs/xfs/xfs_sysfs.h
@@ -53,6 +53,8 @@ xfs_sysfs_del(
 }
 
 int	xfs_mount_sysfs_init(struct xfs_mount *mp);
+int	xfs_zoned_sysfs_init(struct xfs_mount *mp);
+void	xfs_zoned_sysfs_del(struct xfs_mount *mp);
 void	xfs_mount_sysfs_del(struct xfs_mount *mp);
 
 #endif	/* __XFS_SYSFS_H__ */
-- 
2.43.0

Re: [PATCH v2] xfs: tear down zoned sysfs before freeing zone info

[Damien Le Moal]

On 7/5/26 11:41, Cen Zhang wrote:
> The zoned sysfs directory is currently registered as part of the generic
> per-mount sysfs setup, but the data it exposes has a narrower lifetime.
> mp->m_zone_info is allocated later by xfs_mount_zones() and freed by
> xfs_unmount_zones(), while the zoned sysfs kobject remains registered
> until xfs_mount_sysfs_del() runs near the end of xfs_unmountfs().
> 
> A read of nr_open_zones can therefore enter through the still-live sysfs
> kobject after xfs_unmount_zones() has freed mp->m_zone_info, leading to a
> use-after-free in nr_open_zones_show().
> 
> Make the zoned sysfs lifetime match the zone allocator lifetime.  Create
> the zoned sysfs directory only after xfs_mount_zones() succeeds, and
> remove it before xfs_unmount_zones() frees m_zone_info during unmount and
> mount failure cleanup.

[...]

> Tear down the zoned sysfs directory before freeing m_zone_info instead of
> serializing nr_open_zones_show() with s_umount.

Hmmm... What if the user already has entered the show function but gets
preempted before referencing the zone info and umount proceeds ? That problem
remains, no ?

> 
>  fs/xfs/xfs_mount.c | 10 +++++++++-
>  fs/xfs/xfs_sysfs.c | 28 +++++++++++++++++-----------
>  fs/xfs/xfs_sysfs.h |  2 ++
>  3 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index be90c7b03994..2dcb10bc6187 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -1178,6 +1178,9 @@ xfs_mountfs(
>  		error = xfs_mount_zones(mp);
>  		if (error)
>  			goto out_rtunmount;
> +		error = xfs_zoned_sysfs_init(mp);
> +		if (error)
> +			goto out_unmount_zones;

Why not call this in xfs_mount_zones() ?

>  	}
>  
>  	/*
> @@ -1233,6 +1236,9 @@ xfs_mountfs(
>   out_agresv:
>  	xfs_fs_unreserve_ag_blocks(mp);
>  	xfs_qm_unmount_quotas(mp);
> +	if (xfs_has_zoned(mp))
> +		xfs_zoned_sysfs_del(mp);

And this as the first thing in xfs_unmount_zones() ?

Another simpler approach could be to simply call xfs_mount_sysfs_del() earlier
in xfs_unmountfs(). Though that may have other implications and I have not
checked details.

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] xfs: tear down zoned sysfs before freeing zone info

[Cen Zhang]

Hi Damien,

Thanks for your review and suggestions.

> Hmmm... What if the user already has entered the show function but gets
> preempted before referencing the zone info and umount proceeds ? That problem
> remains, no ?

I double checked the sysfs/kernfs paths, and I do not think that specific
case remains with the current ordering.  sysfs reads go through the kernfs
seq_file path: kernfs_seq_start() takes an active reference before
sysfs_kf_seq_show() calls the attribute's ->show method, and
kernfs_seq_stop() drops it afterwards.  On removal, kobject_del() reaches
sysfs_remove_dir(), which calls kernfs_remove(); kernfs_remove() deactivates
the subtree and kernfs_drain() waits for active references to drain.

So if nr_open_zones_show() has already been entered and then gets preempted,
xfs_zoned_sysfs_del() should wait for that active callback to finish before
returning.  Only after that would xfs_unmount_zones() free m_zone_info.

> Why not call this in xfs_mount_zones() ?

Yes, that is cleaner.  The zoned sysfs files expose zone allocator state, so
putting xfs_zoned_sysfs_init() in xfs_mount_zones() makes the ownership and
lifetime boundary more obvious.  I will move it there for v3.

> And this as the first thing in xfs_unmount_zones() ?

Agreed.  I will make xfs_unmount_zones() remove the zoned sysfs directory
before zone GC teardown and before xfs_free_zone_info().  That keeps all
zone-info users behind the same allocator teardown entry point.

One detail is that if xfs_zoned_sysfs_init() is called after
xfs_zone_gc_mount() and then fails, the mount error path in xfs_mount_zones()
must unwind the GC thread before freeing m_zone_info.  I will handle that in
the v3 error path.

> Another simpler approach could be to simply call xfs_mount_sysfs_del() earlier
> in xfs_unmountfs(). Though that may have other implications and I have not
> checked details.

I would rather avoid moving the whole xfs_mount_sysfs_del() earlier.  The
bug is specific to the zoned sysfs subtree because those files expose
m_zone_info, whereas the rest of the per-mount sysfs tree has different
state and lifetime.  Moving only the zoned subtree into
xfs_mount_zones()/xfs_unmount_zones() seems narrower and keeps unrelated
sysfs teardown ordering unchanged.

Best Regards,
Cen Zhang