* [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®