Re: [PATCH v2] btrfs: unwind device add when sysfs registration fails

[Daniel Vacek]

On Wed, 7 Oct 2026 at 18:04, Adarsh Das <adarshdas950@xxxxxxxxx> wrote:
> btrfs_init_new_device() ignored the return value of
> btrfs_sysfs_add_device(). When sysfs link creation failed (e.g. -EEXIST),
> the add path continued, aborted the transaction with the same errno, and
> tripped WARN_ON(btrfs_abort_should_print_stack()) while leaving the device
> half-registered in memory.
>
> Check the return value and unwind when registration fails, without
> aborting the filesystem for errnos like -EEXIST. If add failed before
> anything was published in sysfs, tear down the in-memory device state only
> and do not call btrfs_sysfs_remove_device(). When the block-device link
> was created but devid kobject registration fails, drop the link inside
> btrfs_sysfs_add_device() before returning the error.
>
> Reported-by: syzbot+3bf3e110b2d406b8166c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/all/6a791c94.01d0871a.3a0d52.0099.GAE@xxxxxxxxxx
> Tested-by: syzbot+3bf3e110b2d406b8166c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Adarsh Das <adarshdas950@xxxxxxxxx>
> ---
> v2:
>  - Unwind without sysfs remove when add failed (review on v1)
>  - Roll back block-device link if devid kobject add fails
> v1: https://lore.kernel.org/all/20261003111951.24527-1-adarshdas950@xxxxxxxxx/
> ---
>  fs/btrfs/sysfs.c   | 3 +++
>  fs/btrfs/volumes.c | 7 ++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 39cb01ee441a..2f034689e71b 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -2165,6 +2165,9 @@ int btrfs_sysfs_add_device(struct btrfs_device *device)
>         ret = kobject_init_and_add(&device->devid_kobj, &devid_ktype,
>                                    devinfo_kobj, "%llu", device->devid);
>         if (ret) {
> +               if (device->bdev)
> +                       sysfs_remove_link(devices_kobj,
> +                                         bdev_kobj(device->bdev)->name);
>                 kobject_put(&device->devid_kobj);
>                 btrfs_warn(device->fs_info,
>                            "devinfo init for devid %llu failed: %d",
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 85ea9c5d4536..56b38f012093 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3066,7 +3066,11 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>         mutex_unlock(&fs_info->chunk_mutex);
>
>         /* Add sysfs device entry */
> -       btrfs_sysfs_add_device(device);
> +       ret = btrfs_sysfs_add_device(device);
> +       if (ret) {
> +               mutex_unlock(&fs_devices->device_list_mutex);
> +               goto error_unwind_device;
> +       }
>
>         mutex_unlock(&fs_devices->device_list_mutex);
>
> @@ -3147,6 +3151,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>
>  error_sysfs:
>         btrfs_sysfs_remove_device(device);
> +error_unwind_device:
>         mutex_lock(&fs_info->fs_devices->device_list_mutex);
>         if (seeding_dev)
>                 btrfs_assign_next_active_device(device, seed_devices->latest_dev);

I'm wondering why not this?:

@@ -3066,7 +3066,9 @@ int btrfs_init_new_device(struct btrfs_fs_info
*fs_info, const char *device_path
        mutex_unlock(&fs_info->chunk_mutex);

        /* Add sysfs device entry */
-       btrfs_sysfs_add_device(device);
+       ret = btrfs_sysfs_add_device(device);
+       if (ret)
+               goto error_device;

        mutex_unlock(&fs_devices->device_list_mutex);

@@ -3148,6 +3150,7 @@ int btrfs_init_new_device(struct btrfs_fs_info
*fs_info, const char *device_path
 error_sysfs:
        btrfs_sysfs_remove_device(device);
        mutex_lock(&fs_info->fs_devices->device_list_mutex);
+error_device:
        mutex_lock(&fs_info->chunk_mutex);
        list_del_rcu(&device->dev_list);
        list_del(&device->dev_alloc_list);

--nX