[PATCH] md: don't allow changing the personality of a failed array

[PATCH] md: don't allow changing the personality of a failed array

[Sanan Hasanov]

From: Sanan Hasanov <sanan.hasanov@ucf.edu>

level_store() lets userspace switch a running array to another
personality through the takeover path, and then ignores the return value
of pers->run(). Takeover is expected to always produce something the new
personality can run, but that no longer holds for an array that has
already failed: since commit 57668f0a4cc4 ("raid5: introduce MD_BROKEN"),
has_failed() treats MD_BROKEN as fatal, so raid5_run() refuses such an
array. It then frees the new conf and clears mddev->private, while
level_store() has already installed the raid5 personality and freed the
old one. The array is left with mddev->pers == &raid5_personality and
mddev->private == NULL, and the next operation that reaches the
personality dereferences NULL:

  md/raid0md0: Disk failure on loop0 detected, failing array.
  md/raid:md0: not enough operational devices (1/3 failed)
  md/raid:md0: failed to run raid set.
  KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
  RIP: 0010:kasan_byte_accessible+0x12/0x30 mm/kasan/generic.c:210
  Call Trace:
   spin_lock_irq include/linux/spinlock_rt.h:96 [inline]
   lock_all_device_hash_locks_irq drivers/md/raid5.c:107 [inline]
   raid5_quiesce+0xc2/0x8a0 drivers/md/raid5.c
   __md_stop_writes+0xeb/0x4b0 drivers/md/md.c:7038
   md_set_readonly+0x330/0x560 drivers/md/md.c:7138
   array_state_store+0x584/0xe90 drivers/md/md.c:4720

syzbot gets there by failing a member of a raid0 array, which sets
MD_BROKEN, then writing "raid5" to md/level and "readonly" to
md/array_state. The same applies to a two-disk raid1 array whose last
working device has failed: raid1_error() sets MD_BROKEN but leaves
degraded at 1, so raid5_takeover_raid1() still accepts it.

MD_BROKEN is never cleared while the array is running, and a failed
array can't be turned into a working one by changing its level. Refuse
the change up front with -EBUSY, which is also what state_store()
returns when failing a device leaves the array broken.

Fixes: 57668f0a4cc4 ("raid5: introduce MD_BROKEN")
Reported-by: syzbot+0b672810a475890d3636@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0b672810a475890d3636
Cc: stable@vger.kernel.org
Signed-off-by: Sanan Hasanov <sanan.hasanov@ucf.edu>
---
 drivers/md/md.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63c..367b183f1e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4088,6 +4088,12 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
 	    mddev->sysfs_active)
 		goto out_unlock;
 
+	if (test_bit(MD_BROKEN, &mddev->flags)) {
+		pr_warn("md: %s: cannot change personality of a failed array\n",
+			mdname(mddev));
+		goto out_unlock;
+	}
+
 	rv = -EINVAL;
 	if (!mddev->pers->quiesce) {
 		pr_warn("md: %s: %s does not support online personality change\n",
-- 
2.34.1