Re: [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md chunk shrinks

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

Pre-existing issues:
- [Critical] The patch intends to reject shrinking reshapes when `ID_LLBITM=
AP` is active by returning `-EOPNOTSUPP` from `raid10_check_reshape()`. How=
ever, a severe pre-existing logic flaw in `mddev_start_reshape()` causes it=
 to evaluate any non-zero return (including negative error codes) as a trut=
hy condition. This bypasses the patch's protection entirely and triggers a =
destructive array corruption if the reshape is interrupted.
--

--- Patch [23]: [PATCH v5 23/29] md/raid10: reject llbitmap reshape when md=
 chunk shrinks ---
commit f65520f6e55cf9e2cea49909a73ee06aeb4f57b1
Author: Yu Kuai <[email protected]>

md/raid10: reject llbitmap reshape when md chunk shrinks

llbitmap reshape keeps one live bitmap and cannot safely make an
existing bitmap bit cover a smaller data range. Shrinking mddev->chunk_sect=
ors
can shrink the effective data range covered by each bit for the RAID10
reshape geometry.

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index ed3c6fbe65f78..1c3393467667b 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -4245,6 +4245,10 @@ static int raid10_check_reshape(struct mddev *mdde=
v)
> =20
>  	if (conf->geo.far_copies !=3D 1 && !conf->geo.far_offset)
>  		return -EINVAL;
> +	if (mddev->bitmap_id =3D=3D ID_LLBITMAP &&
> +	    mddev->new_chunk_sectors &&
> +	    mddev->new_chunk_sectors < mddev->chunk_sectors)
> +		return -EOPNOTSUPP;

[Severity: Critical]
This is a pre-existing issue, but returning an error code here doesn't seem
to prevent the reshape from starting if it was already interrupted.

Looking at mddev_start_reshape() in drivers/md/md.c, it evaluates the negat=
ive
error code as a truthy condition:

	if (mddev->reshape_position =3D=3D MaxSector ||
	    mddev->pers->check_reshape =3D=3D NULL ||
	    mddev->pers->check_reshape(mddev)) {
		clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
		ret =3D mddev->pers->start_reshape(mddev);

Since -EOPNOTSUPP evaluates to true, will this erroneously call
mddev->pers->start_reshape() instead of aborting the operation?

If so, does this cause raid10_start_reshape() to destructively reset the
reshape progress back to 0 for an interrupted reshape?

	} else
		conf->reshape_progress =3D 0;
	conf->reshape_safe =3D conf->reshape_progress;

> =20
>  	if (setup_geo(&geo, mddev, geo_start) !=3D conf->copies)
>  		/* mustn't change number of copies */

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260802195038.1642=
[email protected]?part=3D23