Re: [PATCH] btrfs: drain sysfs callbacks before stopping transaction kthread

[Qu Wenruo]

在 2026/8/20 21:57, Jiacheng Xu 写道:
> btrfs_label_store() and btrfs_feature_attr_store() wake up the
> transaction kthread through fs_info->transaction_kthread.
>
> During filesystem teardown, close_ctree() stops the transaction kthread
> before removing the mounted filesystem's sysfs attributes. A concurrent
> sysfs write can therefore enter one of these callbacks after the kthread
> has been stopped and pass an invalid task pointer to wake_up_process().
>
> This results in a concurrent null-pointer dereference in
> try_to_wake_up(). The scheduler is not the root cause; the invalid
> transaction kthread pointer is used by a Btrfs sysfs callback during
> teardown.
>
> Split mounted sysfs cleanup into two stages. Remove attributes which may
> have store callbacks before stopping the transaction kthread. The
> remaining sysfs kobjects are removed at the original teardown point,
> after the kthread has been stopped.

Why not just simpliy reject sysfs write operations when the fs has 
CLOSING_START or without FS_OPEN flags?

> Apply the same ordering to the open_ctree() failure path when the
> transaction kthread has already been created.
>
> Tested-by: Jiacheng Xu <stitch@xxxxxxxxxx>
> Signed-off-by: Jiacheng Xu <stitch@xxxxxxxxxx>
> ---
> fs/btrfs/disk-io.c | 16 ++++++++++++++--
> fs/btrfs/sysfs.c   | 26 +++++++++++++++++++++-----
> fs/btrfs/sysfs.h   |  3 +++
> 3 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 2f1666d9544e..4f5bcc576dc6 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3363,6 +3363,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>        struct btrfs_root *tree_root;
>        struct btrfs_root *chunk_root;
>        struct btrfs_root *remap_root;
> +     bool sysfs_attrs_removed = false;
>        int ret;
>        int level;
>
> @@ -3780,6 +3781,9 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
> fail_qgroup:
>        btrfs_free_qgroup_config(fs_info);
> fail_trans_kthread:
> +     btrfs_sysfs_remove_mounted_attrs(fs_info);
> +     sysfs_attrs_removed = true;
> +
>        kthread_stop(fs_info->transaction_kthread);
>        btrfs_cleanup_transaction(fs_info);
>        btrfs_free_fs_roots(fs_info);
> @@ -3793,7 +3797,9 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>        filemap_write_and_wait(fs_info->btree_inode->i_mapping);
>
> fail_sysfs:
> -     btrfs_sysfs_remove_mounted(fs_info);
> +     if (!sysfs_attrs_removed)
> +             btrfs_sysfs_remove_mounted_attrs(fs_info);
> +     btrfs_sysfs_remove_mounted_kobjects(fs_info);
>
> fail_fsdev_sysfs:
>        btrfs_sysfs_remove_fsid(fs_info->fs_devices);
> @@ -4318,6 +4324,9 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>
>        set_bit(BTRFS_FS_CLOSING_START, &fs_info->flags);
>
> +     /* Drain sysfs callbacks before stopping the transaction kthread. */
> +     btrfs_sysfs_remove_mounted_attrs(fs_info);
> +
>        /*
>        * If we had UNFINISHED_DROPS we could still be processing them, so
>        * clear that bit and wake up relocation so it can stop.
> @@ -4538,7 +4547,7 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
>                          percpu_counter_sum(&fs_info->ordered_bytes));
>
> -     btrfs_sysfs_remove_mounted(fs_info);
> +     btrfs_sysfs_remove_mounted_kobjects(fs_info);
>        btrfs_sysfs_remove_fsid(fs_info->fs_devices);
>
>        btrfs_put_block_group_cache(fs_info);
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 0d14570c8bc2..d90d76a152e9 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -1707,11 +1707,23 @@ static void btrfs_sysfs_remove_fs_devices(struct btrfs_fs_devices *fs_devices)
>        }
> }
>
> -void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
> +/*
> + * Remove attributes which may have store callbacks. kernfs waits for active
> + * callbacks during removal, so this must be done before stopping any kthread
> + * which can be woken up by those callbacks.
> + */
> +void btrfs_sysfs_remove_mounted_attrs(struct btrfs_fs_info *fs_info)
> {
>        struct kobject *fsid_kobj = &fs_info->fs_devices->fsid_kobj;
>
> -     sysfs_remove_link(fsid_kobj, "bdi");
> +     addrm_unknown_feature_attrs(fs_info, false);
> +     sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> +     sysfs_remove_files(fsid_kobj, btrfs_attrs);
> +}
> +
> +static void btrfs_sysfs_remove_mounted_dirs(struct btrfs_fs_info *fs_info)
> +{
> +     sysfs_remove_link(&fs_info->fs_devices->fsid_kobj, "bdi");
>
>        if (fs_info->space_info_kobj) {
>              sysfs_remove_files(fs_info->space_info_kobj, allocation_attrs);
> @@ -1730,9 +1742,18 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
>              kobject_put(fs_info->debug_kobj);
>        }
> #endif
> -     addrm_unknown_feature_attrs(fs_info, false);
> -     sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> -     sysfs_remove_files(fsid_kobj, btrfs_attrs);
> +}
> +
> +void btrfs_sysfs_remove_mounted_kobjects(struct btrfs_fs_info *fs_info)
> +{
> +     btrfs_sysfs_remove_mounted_dirs(fs_info);
> +     btrfs_sysfs_remove_fs_devices(fs_info->fs_devices);
> +}
> +
> +void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
> +{
> +     btrfs_sysfs_remove_mounted_dirs(fs_info);
> +     btrfs_sysfs_remove_mounted_attrs(fs_info);
>        btrfs_sysfs_remove_fs_devices(fs_info->fs_devices);
> }
>
> diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
> index 05498e5346c3..0d008fc8f1b8 100644
> --- a/fs/btrfs/sysfs.h
> +++ b/fs/btrfs/sysfs.h
> @@ -35,6 +35,9 @@ void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
> int __init btrfs_init_sysfs(void);
> void __cold btrfs_exit_sysfs(void);
> int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info);
> +void btrfs_sysfs_remove_mounted_attrs(struct btrfs_fs_info *fs_info);
> +void btrfs_sysfs_remove_mounted_kobjects(struct btrfs_fs_info *fs_info);
> void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info);
> void btrfs_sysfs_add_block_group_type(struct btrfs_block_group *cache);
> int btrfs_sysfs_add_space_info_type(struct btrfs_space_info *space_info);