Re: [PATCH v2 2/4] md: Handle pers->run failure in level_store
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-22 15:01:48
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] md: NULL pointer dereference in md_handle_request() when resuming array after pers->run() failure - [Medium] md: sysfs group md_redundancy_group leaked on pers->run() failure -- commit 4ab3c278fcfe04083eb0fb2b801da26eb96c6cec Author: Zhihao Cheng [off-list ref] md: Handle pers->run failure in level_store This commit fixes a NULL pointer dereference that occurs when raid_disks is set after raid5_run() fails during array conversion. It handles the error by stopping the array if the new personality fails to run, allowing a subsequent active order to restart it.
quoted hunk ↗ jump to hunk
diff --git a/drivers/md/md.c b/drivers/md/md.c index 680b34a63cb3d..f8fffc5de1143 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -4229,7 +4230,31 @@ level_store(struct mddev *mddev, const char *buf, size_t len) mddev->in_sync = 1; timer_delete_sync(&mddev->safemode_timer); } - pers->run(mddev); + rv = pers->run(mddev); + if (rv) { + /* + * ->run() has released the private data of the new personality, + * while the old one has already been released as well. There is + * nothing to fall back to, so stop the array to avoid leaving + * 'mddev->pers' pointing to a personality which has no private + * data, and reminds user to try to active the mddev again. + */ + pr_warn("md: %s: failed to run %s after takeover, please try to active\n", + mdname(mddev), pers->head.name); + if (md_bitmap_enabled(mddev, true)) + mddev->bitmap_ops->flush(mddev); + clear_bit(MD_SERIALIZE_POLICY, &mddev->flags); + mddev_destroy_serial_pool(mddev, NULL); + __md_stop(mddev);
[Severity: High] Does calling __md_stop() here introduce a NULL pointer dereference for in-flight IOs? If level_store() is called while IOs are active, mddev_suspend_and_lock() will force them to wait in md_handle_request(). __md_stop() sets mddev->pers = NULL. When the error path later reaches out_unlock and calls mddev_unlock_and_resume(), will the trapped IOs wake up and unconditionally call mddev->pers->make_request() while mddev->pers is NULL, panicking the kernel?
quoted hunk ↗ jump to hunk
+ rdev_for_each(rdev, mddev) + if (rdev->raid_disk >= 0) + sysfs_unlink_rdev(mddev, rdev); + set_capacity_and_notify(mddev->gendisk, 0); + mddev->changed = 1; + md_new_event(); + sysfs_notify_dirent_safe(mddev->sysfs_state); + goto out_unlock;
[Severity: Medium] When converting from a non-redundancy array to a redundancy array, sysfs_create_group() is called earlier in level_store() to add md_redundancy_group. Does this error path leak the sysfs group and its kernfs_node references, since mddev->to_remove is not set to &md_redundancy_group to trigger cleanup in mddev_unlock()?
quoted hunk ↗ jump to hunk
+ } set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags); if (!mddev->thread) md_update_sb(mddev, 1);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260922141456.738937-1-chengzhihao1@huawei.com?part=2