On Thu, May 5, 2022 at 1:18 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> wrote:
>
> From: Guoqing Jiang <guoqing.jiang@xxxxxxxxxxxxxxx>
>
> Generally, the md_unregister_thread is called with reconfig_mutex, but
> raid_message in dm-raid doesn't hold reconfig_mutex to unregister thread,
> so md_unregister_thread can be called simulitaneously from two call sites
> in theory.
Can we add lock/unlock into raid_message? Are there some constraints here?
Thanks,
Song
>
> Then after previous commit which remove the protection of reconfig_mutex
> for md_unregister_thread completely, the potential issue could be worse
> than before.
>
> Let's take pers_lock at the beginning of function to ensure reentrancy.
>
> Reported-by: Donald Buczek <buczek@xxxxxxxxxxxxx>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> ---
> drivers/md/md.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index a70e7f0f9268..c401e063bec8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7962,17 +7962,22 @@ EXPORT_SYMBOL(md_register_thread);
>
> void md_unregister_thread(struct md_thread **threadp)
> {
> - struct md_thread *thread = *threadp;
> - if (!thread)
> - return;
> - pr_debug("interrupting MD-thread pid %d\n", task_pid_nr(thread->tsk));
> - /* Locking ensures that mddev_unlock does not wake_up a
> + struct md_thread *thread;
> +
> + /*
> + * Locking ensures that mddev_unlock does not wake_up a
> * non-existent thread
> */
> spin_lock(&pers_lock);
> + thread = *threadp;
> + if (!thread) {
> + spin_unlock(&pers_lock);
> + return;
> + }
> *threadp = NULL;
> spin_unlock(&pers_lock);
>
> + pr_debug("interrupting MD-thread pid %d\n", task_pid_nr(thread->tsk));
> kthread_stop(thread->tsk);
> kfree(thread);
> }
> --
> 2.31.1
>
