Re: [PATCH v4 13/25] md/md-llbitmap: finish reshape geometry

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

New issues:
- [High] The `llbitmap_reshape_finish` callback is dead code and misses API=
 integration, which would cause a sleep-in-atomic panic if properly integra=
ted.

Pre-existing issues:
- [Critical] The pending timer can be re-armed during array teardown after =
`timer_delete_sync()`, leading to a Use-After-Free when the timer fires on =
the freed `llbitmap`.
--

--- Patch [13]: [PATCH v4 13/25] md/md-llbitmap: finish reshape geometry ---
commit 726da375aa50cd87c4a667a63fcb2d5924d50556
Author: Yu Kuai <[email protected]>

md/md-llbitmap: finish reshape geometry

Commit the staged llbitmap geometry when reshape finishes. When assembling a
stopped reshape, md_run() creates the bitmap before publishing mddev->pers.
Refresh the staged reshape geometry again from llbitmap_load().

Link: https://lore.kernel.org/all/20260726185916.2223460-1-mykola@meshstor.=
io/

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 37d1312e3eab9..f41f9ba2063b5 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> @@ -1694,6 +1703,30 @@ static void llbitmap_dirty_bits(struct mddev *mdde=
v, unsigned long s,
>  	llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
>  }
> =20
> +static void llbitmap_reshape_finish(struct mddev *mddev)
> +{
> +	struct llbitmap *llbitmap =3D mddev->bitmap;
> +
> +	if (mddev->pers->quiesce)
> +		mddev->pers->quiesce(mddev, 1);
> +
> +	mutex_lock(&mddev->bitmap_info.mutex);

[Severity: High]
Is this callback actually invoked anywhere by the MD core?

Looking at the MD core code, it seems md_finish_reshape() doesn't call
bitmap_ops->reshape_finish(). This means the staged geometry remains
uncommitted.

Additionally, if it were to be called from md_finish_reshape(), wouldn't
this cause a sleep-in-atomic panic? The callers of md_finish_reshape() hold
a spinlock, but this function acquires a mutex and performs a sleeping wait
via mddev->pers->quiesce().

> +	llbitmap_flush(mddev);
> +
> +	llbitmap->chunksize =3D llbitmap->reshape_chunksize;
> +	llbitmap->chunkshift =3D ffz(~llbitmap->chunksize);
> +	llbitmap->chunks =3D llbitmap->reshape_chunks;
> +	llbitmap->sync_size =3D llbitmap->reshape_sync_size;
> +	llbitmap_refresh_reshape(llbitmap);
> +	mddev->bitmap_info.chunksize =3D llbitmap->chunksize;
> +	llbitmap_update_sb(llbitmap);
> +	__llbitmap_flush(mddev);
> +	mutex_unlock(&mddev->bitmap_info.mutex);
> +
> +	if (mddev->pers->quiesce)
> +		mddev->pers->quiesce(mddev, 0);
> +}
> +

[ ... ]

> @@ -1991,6 +2024,7 @@ static struct bitmap_operations llbitmap_ops =3D {
>  	.get_stats		=3D llbitmap_get_stats,
>  	.dirty_bits		=3D llbitmap_dirty_bits,
>  	.prepare_range		=3D llbitmap_prepare_range,
> +	.reshape_finish		=3D llbitmap_reshape_finish,
>  	.write_all		=3D llbitmap_write_all,
> =20
>  	.groups			=3D md_llbitmap_groups,

[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free risk during
array teardown in llbitmap_destroy()?

Looking at llbitmap_destroy(), the pending timer is deleted before flushing
the workqueue:

    timer_delete_sync(&llbitmap->pending_timer);
    flush_workqueue(md_llbitmap_io_wq);

If the array is being destroyed on an error path where BITMAP_WRITE_ERROR is
set, __md_stop_writes() skips calling mddev->bitmap_ops->flush(mddev), so
the LLPageFlush bit is never set.

When the workqueue executes md_llbitmap_daemon_fn(), it can encounter
unexpired dirty pages without the flush bit, set restart =3D true, and call
mod_timer().

Could this re-arm the timer right before the llbitmap is freed, allowing
the timer to fire on the freed object?

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