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