[PATCH 2/2] md: call del_gendisk in control path

[Xiao Ni <xni@redhat.com>]

Now del_gendisk and put_disk are called asynchronously in workqueue work.
The asynchronous way also 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.

But there is 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. And if we delete MD_DELETED and only use
MD_CLOSING in stop control path, the sysfs files can't be accessed if
do_md_stop stuck when io hange. So we keep MD_DELETED here and set
MD_DELETED before mddev_unlock.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 drivers/md/md.c | 53 ++++++++++++++++++++++++++++++++++++++++++-------
 drivers/md/md.h | 16 ++++++++++++++-
 2 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9b9950ed6ee9..a62867f34aa8 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -606,15 +606,13 @@ static inline struct mddev *mddev_get(struct mddev *mddev)
 }
 
 static void mddev_delayed_delete(struct work_struct *ws);
+static bool can_delete_gendisk(struct mddev *mddev);
 
 static void __mddev_put(struct mddev *mddev)
 {
-	if (mddev->raid_disks || !list_empty(&mddev->disks) ||
-	    mddev->ctime || mddev->hold_active)
-		return;
 
-	/* Array is not configured at all, and not held active, so destroy it */
-	set_bit(MD_DELETED, &mddev->flags);
+	if (can_delete_gendisk(mddev) == false)
+		return;
 
 	/*
 	 * Call queue_work inside the spinlock so that flush_workqueue() after
@@ -4400,6 +4398,7 @@ array_state_show(struct mddev *mddev, char *page)
 	return sprintf(page, "%s\n", array_states[st]);
 }
 
+static void delete_gendisk(struct mddev *mddev);
 static int do_md_stop(struct mddev *mddev, int ro);
 static int md_set_readonly(struct mddev *mddev);
 static int restart_array(struct mddev *mddev);
@@ -4533,6 +4532,9 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
 	    (err && st == clear))
 		clear_bit(MD_CLOSING, &mddev->flags);
 
+	if ((st == clear || st == inactive) && !err)
+		delete_gendisk(mddev);
+
 	return err ?: len;
 }
 static struct md_sysfs_entry md_array_state =
@@ -5721,19 +5723,30 @@ md_attr_store(struct kobject *kobj, struct attribute *attr,
 	struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
 	struct mddev *mddev = container_of(kobj, struct mddev, kobj);
 	ssize_t rv;
+	struct kernfs_node *kn = NULL;
 
 	if (!entry->store)
 		return -EIO;
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
+
+	if (entry->store == array_state_store && cmd_match(page, "clear"))
+		kn = sysfs_break_active_protection(kobj, attr);
+
 	spin_lock(&all_mddevs_lock);
 	if (!mddev_get(mddev)) {
 		spin_unlock(&all_mddevs_lock);
+		if (kn)
+			sysfs_unbreak_active_protection(kn);
 		return -EBUSY;
 	}
 	spin_unlock(&all_mddevs_lock);
 	rv = entry->store(mddev, page, length);
 	mddev_put(mddev);
+
+	if (kn)
+		sysfs_unbreak_active_protection(kn);
+
 	return rv;
 }
 
@@ -5746,7 +5759,6 @@ static void md_kobj_release(struct kobject *ko)
 	if (mddev->sysfs_level)
 		sysfs_put(mddev->sysfs_level);
 
-	del_gendisk(mddev->gendisk);
 	put_disk(mddev->gendisk);
 }
 
@@ -6526,6 +6538,28 @@ static int md_set_readonly(struct mddev *mddev)
 	return err;
 }
 
+static bool can_delete_gendisk(struct mddev *mddev)
+{
+	if (mddev->raid_disks || !list_empty(&mddev->disks) ||
+	    mddev->ctime || mddev->hold_active)
+		return false;
+
+	return true;
+}
+
+/* Call this function after do_md_stop with mode 0.
+ * And it can't call this function under reconfig_mutex to
+ * avoid deadlock(e.g. call del_gendisk under the lock and
+ * an access to sysfs files waits the lock)
+ */
+static void delete_gendisk(struct mddev *mddev)
+{
+	if (can_delete_gendisk(mddev) == false)
+		return;
+
+	del_gendisk(mddev->gendisk);
+}
+
 /* mode:
  *   0 - completely stop and dis-assemble array
  *   2 - stop but do not disassemble array
@@ -6588,8 +6622,8 @@ static int do_md_stop(struct mddev *mddev, int mode)
 		mddev->bitmap_info.offset = 0;
 
 		export_array(mddev);
-
 		md_clean(mddev);
+		set_bit(MD_DELETED, &mddev->flags);
 	}
 	md_new_event();
 	sysfs_notify_dirent_safe(mddev->sysfs_state);
@@ -6616,6 +6650,7 @@ static void autorun_array(struct mddev *mddev)
 	if (err) {
 		pr_warn("md: do_md_run() returned %d\n", err);
 		do_md_stop(mddev, 0);
+		delete_gendisk(mddev);
 	}
 }
 
@@ -7886,6 +7921,10 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
 out:
 	if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
 		clear_bit(MD_CLOSING, &mddev->flags);
+
+	if (cmd == STOP_ARRAY && err == 0)
+		delete_gendisk(mddev);
+
 	return err;
 }
 #ifdef CONFIG_COMPAT
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 1cf00a04bcdd..45f1027986e4 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -697,11 +697,25 @@ static inline bool reshape_interrupted(struct mddev *mddev)
 
 static inline int __must_check mddev_lock(struct mddev *mddev)
 {
-	return mutex_lock_interruptible(&mddev->reconfig_mutex);
+	int ret = 0;
+
+	ret = mutex_lock_interruptible(&mddev->reconfig_mutex);
+
+	/* MD_DELETED is set in do_md_stop with reconfig_mutex
+	 * So check it here also.
+	 */
+	if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
+		ret = -EBUSY;
+		mutex_unlock(&mddev->reconfig_mutex);
+	}
+
+	return ret;
 }
 
 /* Sometimes we need to take the lock in a situation where
  * failure due to interrupts is not acceptable.
+ * It doesn't need to check MD_DELETED here, the owner which
+ * holds the lock here can't be stopped.
  */
 static inline void mddev_lock_nointr(struct mddev *mddev)
 {
-- 
2.32.0 (Apple Git-132)