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

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 tim=
er re-arming in daemon work.
--

--- Patch [16]: [PATCH v5 16/29] md/md-llbitmap: track target reshape geome=
try fields ---
commit dabf6384b9392c331f7703e778887a4f5f7bb15c
Author: Yu Kuai <[email protected]>

md/md-llbitmap: track target reshape geometry fields
   =20
Track llbitmap bookkeeping for the target reshape geometry while keeping
a single live bitmap instance.
   =20
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, sec=
tor_t blocks, int chunksize)
>  		goto out;
> =20
>  	if (reshape) {
> +		llbitmap->reshape_sync_size =3D blocks;
> +		llbitmap->reshape_chunksize =3D bitmap_chunksize;
> +		llbitmap->reshape_chunks =3D chunks;
>  		llbitmap->chunks =3D 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 incr=
ease.
While llbitmap->chunks is clamped here, llbitmap->chunkshift is not updated.

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

drivers/md/md-llbitmap.c:llbitmap_start_write() {
    ...
    unsigned long start =3D 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.

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