Skip to content

Commit ca9f84d

Browse files
YuKuai-huaweigregkh
authored andcommitted
md: fix mddev uaf while iterating all_mddevs list
commit 8542870237c3a48ff049b6c5df5f50c8728284fa upstream. While iterating all_mddevs list from md_notify_reboot() and md_exit(), list_for_each_entry_safe is used, and this can race with deletint the next mddev, causing UAF: t1: spin_lock //list_for_each_entry_safe(mddev, n, ...) mddev_get(mddev1) // assume mddev2 is the next entry spin_unlock t2: //remove mddev2 ... mddev_free spin_lock list_del spin_unlock kfree(mddev2) mddev_put(mddev1) spin_lock //continue dereference mddev2->all_mddevs The old helper for_each_mddev() actually grab the reference of mddev2 while holding the lock, to prevent from being freed. This problem can be fixed the same way, however, the code will be complex. Hence switch to use list_for_each_entry, in this case mddev_put() can free the mddev1 and it's not safe as well. Refer to md_seq_show(), also factor out a helper mddev_put_locked() to fix this problem. Cc: Christoph Hellwig <[email protected]> Link: https://lore.kernel.org/linux-raid/[email protected] Fixes: f265143 ("md: stop using for_each_mddev in md_notify_reboot") Fixes: 16648ba ("md: stop using for_each_mddev in md_exit") Reported-and-tested-by: Guillaume Morin <[email protected]> Closes: https://lore.kernel.org/all/[email protected]/ Signed-off-by: Yu Kuai <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> [skip md_seq_show() that is not exist] Signed-off-by: Yu Kuai <[email protected]> Cc: Salvatore Bonaccorso <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent bf1dc50 commit ca9f84d

File tree

1 file changed

+12
-6
lines changed

1 file changed

+12
-6
lines changed

drivers/md/md.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,12 @@ static void __mddev_put(struct mddev *mddev)
684684
queue_work(md_misc_wq, &mddev->del_work);
685685
}
686686

687+
static void mddev_put_locked(struct mddev *mddev)
688+
{
689+
if (atomic_dec_and_test(&mddev->active))
690+
__mddev_put(mddev);
691+
}
692+
687693
void mddev_put(struct mddev *mddev)
688694
{
689695
if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
@@ -9704,11 +9710,11 @@ EXPORT_SYMBOL_GPL(rdev_clear_badblocks);
97049710
static int md_notify_reboot(struct notifier_block *this,
97059711
unsigned long code, void *x)
97069712
{
9707-
struct mddev *mddev, *n;
9713+
struct mddev *mddev;
97089714
int need_delay = 0;
97099715

97109716
spin_lock(&all_mddevs_lock);
9711-
list_for_each_entry_safe(mddev, n, &all_mddevs, all_mddevs) {
9717+
list_for_each_entry(mddev, &all_mddevs, all_mddevs) {
97129718
if (!mddev_get(mddev))
97139719
continue;
97149720
spin_unlock(&all_mddevs_lock);
@@ -9720,8 +9726,8 @@ static int md_notify_reboot(struct notifier_block *this,
97209726
mddev_unlock(mddev);
97219727
}
97229728
need_delay = 1;
9723-
mddev_put(mddev);
97249729
spin_lock(&all_mddevs_lock);
9730+
mddev_put_locked(mddev);
97259731
}
97269732
spin_unlock(&all_mddevs_lock);
97279733

@@ -10043,7 +10049,7 @@ void md_autostart_arrays(int part)
1004310049

1004410050
static __exit void md_exit(void)
1004510051
{
10046-
struct mddev *mddev, *n;
10052+
struct mddev *mddev;
1004710053
int delay = 1;
1004810054

1004910055
unregister_blkdev(MD_MAJOR,"md");
@@ -10064,7 +10070,7 @@ static __exit void md_exit(void)
1006410070
remove_proc_entry("mdstat", NULL);
1006510071

1006610072
spin_lock(&all_mddevs_lock);
10067-
list_for_each_entry_safe(mddev, n, &all_mddevs, all_mddevs) {
10073+
list_for_each_entry(mddev, &all_mddevs, all_mddevs) {
1006810074
if (!mddev_get(mddev))
1006910075
continue;
1007010076
spin_unlock(&all_mddevs_lock);
@@ -10076,8 +10082,8 @@ static __exit void md_exit(void)
1007610082
* the mddev for destruction by a workqueue, and the
1007710083
* destroy_workqueue() below will wait for that to complete.
1007810084
*/
10079-
mddev_put(mddev);
1008010085
spin_lock(&all_mddevs_lock);
10086+
mddev_put_locked(mddev);
1008110087
}
1008210088
spin_unlock(&all_mddevs_lock);
1008310089

0 commit comments

Comments
 (0)