[PATCH] md: don't ignore bd_link_disk_holder() failure in bind_rdev_to_array()

[PATCH] md: don't ignore bd_link_disk_holder() failure in bind_rdev_to_array()

[Nguyen Ngoc Thang]

bind_rdev_to_array() adds the rdev to mddev->disks and then calls
bd_link_disk_holder() without checking the result. If the link fails
(e.g. an allocation failure in the holder or symlink setup), no
bd_holder_disk is recorded for the member, but the rdev stays bound.

Stopping the array later calls bd_unlink_disk_holder() for every
member, which finds no holder and warns:

  WARNING: block/holder.c:147 at bd_unlink_disk_holder+0xd8/0x2d4
  Call trace:
   bd_unlink_disk_holder+0xd8/0x2d4 block/holder.c:146
   md_kick_rdev_from_array+0x8c/0x324 drivers/md/md.c:2683
   export_array drivers/md/md.c:2714 [inline]
   do_md_stop+0x708/0xe6c drivers/md/md.c:7252
   md_ioctl+0x14ec/0x23e4 drivers/md/md.c:8462

Link the holder before registering the rdev kobject, fail the bind
if it cannot be created, and drop it again if kobject_add() fails.
All callers already export the rdev when binding fails.

Fixes: e09b457bdb7e ("block: simplify holder symlink handling")
Reported-by: syzbot+0a8119ab801b078213ea@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0a8119ab801b078213ea
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Tested in QEMU (x86_64, KASAN + lockdep + failslab) with a reproducer
that sweeps fail-nth over ADD_NEW_DISK on a RAID1 /dev/md0 backed by
/dev/ram0 and stops the array after each attempt:

  before: ADD_NEW_DISK returns 0 after the holder link fails, and the
          next STOP_ARRAY hits WARNING at block/holder.c:147
  after:  ADD_NEW_DISK fails with -ENOMEM instead, no warning, and
          no KASAN or lockdep reports

 drivers/md/md.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 5b2add94631b..0ad8f298aa78 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2618,9 +2618,14 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
 	if (mddev->raid_disks)
 		mddev_create_serial_pool(mddev, rdev);
 
-	if ((err = kobject_add(&rdev->kobj, &mddev->kobj, "dev-%s", b)))
+	err = bd_link_disk_holder(rdev->bdev, mddev->gendisk);
+	if (err)
 		goto fail;
 
+	err = kobject_add(&rdev->kobj, &mddev->kobj, "dev-%s", b);
+	if (err)
+		goto fail_unlink;
+
 	/* failure here is OK */
 	err = sysfs_create_link(&rdev->kobj, bdev_kobj(rdev->bdev), "block");
 	rdev->sysfs_state = sysfs_get_dirent_safe(rdev->kobj.sd, "state");
@@ -2630,10 +2635,10 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
 		sysfs_get_dirent_safe(rdev->kobj.sd, "bad_blocks");
 
 	list_add_rcu(&rdev->same_set, &mddev->disks);
-	bd_link_disk_holder(rdev->bdev, mddev->gendisk);
-
 	return 0;
 
+ fail_unlink:
+	bd_unlink_disk_holder(rdev->bdev, mddev->gendisk);
  fail:
 	pr_warn("md: failed to register dev-%s for %s\n",
 		b, mdname(mddev));
-- 
2.43.0