linux-kernel - Re: [PATCH] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d2574af5-9410-d296-ef74-97f3e43dc1cc@huaweicloud.com>
Date: Wed, 15 Oct 2025 14:58:48 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>, Yun Zhou <yun.zhou@...driver.com>
Cc: agk@...hat.com, snitzer@...nel.org, mpatocka@...hat.com, song@...nel.org,
 dm-devel@...ts.linux.dev, linux-kernel@...r.kernel.org,
 linux-raid@...r.kernel.org, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH] md: fix rcu protection in md_wakeup_thread

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);