Re: [PATCH v4 16/25] md/md-llbitmap: don't skip reshape ranges from bitmap state

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

Pre-existing issues:
- [High] Integer overflow in `blocks` calculation can cause `md_do_sync` to=
 skip the entire array.
--

--- Patch [16]: [PATCH v4 16/25] md/md-llbitmap: don't skip reshape ranges =
from bitmap state ---
commit 20fdc8d04a69beb6ba6a356b102dd6e6920204f1
Author: Yu Kuai <[email protected]>

md/md-llbitmap: don't skip reshape ranges from bitmap state

Reshape progress is tracked by array metadata rather than llbitmap.
Do not let llbitmap skip_sync_blocks() suppress reshape ranges based on
stale bitmap state before the corresponding checkpoint is persisted.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index eed3e248b730d..5597b1afcb580 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1686,12 +1686,20 @@ static sector_t llbitmap_skip_sync_blocks(struct =
mddev *mddev, sector_t offset)
>  {
>  	struct llbitmap *llbitmap =3D mddev->bitmap;
>  	unsigned long p =3D offset >> llbitmap->chunkshift;
>  	int blocks =3D llbitmap->chunksize - (offset & (llbitmap->chunksize - 1=
));
        ^^^^

[Severity: High]
This isn't a bug introduced by this patch, but can this integer subtraction
overflow?

If llbitmap->chunksize is dynamically calculated up to 2^31 sectors for
extremely large arrays, would 'blocks' overflow to a negative value since it
is a 32-bit signed int?

When returned as a sector_t (unsigned 64-bit), C sign-extends it to a massi=
ve
value (0xffffffff80000000). Could the caller md_do_sync() add this to its
position tracker, exceeding max_sectors and prematurely terminating the res=
ync
process, which would leave the array degraded?

>  	enum llbitmap_state c;
> =20
>  	if (p >=3D llbitmap->chunks)
>  		return 0;
>  	c =3D llbitmap_read(llbitmap, p);
> =20
> +	/*
> +	 * Reshape progress is tracked by array metadata rather than llbitmap.
> +	 * Skipping reshape ranges from stale bitmap state can lose data after a
> +	 * restart before the corresponding bits are checkpointed to disk.
> +	 */
> +	if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
> +		return 0;
> +
>  	/* always skip unwritten blocks */
>  	if (c =3D=3D BitUnwritten)
>  		return blocks;

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