Re: [PATCH] md: fix NULL dereference of pers in md_start_sync()

[yu kuai]

Hi,

在 2026/11/6 16:35, Li Youhong 写道:
> From: Li Youhong <liyouhong@xxxxxxxxxx>
>
> md_start_sync() is queued on md_misc_wq to start resync or to
> manage spare disks. On a read-only array it calls
> remove_and_add_spares(), which ends up in remove_spares() and
> invokes pers->hot_remove_disk.
>
> md_start_sync() does not check whether mddev->pers is still
> valid before using it. dm-raid stops the array through
> md_stop(), which clears pers. If the queued work then runs,
> remove_spares() dereferences pers->hot_remove_disk and crashes.
> The read-write path has the same exposure in
> md_choose_sync_action().
>
> Return via not_running if pers is already gone.
>
> Fixes: db5e653d7c9f ("md: delay choosing sync action to md_start_sync()")
> Reported-by: syzbot+a53c2918175b36cfd601@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=a53c2918175b36cfd601
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Li Youhong <liyouhong@xxxxxxxxxx>
> ---
>   drivers/md/md.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3..b7e162f52408 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -10286,6 +10286,13 @@ static void md_start_sync(struct work_struct *ws)
>   		noio_flags = memalloc_noio_save();
>   	}
>   
> +	/*
> +	 * md_stop() clears pers. This work may have been queued earlier and
> +	 * can run after that, so pers may already be NULL here.
> +	 */
> +	if (!mddev->pers)
> +		goto not_running;

The plain NULL checking is not a correct fix, as md_start_sync() should not be running
after md_stop(), I think the root cause is that dm-raid is missing a call like
stop_sync_thread() to flush the sync_work.

> +
>   	if (!md_is_rdwr(mddev)) {
>   		/*
>   		 * On a read-only array we can:

-- 
Thanks,
Kuai