linux-kernel - Re: [PATCH] [v2] md: fix rcu protection in md_wakeup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <02e14a95-a421-04c5-0a26-2b108bf19117@huaweicloud.com>
Date: Sat, 18 Oct 2025 15:20:04 +0800
From: Li Nan <linan666@...weicloud.com>
To: Yun Zhou <yun.zhou@...driver.com>, song@...nel.org, yukuai3@...wei.com
Cc: linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [v2] md: fix rcu protection in md_wakeup_thread



在 2025/10/15 16:32, Yun Zhou 写道:
> We attempted to use RCU to protect the pointer 'thread', but directly
> passed the value when calling md_wakeup_thread(). This means that the
> RCU pointer has been acquired before rcu_read_lock(), which renders
> rcu_read_lock() ineffective and could lead to a use-after-free.
> 
> Fixes: 446931543982 ("md: protect md_thread with rcu")
> Signed-off-by: Yun Zhou <yun.zhou@...driver.com>
> ---
>   drivers/md/md.c | 14 ++++++--------
>   drivers/md/md.h |  8 +++++++-
>   2 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 4e033c26fdd4..7df3c47ee709 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -100,7 +100,7 @@ static int remove_and_add_spares(struct mddev *mddev,
>   				 struct md_rdev *this);
>   static void mddev_detach(struct mddev *mddev);
>   static void export_rdev(struct md_rdev *rdev, struct mddev *mddev);
> -static void md_wakeup_thread_directly(struct md_thread __rcu *thread);
> +static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
>   
>   /*
>    * Default number of read corrections we'll attempt on an rdev
> @@ -4982,7 +4982,7 @@ static void stop_sync_thread(struct mddev *mddev, bool locked)
>   	 * Thread might be blocked waiting for metadata update which will now
>   	 * never happen
>   	 */
> -	md_wakeup_thread_directly(mddev->sync_thread);
> +	md_wakeup_thread_directly(&mddev->sync_thread);
>   	if (work_pending(&mddev->sync_work))
>   		flush_work(&mddev->sync_work);
>   
> @@ -8194,22 +8194,21 @@ static int md_thread(void *arg)
>   	return 0;
>   }
>   
> -static void md_wakeup_thread_directly(struct md_thread __rcu *thread)
> +static void md_wakeup_thread_directly(struct md_thread __rcu **thread)
>   {
>   	struct md_thread *t;
>   
>   	rcu_read_lock();
> -	t = rcu_dereference(thread);
> +	t = rcu_dereference(*thread);
>   	if (t)
>   		wake_up_process(t->tsk);
>   	rcu_read_unlock();
>   }
>   
> -void md_wakeup_thread(struct md_thread __rcu *thread)
> +void __md_wakeup_thread(struct md_thread __rcu *thread)
>   {
>   	struct md_thread *t;
>   
> -	rcu_read_lock();
>   	t = rcu_dereference(thread);
>   	if (t) {
>   		pr_debug("md: waking up MD thread %s.\n", t->tsk->comm);
> @@ -8217,9 +8216,8 @@ void md_wakeup_thread(struct md_thread __rcu *thread)
>   		if (wq_has_sleeper(&t->wqueue))
>   			wake_up(&t->wqueue);
>   	}
> -	rcu_read_unlock();
>   }
> -EXPORT_SYMBOL(md_wakeup_thread);
> +EXPORT_SYMBOL(__md_wakeup_thread);
>   
>   struct md_thread *md_register_thread(void (*run) (struct md_thread *),
>   		struct mddev *mddev, const char *name)
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 51af29a03079..c989cc9f6216 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -878,6 +878,12 @@ struct md_io_clone {
>   
>   #define THREAD_WAKEUP  0
>   
> +#define md_wakeup_thread(thread) do {   \
> +	rcu_read_lock();                    \
> +	__md_wakeup_thread(thread);         \
> +	rcu_read_unlock();                  \
> +} while (0)
> +
>   static inline void safe_put_page(struct page *p)
>   {
>   	if (p) put_page(p);
> @@ -891,7 +897,7 @@ extern struct md_thread *md_register_thread(
>   	struct mddev *mddev,
>   	const char *name);
>   extern void md_unregister_thread(struct mddev *mddev, struct md_thread __rcu **threadp);
> -extern void md_wakeup_thread(struct md_thread __rcu *thread);
> +extern void __md_wakeup_thread(struct md_thread __rcu *thread);
>   extern void md_check_recovery(struct mddev *mddev);
>   extern void md_reap_sync_thread(struct mddev *mddev);
>   extern enum sync_action md_sync_action(struct mddev *mddev);

LGTM

Reviewed-by: Li Nan <linan122@...wei.com>

-- 
Thanks,
Nan