Re: [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush

[sashiko-bot@kernel.org]

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

New issues:
- [High] The patch unmasks a use-after-free of `llbitmap` during array teardown by allowing the daemon worker to re-arm the timer after it has been deleted.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but threads waiting for IO barriers will hang indefinitely if page suspension times out.
--

--- Patch [1]: [PATCH v5 01/29] md/md-llbitmap: clear flush state after daemon flush ---
commit 5b6bbbac2eb163d696a69aa1800138e01de10a7e
Author: Yu Kuai <yukuai@fygo.io>

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

This patch changes the llbitmap daemon to consume the LLPageFlush bit using
test_and_clear_bit() instead of just testing it. This ensures that after an
explicit flush, subsequent dirty bits on the same page will correctly wait
for the barrier_idle window before being cleaned.

> 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
> @@ -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]
Does consuming the LLPageFlush bit here expose a use-after-free during
array teardown?

During normal array teardown, llbitmap_flush() sets LLPageFlush and queues
daemon_work. Because this patch consumes LLPageFlush, a subsequent execution
of the work can see LLPageFlush as clear. It will then evaluate
time_before(jiffies, pctl->expire) as true, set restart = true, and blindly
call mod_timer() at the end of md_llbitmap_daemon_fn().

If this work runs via flush_workqueue(md_llbitmap_io_wq) in
llbitmap_destroy(), it arms the timer after timer_delete_sync() has already
finished:

drivers/md/md-llbitmap.c:llbitmap_destroy() {
    ...
    timer_delete_sync(&llbitmap->pending_timer);
    flush_workqueue(md_llbitmap_io_wq);
    ...
}

The llbitmap is then freed, and the timer can fire on freed memory.

>  			restart = true;
>  			continue;
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but threads waiting for IO
barriers will hang indefinitely if page suspension times out.

If llbitmap_suspend_timeout() times out (e.g., due to slow IO), it resurrects
the percpu_ref but skips calling wake_up(&pctl->wait):

drivers/md/md-llbitmap.c:llbitmap_suspend_timeout() {
    ...
    percpu_ref_resurrect(&pctl->active);
    return -ETIMEDOUT;
    ...
}

The daemon then continues and skips llbitmap_resume() which normally does the
wakeup. Threads waiting in llbitmap_raise_barrier() for the dying state to
clear will never be woken up:

drivers/md/md-llbitmap.c:llbitmap_raise_barrier() {
    ...
    wait_event(pctl->wait, !percpu_ref_is_dying(&pctl->active));
    ...
}

Can this lead to a permanent hang of IO threads accessing the md array?

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