Re: [PATCH v3 19/21] md/raid5: reject llbitmap reshape when md chunk shrinks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Writing a value less than 512 to the `chunk_size` sysfs attribute bypasses chunk validation and causes a divide-by-zero kernel panic during reshape.
--

--- Patch [19]: [PATCH v3 19/21] md/raid5: reject llbitmap reshape when md chunk shrinks ---
commit fd741104c871960b0c928c6a03adfc7d3df39d70
Author: Yu Kuai <yukuai@fygo.io>

md/raid5: reject llbitmap reshape when md chunk shrinks
    
This commit prevents llbitmap reshapes from shrinking the chunk size,
as doing so cannot safely make an existing bitmap bit cover a smaller
data range.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 8b28afa538e2b..f0817599dd271 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -8509,6 +8509,9 @@ static int check_reshape(struct mddev *mddev)
>  	if (!check_stripe_cache(mddev))
>  		return -ENOSPC;
>  
> +	if (mddev->bitmap_id == ID_LLBITMAP &&
> +	    mddev->new_chunk_sectors < mddev->chunk_sectors)
> +		return -EOPNOTSUPP;

[Severity: High]
This is a pre-existing issue, but since we are looking at new_chunk_sectors
validation, what prevents a user from triggering a divide-by-zero panic by
writing a value smaller than 512 bytes to the chunk_size sysfs attribute?

In chunk_size_store(), user input less than 512 bytes is converted to
0 sectors before calling into the reshape checks:

drivers/md/md.c:chunk_size_store() {
    ...
    mddev->new_chunk_sectors = n >> 9;
    err = mddev->pers->check_reshape(mddev);
    ...
}

The downstream chunk validation logic skips the chunk check if the value
is zero:

    if (new_chunk > 0) {
        if (!is_power_of_2(new_chunk))
            return -EINVAL;
    }

If a user has no llbitmap configured, this 0 sector value appears to bypass
validation. If conf->chunk_sectors is updated to 0, will subsequent I/O or
reshape operations that divide by conf->chunk_sectors (such as
raid5_bitmap_sector_map() or raid5_bio_lowest_chunk_sector()) trigger a
divide-by-zero panic?

>  	if (mddev->new_chunk_sectors > mddev->chunk_sectors ||
>  	    mddev->delta_disks > 0)
>  		if (resize_chunks(conf,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785206690.git.yukuai@fygo.io?part=19