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

[Yu Kuai]

Hi,

在 2025/10/15 14:35, Paul Menzel 写道:
Dear Yun,

Thank you for your patch.

Am 15.10.25 um 07:59 schrieb 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.

Could you elaborate a little more – especially as nobody has noticed 
this so far since v6.5-rc1?


This looks correct, memory dereference should be protected by rcu, while
in fact this is done before function parameter passing.

However, in most places, a null check should be enough because the md
thread can't be unregistered concurrently, that's probably no one ever
meet the problem.

However, for the modification, I'll prefer a new marcon like following,
so that you don't need to update all the callers:

Thanks,
Kuai

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 1de550108756..d48ee1b50d27 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8384,11 +8384,10 @@ static void md_wakeup_thread_directly(struct 
md_thread __rcu *thread)
        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);
@@ -8396,9 +8395,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 1979c2d4fe89..9ec62afc2a7d 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -882,6 +882,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);
@@ -895,7 +901,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);