linux-kernel - Re: [BUG] possible race between md_free_disk and md_notify

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: <1e288eb5-c67b-c9ca-c57e-2855b18785b1@huaweicloud.com>
Date: Thu, 20 Feb 2025 12:06:44 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Guillaume Morin <guillaume@...infr.org>
Cc: Yu Kuai <yukuai1@...weicloud.com>, linux-raid@...r.kernel.org,
 linux-kernel@...r.kernel.org, song@...nel.org,
 "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [BUG] possible race between md_free_disk and md_notify_reboot

Hi,

在 2025/02/20 11:45, Guillaume Morin 写道:
> On 20 Feb 11:19, Yu Kuai wrote:
>>
>> Hi,
>>
>> 在 2025/02/20 11:05, Guillaume Morin 写道:
>>> how it was guaranteed that mddev_get() would fail as mddev_free() does not check or synchronize with the active atomic
>>
>> Please check how mddev is freed, start from mddev_put(). There might be
>> something wrong, but it's not what you said.
> 
> I will take a look. Though if you're confident that this logic protects
> any uaf, that makes sense to me.
> 
> However as I mentioned this is not what the crash was about (I mentioned
> the UAF in passing). The GPF seems to be about deleting the _next_
> pointer while iterating over all mddevs. The mddev_get on the
> current item is not going to help with this.
> 

You don't need to emphasize this, it is still speculate without solid
theoretical analysis. The point about mddev_get() is that it's done
inside the lock, it shoud gurantee continue iterating should be fine.

I just take a quick look, the problem looks obviously to me, see how
md_seq_show() handle the iteration.

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 465ca2af1e6e..7c7a58f618c1 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9911,8 +9911,11 @@ static int md_notify_reboot(struct notifier_block 
*this,
                         mddev_unlock(mddev);
                 }
                 need_delay = 1;
-               mddev_put(mddev);
-               spin_lock(&all_mddevs_lock);
+
+               spin_lock(&all_mddevs_lock)
+               if (atomic_dec_and_test(&mddev->active))
+                       __mddev_put(mddev);
+
         }
         spin_unlock(&all_mddevs_lock);


Thanks,
Kuai