Re: [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Time-of-check to time-of-use (TOCTOU) race in the teardown path allows the timer to be re-armed after deletion, causing a use-after-free.
--

--- Patch [9]: [PATCH v5 09/29] md/md-llbitmap: stop daemon timer rearm on destroy ---
commit ea383c856bd9022c7e57c7a025fcf8fb539acf1e
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: stop daemon timer rearm on destroy

This commit attempts to fix a race where the daemon timer is re-armed
during array teardown. It adds a BITMAP_SHUTDOWN flag, sets it before
deleting the timer and canceling the daemon work, and checks the flag
before queuing or rearming the work.

> 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

[ ... ]

> @@ -1096,7 +1103,7 @@ static void md_llbitmap_daemon_fn(struct work_struct *work)
>  		goto retry;
>  
>  	/* If some page is dirty but not expired, setup timer again */
> -	if (restart)
> +	if (restart && !test_bit(BITMAP_SHUTDOWN, &llbitmap->flags))
>  		mod_timer(&llbitmap->pending_timer,
>  			  jiffies + llbitmap->mddev->bitmap_info.daemon_sleep * HZ);
>  }

[ ... ]

> @@ -1179,7 +1186,9 @@ static void llbitmap_destroy(struct mddev *mddev)
>  
>  	mutex_lock(&mddev->bitmap_info.mutex);
>  
> +	set_bit(BITMAP_SHUTDOWN, &llbitmap->flags);
>  	timer_delete_sync(&llbitmap->pending_timer);
> +	cancel_work_sync(&llbitmap->daemon_work);
>  	flush_workqueue(md_llbitmap_io_wq);
>  	flush_workqueue(md_llbitmap_unplug_wq);

[Severity: High]
This isn't a bug introduced by this patch, but does this teardown sequence
still leave a race condition that can result in a use-after-free?

Since the BITMAP_SHUTDOWN check and mod_timer() in md_llbitmap_daemon_fn()
are not atomic, a worker can evaluate BITMAP_SHUTDOWN as false but get
preempted before arming the timer.

Concurrently, llbitmap_destroy() sets BITMAP_SHUTDOWN, calls
timer_delete_sync() (which returns immediately if the timer isn't pending),
and blocks in cancel_work_sync().

When the preempted worker resumes, it executes mod_timer(), re-arming the
timer. Once the worker finishes, cancel_work_sync() unblocks and
llbitmap_destroy() frees llbitmap, leaving the timer armed and able to
access freed memory when it fires.

Would replacing timer_delete_sync() with timer_shutdown_sync() safely
prevent any subsequent mod_timer() calls from succeeding?

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