On 5/11/22 2:02 AM, Song Liu wrote:
On Tue, May 10, 2022 at 5:35 AM Donald Buczek <buczek@xxxxxxxxxxxxx> wrote:
On 5/10/22 2:09 PM, Guoqing Jiang wrote:
On 5/10/22 8:01 PM, Donald Buczek wrote:
I guess v2 is the best at the moment. I pushed a slightly modified v2 to
md-next.
I think, this can be used to get a double-free from md_unregister_thread.
Please review
https://lore.kernel.org/linux-raid/8312a154-14fb-6f07-0cf1-8c970187cc49@xxxxxxxxxxxxx/
That is supposed to be addressed by the second one, pls consider it too.
Right, but this has not been pulled into md-next. I just wanted to note, that the current state of md-next has this problem.
Thanks for reminder.
If the other patch is taken, too, and works as intended, that would be solved.
[PATCH 2/2] md: protect md_unregister_thread from reentrancy
Good catch!
Guoqing, current 2/2 doesn't apply cleanly. Could you please resend it on top of
md-next?
Hmm, no issue from my side.
~/source/md> git am
0001-md-protect-md_unregister_thread-from-reentrancy.patch
Applying: md: protect md_unregister_thread from reentrancy
~/source/md> git log --oneline |head -5
dc7147a88766 md: protect md_unregister_thread from reentrancy
5a36c493dc82 md: don't unregister sync_thread with reconfig_mutex held
49c3b9266a71 block: null_blk: Improve device creation with configfs
db060f54e0c5 block: null_blk: Cleanup messages
b3a0a73e8a79 block: null_blk: Cleanup device creation and deletion
Anyway, it is attached. I will rebase it to your latest tree if
something gets wrong.
Thanks,
Guoqing
>From a2da80f62f15023e3fee7a02488c143dfff647b3 Mon Sep 17 00:00:00 2001
From: Guoqing Jiang <guoqing.jiang@xxxxxxxxxxxxxxx>
Date: Fri, 29 Apr 2022 16:49:09 +0800
Subject: [PATCH 2/2] md: protect md_unregister_thread from reentrancy
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.
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
