From: Josef Bacik josef@toxicpanda.com
[ Upstream commit 8cbc3001a3264d998d6b6db3e23f935c158abd4d ]
The submit helper will always run bio_endio() on the bio if it fails to submit, so cleaning up the bio just leads to a variety of use-after-free and NULL pointer dereference bugs because we race with the endio function that is cleaning up the bio. Instead just return BLK_STS_OK as the repair function has to continue to process the rest of the pages, and the endio for the repair bio will do the appropriate cleanup for the page that it was given.
Reviewed-by: Boris Burkov boris@bur.io Signed-off-by: Josef Bacik josef@toxicpanda.com Signed-off-by: David Sterba dsterba@suse.com [Minor context change fixed.] Signed-off-by: Bin Lan bin.lan.cn@windriver.com Signed-off-by: He Zhe zhe.he@windriver.com --- Build test passed. --- fs/btrfs/extent_io.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 346fc46d019b..a1946d62911c 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2624,7 +2624,6 @@ int btrfs_repair_one_sector(struct inode *inode, const int icsum = bio_offset >> fs_info->sectorsize_bits; struct bio *repair_bio; struct btrfs_io_bio *repair_io_bio; - blk_status_t status;
btrfs_debug(fs_info, "repair read error: read error at %llu", start); @@ -2664,13 +2663,13 @@ int btrfs_repair_one_sector(struct inode *inode, "repair read error: submitting new read to mirror %d", failrec->this_mirror);
- status = submit_bio_hook(inode, repair_bio, failrec->this_mirror, - failrec->bio_flags); - if (status) { - free_io_failure(failure_tree, tree, failrec); - bio_put(repair_bio); - } - return blk_status_to_errno(status); + /* + * At this point we have a bio, so any errors from submit_bio_hook() + * will be handled by the endio on the repair_bio, so we can't return an + * error here. + */ + submit_bio_hook(inode, repair_bio, failrec->this_mirror, failrec->bio_flags); + return BLK_STS_OK; }
static void end_page_read(struct page *page, bool uptodate, u64 start, u32 len)
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: 8cbc3001a3264d998d6b6db3e23f935c158abd4d
WARNING: Author mismatch between patch and upstream commit: Backport author: bin.lan.cn@windriver.com Commit author: Josef Bacikjosef@toxicpanda.com
Status in newer kernel trees: 6.14.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1)
Note: The patch differs from the upstream commit: --- 1: 8cbc3001a3264 ! 1: 90c24cbc95555 btrfs: do not clean up repair bio if submit fails @@ Metadata ## Commit message ## btrfs: do not clean up repair bio if submit fails
+ [ Upstream commit 8cbc3001a3264d998d6b6db3e23f935c158abd4d ] + The submit helper will always run bio_endio() on the bio if it fails to submit, so cleaning up the bio just leads to a variety of use-after-free and NULL pointer dereference bugs because we race with the endio @@ Commit message Reviewed-by: Boris Burkov boris@bur.io Signed-off-by: Josef Bacik josef@toxicpanda.com Signed-off-by: David Sterba dsterba@suse.com + [Minor context change fixed.] + Signed-off-by: Bin Lan bin.lan.cn@windriver.com + Signed-off-by: He Zhe zhe.he@windriver.com
## fs/btrfs/extent_io.c ## @@ fs/btrfs/extent_io.c: int btrfs_repair_one_sector(struct inode *inode, const int icsum = bio_offset >> fs_info->sectorsize_bits; struct bio *repair_bio; - struct btrfs_bio *repair_bbio; + struct btrfs_io_bio *repair_io_bio; - blk_status_t status;
btrfs_debug(fs_info, ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.15.y | Success | Success |
linux-stable-mirror@lists.linaro.org