Re: [PATCH v3 2/4] md: Handle pers->run failure in level_store
From: sashiko-bot@kernel.org
Date: 2026-09-23 11:46:30
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] md: NULL pointer dereference in md_handle_request() after pers->run() fails
--
commit 3549464235e3de415ce8ceb834293e195eafad7d
Author: Zhihao Cheng [off-list ref]
md: Handle pers->run failure in level_store
This commit adds error handling for the pers->run(mddev) call in
level_store(). When pers->run() fails, it stops the array via __md_stop()
and unlinks sysfs nodes before unlocking and returning.
quoted hunk ↗ jump to hunk
diff --git a/drivers/md/md.c b/drivers/md/md.c index 680b34a63cb3d..0a3252098780b 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -4229,7 +4230,33 @@ 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 (mddev->pers->sync_request && mddev->to_remove == NULL) + mddev->to_remove = &md_redundancy_group; + 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); + 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: High]
Can this trigger a NULL pointer dereference in md_handle_request() for
concurrent bios?
If pers->run() fails, __md_stop() sets mddev->pers to NULL. The error path
then jumps to out_unlock, which subsequently calls mddev_unlock_and_resume().
When the array is resumed, mddev_unlock_and_resume() resurrects active_io
and wakes up mddev->sb_wait. If there are concurrent bios submitted to the
array, they may be blocked in md_handle_request() waiting on sb_wait:
drivers/md/md.c:md_handle_request() {
...
if (is_suspended(mddev, bio)) {
/* Bail out if REQ_NOWAIT is set for the bio */
if (bio->bi_opf & REQ_NOWAIT) {
bio_wouldblock_error(bio);
return true;
}
wait_event(mddev->sb_wait, !is_suspended(mddev, bio));
}
if (!percpu_ref_tryget_live(&mddev->active_io))
goto check_suspended;
}
if (!mddev->pers->make_request(mddev, bio)) {
...
}
Once woken up, they will re-acquire active_io and unconditionally dereference
mddev->pers->make_request. Since mddev->pers was set to NULL by __md_stop(),
will this result in a kernel panic?
+ } set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags); if (!mddev->thread) md_update_sb(mddev, 1);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260923112159.94175-1-chengzhihao1@huawei.com?part=2