Re: [PATCH v2 13/20] md/md-llbitmap: clamp state-machine walks to tracked bits

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

Pre-existing issues:
- [Critical] llbitmap_cond_end_sync prematurely marks partially synced chunks as completed, risking data corruption and skipped resyncs.
--

commit e55ca490604a05215923d84a1b1272a73343ea48
Author: Yu Kuai <[email protected]>

md/md-llbitmap: clamp state-machine walks to tracked bits

This commit adds bounds checking to llbitmap_state_machine() to prevent
walking past the tracked bitmap chunks. It ensures the state machine range
is clamped when the requested end bit exceeds the tracked boundary.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 00b77ecd600be..581ee9e4a3467 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -983,7 +983,10 @@ static enum llbitmap_state llbitmap_state_machine(struct llbitmap *llbitmap,
>  		llbitmap_init_state(llbitmap);
>  		return BitNone;
>  	}
> -
> +	if (start >= llbitmap->chunks)
> +		return BitNone;
> +	if (end >= llbitmap->chunks)
> +		end = llbitmap->chunks - 1;
>  	while (start <= end) {

[Severity: Critical]
This is a pre-existing issue, but does the inclusive bound here cause
prematurely completed resyncs?

Looking at llbitmap_cond_end_sync(), it passes the current sector's chunk
index as the end bit:

drivers/md/md-llbitmap.c:llbitmap_cond_end_sync() {
    ...
    llbitmap_state_machine(llbitmap, 0, sector >> llbitmap->chunkshift,
                           BitmapActionEndsync);
    ...
}

Because the while loop condition is start <= end, the chunk currently
undergoing resync will be included in BitmapActionEndsync and transition to
BitDirty prematurely.

If start_sync() is subsequently called for the remainder of this chunk,
won't it see BitDirty and return false? This would cause the resync loop
to interpret it as no resync needed, skipping the rest of the chunk's
sectors and leaving them out of sync.

Should the end parameter be treated as exclusive instead, or does
llbitmap_cond_end_sync() need to subtract one from the end chunk when
sector is not aligned to the chunk size?

>  		enum llbitmap_state c = llbitmap_read(llbitmap, start);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13