btrfs: detach failed sprout device from transaction update list
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v2 1/3 btrfs: detach failed sprout device from transaction update list
When creating the first metadata chunk for a sprout filesystem, create_chunk() adds the new device to the transaction dev_update_list through device->post_commit_list.
If the subsequent system chunk creation fails, btrfs_init_new_device() aborts the transaction and releases the device while post_commit_list is still linked. This triggers a warning in btrfs_free_device() and leaves the transaction list referencing freed memory.
Detach the device while holding chunk_mutex before releasing it.
--- fs/btrfs/volumes.c | 2 ++ 1 file changed, 2 insertions(+)
fs/btrfs/volumes.c +2 -0
@@ -2988,6 +2988,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_pathbtrfs_sysfs_remove_device (device);mutex_lock (&fs_info->fs_devices->device_list_mutex);mutex_lock (&fs_info->chunk_mutex); +if (!list_empty (&device->post_commit_list)) +list_del_init (&device->post_commit_list);list_del_rcu (&device->dev_list);list_del (&device->dev_alloc_list); fs_info->fs_devices->num_devices--;
Signature · 1 line
2.53.0
PATCH v2 2/3 btrfs: restore active device pointers after failed sprout
In reply to a message not in the archivebtrfs_init_new_device() switches latest_dev and possibly s_bdev from the seed device to the new sprout device before creating the first writable chunks.
If chunk creation or the subsequent sprout setup fails, the error path releases the new device without switching those pointers back. btrfs_show_devname() can then dereference the freed latest_dev and crash.
Restore the active device pointers to the latest seed device before removing and releasing the failed sprout device.
--- fs/btrfs/volumes.c | 3 +++ 1 file changed, 3 insertions(+)
fs/btrfs/volumes.c +3 -0
@@ -2987,6 +2987,9 @@ 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); +if (seeding_dev) +btrfs_assign_next_active_device (device, + seed_devices->latest_dev);mutex_lock (&fs_info->chunk_mutex);if (!list_empty (&device->post_commit_list))list_del_init (&device->post_commit_list);
Signature · 1 line
2.53.0
PATCH v2 3/3 btrfs: roll back sprout setup after device add failure
In reply to a message not in the archivebtrfs_init_new_device() calls btrfs_setup_sprout() before creating the first writable chunks for a seed filesystem. That moves the seed devices out of fs_info->fs_devices, clears the seeding state and installs a new fsid for the sprout filesystem.
If a later step fails, the error path removes the new device but leaves fs_info->fs_devices in the partially initialized sprout state. The mounted filesystem can then be left with no open devices after the failed device add.
Add the inverse of btrfs_setup_sprout() and use it from the error path so the mounted seed filesystem is restored before the temporary seed_devices copy is released.
--- fs/btrfs/volumes.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
fs/btrfs/volumes.c +38 -0
@@ -2699,6 +2699,42 @@ static void btrfs_setup_sprout(struct btrfs_fs_info *fs_info,btrfs_set_super_flags (disk_super, super_flags); } +static void btrfs_rollback_sprout (struct btrfs_fs_info *fs_info, +struct btrfs_fs_devices *seed_devices) +{ +struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; +struct btrfs_super_block *disk_super = fs_info->super_copy; +struct btrfs_device *device; +u64 super_flags; + +lockdep_assert_held (&uuid_mutex); +lockdep_assert_held (&fs_devices->device_list_mutex); + +list_del_init (&seed_devices->seed_list); +list_splice_init_rcu (&seed_devices->devices, &fs_devices->devices, + synchronize_rcu); +list_for_each_entry (device, &fs_devices->devices, dev_list) + device->fs_devices = fs_devices; + + fs_devices->seeding = true; + fs_devices->num_devices = seed_devices->num_devices; + fs_devices->open_devices = seed_devices->open_devices; + fs_devices->missing_devices = seed_devices->missing_devices; + fs_devices->rotating = seed_devices->rotating; + fs_devices->latest_dev = seed_devices->latest_dev; + +memcpy (fs_devices->fsid, seed_devices->fsid,BTRFS_FSID_SIZE ); +memcpy (fs_devices->metadata_uuid, seed_devices->metadata_uuid, +BTRFS_FSID_SIZE ); +memcpy (disk_super->fsid, seed_devices->fsid,BTRFS_FSID_SIZE ); + + super_flags =btrfs_super_flags (disk_super) |BTRFS_SUPER_FLAG_SEEDING ; +btrfs_set_super_flags (disk_super, super_flags); + + seed_devices->opened =0 ; +free_fs_devices (seed_devices); +} +/* * Store the expected generation for seed devices in device items. */
@@ -3005,6 +3041,8 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path orig_super_total_bytes);btrfs_set_super_num_devices (fs_info->super_copy, orig_super_num_devices); +if (seeding_dev) +btrfs_rollback_sprout (fs_info, seed_devices);mutex_unlock (&fs_info->chunk_mutex);mutex_unlock (&fs_info->fs_devices->device_list_mutex); error_trans:
Signature · 1 line
2.53.0
v3 cover btrfs: fix failed sprout device add rollback
In reply to a message not in the archiveHi,
This series fixes the error path for adding the first writable device to a seed filesystem.
I verified the failure path locally in QEMU with null_blk and a seed btrfs filesystem:
- create btrfs on /dev/nullb0
- set the seed flag with btrfstune -S 1
- mount the filesystem read-only
- run "btrfs device add -f /dev/nullb1 /mnt"
- inject -EIO after metadata chunk creation in init_first_rw_device()
Before these fixes, the injected failure triggered:
- WARN_ON(!list_empty(&device->post_commit_list)) in btrfs_free_device()
- a NULL pointer dereference through btrfs_show_devname()
- the sprout fs_devices state left with no open devices
Without the injected failure, the same device-add path succeeds.
AI disclosure: AI assistance was used during code analysis, patch development, and drafting parts of the commit messages and cover letter. I reviewed the final code and local validation results and take responsibility for this submission.
Changes in v3:
- Add AI assistance disclosure to the cover letter.
- No code changes.
Guanghui Yang (3):
btrfs: detach failed sprout device from transaction update list
btrfs: restore active device pointers after failed sprout
btrfs: roll back sprout setup after device add failure
fs/btrfs/volumes.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
base-commit: 38fec10eb60d687e30c8c6b5420d86e8149f7557
Signature · 1 line
2.53.0
Re: [PATCH v3 0/3] btrfs: fix failed sprout device add rollback
Quoted 20 lines · Guanghui Yang
Hi,
This series fixes the error path for adding the first writable device to a seed filesystem.
I verified the failure path locally in QEMU with
null_blkand a seed btrfs filesystem:- create btrfs on
/dev/nullb0
- set the seed flag with btrfstune -S 1
- mount the filesystem read-only
- run "btrfs device add -f/dev/nullb1/mnt"
- inject-EIOafter metadata chunk creation ininit_first_rw_device()Before these fixes, the injected failure triggered:
- WARN_ON(!list_empty(&device->post_commit_list)) in
btrfs_free_device()
- a NULL pointer dereference throughbtrfs_show_devname()
- the sproutfs_devicesstate left with no open devices
Quoted 6 lines
Without the injected failure, the same device-add path succeeds.
AI disclosure: AI assistance was used during code analysis, patch development, and drafting parts of the commit messages and cover letter. I reviewed the final code and local validation results and take responsibility for this submission.
I mean the assisted-by tag for each patch.
I'll add them when merging using your last disclosed one.
Especially you seem to intentionally leave a blank line for "Assisted-by" tag but filled nothing.
Hope you are not pretending to find all those bugs by yourself, not only for btrfs but also for all other subsystems you're going to submit patches.
And tell your agent to add the "assisted-by" tag or read the "Documentation/process/coding-assistants.rst" file.
Otherwise the patches look good to me, and will be pushed to for-next.
Quoted 15 lines
Changes in v3:
- Add AI assistance disclosure to the cover letter.
- No code changes.Guanghui Yang (3):
btrfs: detach failed sprout device from transaction update list
btrfs: restore active device pointers after failed sprout
btrfs: roll back sprout setup after device add failurefs/btrfs/volumes.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)base-commit:
38fec10eb60d687e30c8c6b5420d86e8149f7557
Re: [PATCH v3 0/3] btrfs: fix failed sprout device add rollback
Quoted 34 lines · Qu Wenruo
Quoted 20 lines · Guanghui Yang
Hi,
This series fixes the error path for adding the first writable device to a seed filesystem.
I verified the failure path locally in QEMU with
null_blkand a seed btrfs filesystem:- create btrfs on
/dev/nullb0
- set the seed flag with btrfstune -S 1
- mount the filesystem read-only
- run "btrfs device add -f/dev/nullb1/mnt"
- inject-EIOafter metadata chunk creation ininit_first_rw_device()Before these fixes, the injected failure triggered:
- WARN_ON(!list_empty(&device->post_commit_list)) in
btrfs_free_device()
- a NULL pointer dereference throughbtrfs_show_devname()
- the sproutfs_devicesstate left with no open devicesQuoted 6 lines
Without the injected failure, the same device-add path succeeds.
AI disclosure: AI assistance was used during code analysis, patch development, and drafting parts of the commit messages and cover letter. I reviewed the final code and local validation results and take responsibility for this submission.
I mean the assisted-by tag for each patch.
I'll add them when merging using your last disclosed one.
Especially you seem to intentionally leave a blank line for "Assisted- by" tag but filled nothing.
Quoted 7 lines
Hope you are not pretending to find all those bugs by yourself, not only for btrfs but also for all other subsystems you're going to submit patches.
And tell your agent to add the "assisted-by" tag or read the "
Documentation/process/coding-assistants.rst" file.Otherwise the patches look good to me, and will be pushed to for-next.
Reviewed-by Qu Wenruo <[email protected]>
More minor problems to your patches, there are lot of super long lines, over 100 chars,
And DOS line endings in patches.
Run checkpatch before sending it.
Quoted 18 lines
Quoted 16 lines
Changes in v3:
- Add AI assistance disclosure to the cover letter.
- No code changes.Guanghui Yang (3):
btrfs: detach failed sprout device from transaction update list
btrfs: restore active device pointers after failed sprout
btrfs: roll back sprout setup after device add failure
fs/btrfs/volumes.c| 43 +++++++++++++++++++++++++++++++++++++++++++1 file changed, 43 insertions(+)base-commit:
38fec10eb60d687e30c8c6b5420d86e8149f7557