[PATCH] btrfs: properly cleanup replace_task when the replace failed to start
Qu Wenruo <[email protected]>
| Newsgroups | org.kernel.vger.linux-btrfs |
|---|---|
| Message-ID | <2bb108e8d3204506a80562fec03d0d365583631f.1786348282.git.wqu@suse.com> |
In the function btrfs_dev_replace_start(), we have several error paths
that assigns replace_start without reverting it back to NULL.
There are two involved error paths:
- There is already a running dev-replace
Then replace_task is over-written to the current task.
This is the one with long running effect.
- The btrfs_start_transaction() call failed
This is much harder to hit though.
This can result the replace_task check inside btrfs_map_block() to be
incorrectly triggered, not taking dev_replace->rwsem.
Normally that replace_task check is to protect regular IOs from racing
with dev-replace, which will modify the device list.
But since dev_replace->rwsem is incorrectly updated, a process
triggering the update will no longer be protected from dev-replace's
device list modification, thus later IO can get stale device info,
triggering things like use-after-free.
Fix the problem by:
- Moving the replace_task assignment after replace_state check
- Reset replace_task to NULL if btrfs_start_transaction() failed
This is reported by Sashiko, which found the existing bug during review
of another patch, and since the bug is an existing one, it's not shown in
the summary, but only in the detail page.
Link: https://sashiko.dev/#/patchset/tencent_853134544C3CE88A219EEB21346E2510D308%40qq.com
Fixes: 8cca35cb29f8 ("btrfs: don't take dev_replace rwsem on task already holding it")
Signed-off-by: Qu Wenruo <[email protected]>
---
fs/btrfs/dev-replace.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 72cba7fed942..5fc1dec88fb2 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -633,7 +633,6 @@ static int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
goto leave;
down_write(&dev_replace->rwsem);
- dev_replace->replace_task = current;
switch (dev_replace->replace_state) {
case BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED:
case BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED:
@@ -647,6 +646,7 @@ static int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
goto leave;
}
+ dev_replace->replace_task = current;
dev_replace->cont_reading_from_srcdev_mode = read_src;
dev_replace->srcdev = src_device;
dev_replace->tgtdev = tgt_device;
@@ -693,6 +693,7 @@ static int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED;
dev_replace->srcdev = NULL;
dev_replace->tgtdev = NULL;
+ dev_replace->replace_task = NULL;
up_write(&dev_replace->rwsem);
goto leave;
}
--
2.54.0