Re: [PATCH v4 04/25] md/raid5: reject zero-sector reshape chunks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Writing large unsigned values to the `chunk_size` or `layout` sysfs attributes causes integer truncation to negative values, completely bypassing validation and leading to arbitrary geometry corruption or kernel BUGs.
--

--- Patch [4]: [PATCH v4 04/25] md/raid5: reject zero-sector reshape chunks ---
commit 6759d75e360239d90630b8d047daa1d3cb528597
Author: Yu Kuai <yukuai@fygo.io>

md/raid5: reject zero-sector reshape chunks

Sashiko reported that RAID5 can accept a reshape chunk size that becomes
zero sectors. chunk_size_store() stores the sysfs byte value as n >> 9, so
writing a value below 512 bytes sets mddev->new_chunk_sectors to zero.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0c5c9fb0606ee..4276b125f94cb 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -8479,6 +8479,8 @@ static int check_reshape(struct mddev *mddev)
>  		return 0; /* nothing to do */
>  	if (has_failed(conf))
>  		return -EINVAL;
> +	if (!mddev->new_chunk_sectors)
> +		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does this check adequately protect against
negative values?

In drivers/md/md.c:chunk_size_store(), user-supplied unsigned values are
shifted and truncated into the signed int field new_chunk_sectors without
overflow checks:

    mddev->new_chunk_sectors = n >> 9;

If a large value is written, new_chunk_sectors can become negative. This
would bypass the !mddev->new_chunk_sectors check introduced here.

Additionally, in raid5_check_reshape(), validation is skipped for negative
values:

    if (new_chunk > 0) {
        ...
    }

Would it be better for this check to reject all values <= 0, to prevent
negative chunk sizes from corrupting I/O geometry?

A similar truncation issue exists for new_layout in
drivers/md/md.c:layout_store(). A negative layout value bypasses validation
in raid5_check_reshape() and eventually triggers a BUG() in
raid5_compute_sector() if it hits the default case.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801172519.2982121-1-yukuai@kernel.org?part=4