Skip to content

Commit 2434b54

Browse files
committed
md: call del_gendisk in control path
JIRA: https://issues.redhat.com/browse/RHEL-94433 JIRA: https://issues.redhat.com/browse/RHEL-20191 JIRA: https://issues.redhat.com/browse/RHEL-9656 commit 9e59d60 Author: Xiao Ni <xni@redhat.com> Date: Wed Jun 11 15:31:06 2025 +0800 md: call del_gendisk in control path Now del_gendisk and put_disk are called asynchronously in workqueue work. The asynchronous way has a problem that the device node can still exist after mdadm --stop command returns in a short window. So udev rule can open this device node and create the struct mddev in kernel again. So put del_gendisk in control path and still leave put_disk in md_kobj_release to avoid uaf of gendisk. Function del_gendisk can't be called with reconfig_mutex. If it's called with reconfig mutex, a deadlock can happen. del_gendisk waits all sysfs files access to finish and sysfs file access waits reconfig mutex. So put del_gendisk after releasing reconfig mutex. But there is still a window that sysfs can be accessed between mddev_unlock and del_gendisk. So some actions (add disk, change level, .e.g) can happen which lead unexpected results. MD_DELETED is used to resolve this problem. MD_DELETED is set before releasing reconfig mutex and it should be checked for these sysfs access which need reconfig mutex. For sysfs access which don't need reconfig mutex, del_gendisk will wait them to finish. But it doesn't need to do this in function mddev_lock_nointr. There are ten places that call it. * Five of them are in dm raid which we don't need to care. MD_DELETED is only used for md raid. * stop_sync_thread, md_do_sync and md_start_sync are related sync request, and it needs to wait sync thread to finish before stopping an array. * md_ioctl: md_open is called before md_ioctl, so ->openers is added. It will fail to stop the array. So it doesn't need to check MD_DELETED here * md_set_readonly: It needs to call mddev_set_closing_and_sync_blockdev when setting readonly or read_auto. So it will fail to stop the array too because MD_CLOSING is already set. Reviewed-by: Yu Kuai <yukuai3@huawei.com> Signed-off-by: Xiao Ni <xni@redhat.com> Link: https://lore.kernel.org/linux-raid/20250611073108.25463-2-xni@redhat.com Signed-off-by: Yu Kuai <yukuai3@huawei.com> Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
1 parent 14a1b94 commit 2434b54

File tree

2 files changed

+47
-12
lines changed

2 files changed

+47
-12
lines changed

drivers/md/md.c

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -637,9 +637,6 @@ static void __mddev_put(struct mddev *mddev)
637637
mddev->ctime || mddev->hold_active)
638638
return;
639639

640-
/* Array is not configured at all, and not held active, so destroy it */
641-
set_bit(MD_DELETED, &mddev->flags);
642-
643640
/*
644641
* Call queue_work inside the spinlock so that flush_workqueue() after
645642
* mddev_find will succeed in waiting for the work to be done.
@@ -874,6 +871,16 @@ void mddev_unlock(struct mddev *mddev)
874871
kobject_del(&rdev->kobj);
875872
export_rdev(rdev, mddev);
876873
}
874+
875+
/* Call del_gendisk after release reconfig_mutex to avoid
876+
* deadlock (e.g. call del_gendisk under the lock and an
877+
* access to sysfs files waits the lock)
878+
* And MD_DELETED is only used for md raid which is set in
879+
* do_md_stop. dm raid only uses md_stop to stop. So dm raid
880+
* doesn't need to check MD_DELETED when getting reconfig lock
881+
*/
882+
if (test_bit(MD_DELETED, &mddev->flags))
883+
del_gendisk(mddev->gendisk);
877884
}
878885
EXPORT_SYMBOL_GPL(mddev_unlock);
879886

@@ -5800,32 +5807,37 @@ md_attr_store(struct kobject *kobj, struct attribute *attr,
58005807
struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
58015808
struct mddev *mddev = container_of(kobj, struct mddev, kobj);
58025809
ssize_t rv;
5810+
struct kernfs_node *kn = NULL;
58035811

58045812
if (!entry->store)
58055813
return -EIO;
58065814
if (!capable(CAP_SYS_ADMIN))
58075815
return -EACCES;
5816+
5817+
if (entry->store == array_state_store && cmd_match(page, "clear"))
5818+
kn = sysfs_break_active_protection(kobj, attr);
5819+
58085820
spin_lock(&all_mddevs_lock);
58095821
if (!mddev_get(mddev)) {
58105822
spin_unlock(&all_mddevs_lock);
5823+
if (kn)
5824+
sysfs_unbreak_active_protection(kn);
58115825
return -EBUSY;
58125826
}
58135827
spin_unlock(&all_mddevs_lock);
58145828
rv = entry->store(mddev, page, length);
58155829
mddev_put(mddev);
5830+
5831+
if (kn)
5832+
sysfs_unbreak_active_protection(kn);
5833+
58165834
return rv;
58175835
}
58185836

58195837
static void md_kobj_release(struct kobject *ko)
58205838
{
58215839
struct mddev *mddev = container_of(ko, struct mddev, kobj);
58225840

5823-
if (mddev->sysfs_state)
5824-
sysfs_put(mddev->sysfs_state);
5825-
if (mddev->sysfs_level)
5826-
sysfs_put(mddev->sysfs_level);
5827-
5828-
del_gendisk(mddev->gendisk);
58295841
put_disk(mddev->gendisk);
58305842
}
58315843

@@ -6672,8 +6684,9 @@ static int do_md_stop(struct mddev *mddev, int mode)
66726684
mddev->bitmap_info.offset = 0;
66736685

66746686
export_array(mddev);
6675-
66766687
md_clean(mddev);
6688+
set_bit(MD_DELETED, &mddev->flags);
6689+
66776690
if (mddev->hold_active == UNTIL_STOP)
66786691
mddev->hold_active = 0;
66796692
}

drivers/md/md.h

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -702,11 +702,26 @@ static inline bool reshape_interrupted(struct mddev *mddev)
702702

703703
static inline int __must_check mddev_lock(struct mddev *mddev)
704704
{
705-
return mutex_lock_interruptible(&mddev->reconfig_mutex);
705+
int ret;
706+
707+
ret = mutex_lock_interruptible(&mddev->reconfig_mutex);
708+
709+
/* MD_DELETED is set in do_md_stop with reconfig_mutex.
710+
* So check it here.
711+
*/
712+
if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
713+
ret = -ENODEV;
714+
mutex_unlock(&mddev->reconfig_mutex);
715+
}
716+
717+
return ret;
706718
}
707719

708720
/* Sometimes we need to take the lock in a situation where
709721
* failure due to interrupts is not acceptable.
722+
* It doesn't need to check MD_DELETED here, the owner which
723+
* holds the lock here can't be stopped. And all paths can't
724+
* call this function after do_md_stop.
710725
*/
711726
static inline void mddev_lock_nointr(struct mddev *mddev)
712727
{
@@ -715,7 +730,14 @@ static inline void mddev_lock_nointr(struct mddev *mddev)
715730

716731
static inline int mddev_trylock(struct mddev *mddev)
717732
{
718-
return mutex_trylock(&mddev->reconfig_mutex);
733+
int ret;
734+
735+
ret = mutex_trylock(&mddev->reconfig_mutex);
736+
if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
737+
ret = -ENODEV;
738+
mutex_unlock(&mddev->reconfig_mutex);
739+
}
740+
return ret;
719741
}
720742
extern void mddev_unlock(struct mddev *mddev);
721743

0 commit comments

Comments
 (0)