[PATCH] md: fix use-after-free in md_attr_store()

[PATCH] md: fix use-after-free in md_attr_store()

[Sajal Gupta]

md_attr_store() calls sysfs_break_active_protection() for a "clear"
write to array_state before it calls mddev_get().
sysfs_break_active_protection() does kobject_get() on the mddev
kobject. If the array is concurrently being deleted, the kobject
refcount can already be zero, which triggers "refcount_t: addition on 0;
use-after-free".

Fix by calling mddev_get() first. If the array is being deleted it
fails with -EBUSY before the kobject is touched, and if it succeeds the
array cannot be freed until mddev_put(). The -EBUSY path no longer
needs to call sysfs_unbreak_active_protection().

Fixes: 9e59d609763f ("md: call del_gendisk in control path")
Reported-by: syzbot+dcad997ed2b5bd95cc5e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=dcad997ed2b5bd95cc5e
Tested-by: syzbot+dcad997ed2b5bd95cc5e@syzkaller.appspotmail.com
Signed-off-by: Sajal Gupta <sajal2005gupta@gmail.com>
---
 drivers/md/md.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 03e35386a802..26f17dd24bdf 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6163,17 +6163,16 @@ md_attr_store(struct kobject *kobj, struct attribute *attr,
 	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);
+
+	if (entry->store == array_state_store && cmd_match(page, "clear"))
+		kn = sysfs_break_active_protection(kobj, attr);
+
 	rv = entry->store(mddev, page, length);
 
 	/*
-- 
2.56.0