Re: [PATCH 8/8] btrfs: do not clean up repair bio if submit fails

[Boris Burkov <boris@bur.io>]

On Thu, Feb 10, 2022 at 05:44:26PM -0500, Josef Bacik wrote:
> 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 UAF and NULL
> pointer deref bugs because we race with the endio function that is
> cleaning up the bio.  Instead just return 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.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
>  fs/btrfs/extent_io.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 29ffb2814e5c..16b6820c913d 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2640,7 +2640,6 @@ 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;
> -	blk_status_t status;
>  
>  	btrfs_debug(fs_info,
>  		   "repair read error: read error at %llu", start);
> @@ -2679,13 +2678,14 @@ 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)
> -- 
> 2.26.3
>