Re: [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields

[sashiko-bot@kernel.org]

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

New issues:
- [High] Out-of-bounds array access and memory corruption during reshaping due to stale chunkshift.

Pre-existing issues:
- [High] Use-After-Free (UAF) due to a TOCTOU race between teardown and timer re-arming in daemon work.
--

--- Patch [16]: [PATCH v5 16/29] md/md-llbitmap: track target reshape geometry fields ---
commit dabf6384b9392c331f7703e778887a4f5f7bb15c
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: track target reshape geometry fields
    
Track llbitmap bookkeeping for the target reshape geometry while keeping
a single live bitmap instance.
    
Add the reshape geometry fields, refresh helper, and update the load and
resize paths to keep the target geometry in sync.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index ddeea20989873..71a4903901425 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> @@ -1302,6 +1340,9 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
>  		goto out;
>  
>  	if (reshape) {
> +		llbitmap->reshape_sync_size = blocks;
> +		llbitmap->reshape_chunksize = bitmap_chunksize;
> +		llbitmap->reshape_chunks = chunks;
>  		llbitmap->chunks = max(old_chunks, chunks);

[Severity: High]
Does this code need to track or update a chunkshift for the reshape path?

During a reshape that increases the array size, bitmap_chunksize might increase.
While llbitmap->chunks is clamped here, llbitmap->chunkshift is not updated.

Later, if llbitmap_start_write() processes an I/O to the newly extended region,
it uses the stale chunkshift to compute the start chunk:

drivers/md/md-llbitmap.c:llbitmap_start_write() {
    ...
    unsigned long start = offset >> llbitmap->chunkshift;
    ...
}

Because chunkshift represents the old, smaller chunk size, start can grossly
exceed llbitmap->chunks. Since llbitmap_start_write() lacks bounds checking
against llbitmap->chunks, it calculates an out-of-bounds page_start index
and calls llbitmap_raise_barrier(), which will directly dereference the
llbitmap->pctl array out of bounds. Can this lead to memory corruption?

>  	} else {
>  		if (blocks < old_blocks && chunks < old_chunks)

[ ... ]

[Severity: High]
This is a pre-existing issue, but could the teardown sequence in
llbitmap_destroy() race with md_llbitmap_daemon_fn() and cause a
use-after-free?

In llbitmap_destroy(), the teardown sets the shutdown flag, deletes the
timer, and then cancels the daemon work:

drivers/md/md-llbitmap.c:llbitmap_destroy() {
    ...
    set_bit(BITMAP_SHUTDOWN, &llbitmap->flags);
    timer_delete_sync(&llbitmap->pending_timer);
    cancel_work_sync(&llbitmap->daemon_work);
    ...
}

However, in md_llbitmap_daemon_fn(), the shutdown check and timer re-arming
are not atomic:

drivers/md/md-llbitmap.c:md_llbitmap_daemon_fn() {
    ...
    if (restart && !test_bit(BITMAP_SHUTDOWN, &llbitmap->flags))
        mod_timer(&llbitmap->pending_timer, ...);
    ...
}

If the work thread evaluates the check to true and is preempted,
llbitmap_destroy() could run, set the shutdown flag, and execute
timer_delete_sync() which returns immediately as the timer isn't pending.
llbitmap_destroy() then blocks in cancel_work_sync().

When the work thread resumes, it calls mod_timer() to re-arm the timer
and exits. cancel_work_sync() then unblocks and llbitmap_destroy() frees
the llbitmap object. When the re-armed timer fires, it will access the
freed memory.

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