Re: [PATCH AUTOSEL 6.18] md: skip bitmap accounting for empty write ranges

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] md: missing sectors check in md_free_cloned_bio() before md_bitmap_end()
--

commit 6836aa7ecbc70ba01261dda2960c8f2c78bf3f55
Author: Yu Kuai <yukuai@fygo.io>

md: skip bitmap accounting for empty write ranges

This commit skips md bitmap start_write() and end_write() accounting when a
write bio has zero sectors or when personality bitmap mapping turns the bio
into an empty bitmap range.

Link: https://patch.msgid.link/20260802195038.164272-11-yukuai@kernel.org

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 03d328506b285..ce3bd127e3bb3 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9266,6 +9266,8 @@ static void md_bitmap_start(struct mddev *mddev,
>  		mddev->pers->bitmap_sector(mddev, &md_io_clone->offset,
>  					   &md_io_clone->sectors);
>  
> +	if (!md_io_clone->sectors)
> +		return;
>  	fn(mddev, md_io_clone->offset, md_io_clone->sectors);
>  }
>  
> @@ -9284,7 +9286,8 @@ static void md_end_clone_io(struct bio *bio)
>  	struct bio *orig_bio = md_io_clone->orig_bio;
>  	struct mddev *mddev = md_io_clone->mddev;
>  
> -	if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
> +	if (bio_data_dir(orig_bio) == WRITE && md_io_clone->sectors &&
> +	    md_bitmap_enabled(mddev, false))
>  		md_bitmap_end(mddev, md_io_clone);
>  
>  	if (bio->bi_status && !orig_bio->bi_status)
> @@ -9308,12 +9311,14 @@ static void md_clone_bio(struct mddev *mddev, struct bio **bio)
>  	md_io_clone = container_of(clone, struct md_io_clone, bio_clone);
>  	md_io_clone->orig_bio = *bio;
>  	md_io_clone->mddev = mddev;
> +	md_io_clone->sectors = 0;
>  	if (blk_queue_io_stat(bdev->bd_disk->queue))
>  		md_io_clone->start_time = bio_start_io_acct(*bio);
>  	else
>  		md_io_clone->start_time = 0;
>  
> -	if (bio_data_dir(*bio) == WRITE && md_bitmap_enabled(mddev, false)) {
> +	if (bio_data_dir(*bio) == WRITE && bio_sectors(*bio) &&
> +	    md_bitmap_enabled(mddev, false)) {
>  		md_io_clone->offset = (*bio)->bi_iter.bi_sector;
>  		md_io_clone->sectors = bio_sectors(*bio);
>  		md_io_clone->rw = op_stat_group(bio_op(*bio));

[Severity: High]
Should md_free_cloned_bio() in drivers/md/md.c also check
md_io_clone->sectors before calling md_bitmap_end(), just like
md_end_clone_io() does here?

In drivers/md/md.c, md_free_cloned_bio() still has:

	if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
		md_bitmap_end(mddev, md_io_clone);

In raid5_make_request() -> md_account_bio() -> md_clone_bio(), the bio is
cloned before calling make_stripe_request(). If make_stripe_request()
returns STRIPE_WAIT_RESHAPE when a reshape is interrupted,
raid5_make_request() aborts the cloned bio via md_free_cloned_bio():

	if (res == STRIPE_WAIT_RESHAPE) {
		md_free_cloned_bio(bi);
		return false;
	}

With this patch, there are two cases where md_bitmap_start() is skipped
with md_io_clone->sectors == 0:

1. When bio_sectors(*bio) == 0 (for example, a zero-sector REQ_OP_WRITE |
REQ_FUA bio, or a zero-sector REQ_OP_WRITE | REQ_PREFLUSH bio falling
through log_handle_flush_request() with -EAGAIN), md_clone_bio() sets
md_io_clone->sectors = 0 and skips initializing md_io_clone->offset and
md_io_clone->rw (leaving stale contents from the mddev->io_clone_set
mempool) and skips calling md_bitmap_start().

2. When bio_sectors(*bio) != 0, but mddev->pers->bitmap_sector()
(raid5_bitmap_sector()) maps the range to md_io_clone->sectors == 0,
md_bitmap_start() returns early without calling start_write() or
start_discard().

If make_stripe_request() then returns STRIPE_WAIT_RESHAPE,
md_free_cloned_bio() will unconditionally call md_bitmap_end(mddev,
md_io_clone) with md_io_clone->sectors == 0 (and uninitialized offset and
rw in the first case). For llbitmap, llbitmap_end_write() and
llbitmap_end_discard() compute:

	unsigned long end = (offset + sectors - 1) >> llbitmap->chunkshift;

When sectors == 0, if offset == 0 this underflows to ULONG_MAX and walks
llbitmap->pctl[] out of bounds, or if offset > 0 it calls
llbitmap_release_barrier() to drop page barrier references on
pctl->active that were never raised.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-307-sashal@kernel.org?part=1