Re: [PATCH v4 01/25] md/md-llbitmap: clear flush state after daemon flush

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Consuming LLPageFlush in the daemon introduces a race condition that re-arms the timer during array shutdown, leading to a Use-After-Free of the llbitmap struct.
--

--- Patch [1]: [PATCH v4 01/25] md/md-llbitmap: clear flush state after daemon flush ---
commit abba2aa11371c8630f75f9bd27a43851e99339eb
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: clear flush state after daemon flush

This commit clears the LLPageFlush bit in md_llbitmap_daemon_fn() rather
than leaving it set. This prevents previously flushed pages from bypassing
the barrier_idle expiry check on subsequent daemon runs, making new writes
wait for the full idle window.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 5a4e2abaa7577..131582724e7e9 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1066,14 +1066,14 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
>  
>  	for (idx = 0; idx < llbitmap->nr_pages; idx++) {
>  		struct llbitmap_page_ctl *pctl = llbitmap->pctl[idx];
> +		bool flush = test_and_clear_bit(LLPageFlush, &pctl->flags);
>  
>  		if (idx > 0) {
>  			start = end + 1;
>  			end = min(end + PAGE_SIZE, llbitmap->chunks - 1);
>  		}
>  
> -		if (!test_bit(LLPageFlush, &pctl->flags) &&
> -		    time_before(jiffies, pctl->expire)) {
> +		if (!flush && time_before(jiffies, pctl->expire)) {

[Severity: High]
Could clearing the flush bit here introduce a race condition during array
shutdown that leads to a use-after-free of the llbitmap struct?

If llbitmap/daemon_sleep is configured to a small value, stopping the array
invokes llbitmap_flush(), which sets LLPageFlush and queues the daemon:

llbitmap_flush()
    ...
    queue_work(md_llbitmap_io_wq, &llbitmap->daemon_work);
    flush_work(&llbitmap->daemon_work);

If the daemon is concurrently running, it can consume the newly set
LLPageFlush bits via test_and_clear_bit(). The explicitly queued daemon run
would then see flush == false, evaluate the unexpired pages, set restart =
true, and re-arm the timer.

If disk flushing takes longer than the sleep interval, the timer can fire
and queue the work again.

Later, llbitmap_destroy() calls:

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

Because the timer already fired, timer_delete_sync() does nothing. Then
flush_workqueue() executes the queued daemon work, which incorrectly re-arms
the timer after the deletion point. The struct is then freed, and the timer
subsequently fires on freed memory.

>  			restart = true;
>  			continue;
>  		}

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