[PATCH] [v2] md: fix rcu protection in md_wakeup_thread

[PATCH] [v2] md: fix rcu protection in md_wakeup_thread

[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@windriver.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);
-- 
2.27.0

Re: [PATCH] [v2] md: fix rcu protection in md_wakeup_thread

[Li Nan]

在 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@windriver.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@huawei.com>

-- 
Thanks,
Nan

Re: [PATCH] [v2] md: fix rcu protection in md_wakeup_thread

[Yu Kuai]

Hi,

在 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@windriver.com>
> ---
>   drivers/md/md.c | 14 ++++++--------
>   drivers/md/md.h |  8 +++++++-
>   2 files changed, 13 insertions(+), 9 deletions(-)

Sorry for the delay.
Reviewed-by: Yu Kuai <yukuai@fnnas.com>