Re: [PATCH v4 18/25] md/md-llbitmap: clamp state-machine walks to tracked bits

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] TOCTOU race in llbitmap_state_machine() allows chunk state to be blindly overwritten by concurrent actions, leading to data corruption.
--

--- Patch [18]: [PATCH v4 18/25] md/md-llbitmap: clamp state-machine walks to tracked bits ---
commit 5929c29d9f90d552df02364e1245faa32b6df618
Author: Yu Kuai <yukuai@fygo.io>

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

This commit addresses an issue where llbitmap_state_machine() can be called
with an end bit beyond the tracked chunks. It clamps the start and end values
to ensure the state machine does not walk past the tracked bitmap boundaries.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1012,7 +1012,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) {
>  		enum llbitmap_state c = llbitmap_read(llbitmap, start);

[Severity: Critical]
This is a pre-existing issue, but does this lockless read-modify-write cycle
allow concurrent updates to overwrite each other?

When llbitmap_state_machine() calculates the new state and eventually calls
llbitmap_write(), it blindly overwrites the state without an atomic cmpxchg
loop or per-chunk locking.

Could this cause data corruption during concurrent execution? For example, if
llbitmap_start_sync() is running lockless with BitmapActionClearUnwritten
across all chunks, and llbitmap_start_write() concurrently processes the same
chunk with BitmapActionStartwrite:

Thread 1 (Sync):
    c = llbitmap_read(llbitmap, start);
    // c is BitCleanUnwritten
    state = state_machine[c][BitmapActionClearUnwritten];
    // state becomes BitUnwritten

Thread 2 (Write):
    c = llbitmap_read(llbitmap, start);
    // c is BitCleanUnwritten
    state = state_machine[c][BitmapActionStartwrite];
    // state becomes BitNeedSync
    llbitmap_write(llbitmap, state, start);

Thread 1 (Sync):
    llbitmap_write(llbitmap, state, start);
    // blindly overwrites the written chunk with BitUnwritten

This would leave the actively written chunk marked as BitUnwritten, causing
llbitmap_skip_sync_blocks() to skip syncing this chunk and leaving stale
parity on the recovering drive.

>

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