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

[sashiko-bot@kernel.org]

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 <yukuai@fygo.io>

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 = mddev->bitmap;
>  	unsigned long p = offset >> llbitmap->chunkshift;
>  	int blocks = 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 massive
value (0xffffffff80000000). Could the caller md_do_sync() add this to its
position tracker, exceeding max_sectors and prematurely terminating the resync
process, which would leave the array degraded?

>  	enum llbitmap_state c;
>  
>  	if (p >= llbitmap->chunks)
>  		return 0;
>  	c = llbitmap_read(llbitmap, p);
>  
> +	/*
> +	 * 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 == BitUnwritten)
>  		return blocks;

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