* [PATCH] btrfs: unwind device add when sysfs registration fails
@ 2026-10-03 11:19 Adarsh Das
2026-10-07 13:58 ` Johannes Thumshirn
0 siblings, 1 reply; 6+ messages in thread
From: Adarsh Das @ 2026-10-03 11:19 UTC (permalink / raw)
To: linux-btrfs
Cc: dsterba, mason, linux-kernel, syzbot+3bf3e110b2d406b8166c, Adarsh Das
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.
Unwind through error_sysfs when sysfs registration fails, matching other
failure paths after the device lists are updated.
Reported-by: syzbot+3bf3e110b2d406b8166c@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a791c94.01d0871a.3a0d52.0099.GAE@google.com
Tested-by: syzbot+3bf3e110b2d406b8166c@syzkaller.appspotmail.com
Assisted-by: LLM
Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
---
fs/btrfs/volumes.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 85ea9c5d4536..1517ed3da22c 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_sysfs;
+ }
mutex_unlock(&fs_devices->device_list_mutex);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: unwind device add when sysfs registration fails
2026-10-03 11:19 [PATCH] btrfs: unwind device add when sysfs registration fails Adarsh Das
@ 2026-10-07 13:58 ` Johannes Thumshirn
2026-10-07 16:03 ` [PATCH v2] " Adarsh Das
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Thumshirn @ 2026-10-07 13:58 UTC (permalink / raw)
To: Adarsh Das, linux-btrfs
Cc: dsterba, mason, linux-kernel, syzbot+3bf3e110b2d406b8166c
On 10/3/26 1:19 PM, Adarsh Das 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.
>
> Unwind through error_sysfs when sysfs registration fails, matching other
> failure paths after the device lists are updated.
>
> Reported-by: syzbot+3bf3e110b2d406b8166c@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/6a791c94.01d0871a.3a0d52.0099.GAE@google.com
> Tested-by: syzbot+3bf3e110b2d406b8166c@syzkaller.appspotmail.com
> Assisted-by: LLM
> Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
> ---
> fs/btrfs/volumes.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 85ea9c5d4536..1517ed3da22c 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_sysfs;
> + }
>
> mutex_unlock(&fs_devices->device_list_mutex);
>
Are you sure this is safe? If btrfs_sysfs_add_device() fails i.e. at
sysfs_create_link() and you do 'goto error_sysfs;' which then calls
btrfs_sysfs_remove_device(). That one then calls sysfs_remove_link() for
a link that was never created. This is only working at the moment,
because sysfs_remove_link() doesn't check for errors of
kernfs_remove_by_name() (which in this case returns ENOENT).
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] btrfs: unwind device add when sysfs registration fails
2026-10-07 13:58 ` Johannes Thumshirn
@ 2026-10-07 16:03 ` Adarsh Das
2026-10-13 8:35 ` Johannes Thumshirn
2026-10-13 11:13 ` Daniel Vacek
0 siblings, 2 replies; 6+ messages in thread
From: Adarsh Das @ 2026-10-07 16:03 UTC (permalink / raw)
To: johannes.thumshirn
Cc: adarshdas950, dsterba, linux-btrfs, linux-kernel, mason,
syzbot+3bf3e110b2d406b8166c
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@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/6a791c94.01d0871a.3a0d52.0099.GAE@google.com
Tested-by: syzbot+3bf3e110b2d406b8166c@syzkaller.appspotmail.com
Assisted-by: LLM
Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
---
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@gmail.com/
---
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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: unwind device add when sysfs registration fails
2026-10-07 16:03 ` [PATCH v2] " Adarsh Das
@ 2026-10-13 8:35 ` Johannes Thumshirn
2026-10-13 11:13 ` Daniel Vacek
1 sibling, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2026-10-13 8:35 UTC (permalink / raw)
To: Adarsh Das
Cc: dsterba, linux-btrfs, linux-kernel, mason, syzbot+3bf3e110b2d406b8166c
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: unwind device add when sysfs registration fails
2026-10-07 16:03 ` [PATCH v2] " Adarsh Das
2026-10-13 8:35 ` Johannes Thumshirn
@ 2026-10-13 11:13 ` Daniel Vacek
2026-10-13 11:22 ` Johannes Thumshirn
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vacek @ 2026-10-13 11:13 UTC (permalink / raw)
To: Adarsh Das
Cc: johannes.thumshirn, dsterba, linux-btrfs, linux-kernel, mason,
syzbot+3bf3e110b2d406b8166c
On Wed, 7 Oct 2026 at 18:04, Adarsh Das <adarshdas950@gmail.com> 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@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/all/6a791c94.01d0871a.3a0d52.0099.GAE@google.com
> Tested-by: syzbot+3bf3e110b2d406b8166c@syzkaller.appspotmail.com
> Assisted-by: LLM
> Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
> ---
> 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@gmail.com/
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: unwind device add when sysfs registration fails
2026-10-13 11:13 ` Daniel Vacek
@ 2026-10-13 11:22 ` Johannes Thumshirn
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2026-10-13 11:22 UTC (permalink / raw)
To: Daniel Vacek, Adarsh Das
Cc: dsterba, linux-btrfs, linux-kernel, mason, syzbot+3bf3e110b2d406b8166c
On 10/13/26 1:13 PM, Daniel Vacek wrote:
> On Wed, 7 Oct 2026 at 18:04, Adarsh Das <adarshdas950@gmail.com> 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@syzkaller.appspotmail.com
>> Link: https://lore.kernel.org/all/6a791c94.01d0871a.3a0d52.0099.GAE@google.com
>> Tested-by: syzbot+3bf3e110b2d406b8166c@syzkaller.appspotmail.com
>> Assisted-by: LLM
>> Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
>> ---
>> 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@gmail.com/
>> ---
>> 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
This would remove the need for the lock of the device_list_mutex indeed.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-13 11:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-03 11:19 [PATCH] btrfs: unwind device add when sysfs registration fails Adarsh Das
2026-10-07 13:58 ` Johannes Thumshirn
2026-10-07 16:03 ` [PATCH v2] " Adarsh Das
2026-10-13 8:35 ` Johannes Thumshirn
2026-10-13 11:13 ` Daniel Vacek
2026-10-13 11:22 ` Johannes Thumshirn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®