[PATCH v9 26/29] scsi: sd: support multipath disk
sashiko-bot at kernel.org
sashiko-bot at kernel.org
Sun Nov 8 13:23:51 PST 2026
Thank you for your contribution! Sashiko AI review found 7 potential
issues to consider:
- [High] scsi: sd: premature sd_mpath_remove() before del_gendisk() in
sd_remove()
- [High] scsi: sd: broken revalidation and state sync in
sd_mpath_revalidate_head()
- [Medium] scsi: sd: clear current_path after mpath_delete_device() in
sd_mpath_remove()
- [Medium] scsi: sd: premature sd_mpath_disk list unlink races with
sd_mpath_probe()
- [Medium] scsi: sd: uncancelled remove_work in sd_mpath_fail_probe()
causes UAF
- [Medium] scsi: sd: parent sd_mpath_disk->dev deleted before child
mpath_head->disk
- [Low] scsi: sd: unsigned sdkp->index makes >= 0 check always true
--
commit a00d9c64612f4369c0c2dd5d2bf64b95ed52be50
Author: John Garry <john.garry at linux.dev>
scsi: sd: support multipath disk
This commit adds support for attaching and managing SCSI multipath disks in
the sd driver, creating a multipath head gendisk while keeping per-path
gendisks hidden.
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index fb87b1663ad62..ab4ce6c2a8dd2 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
[ ... ]
> @@ -817,7 +835,8 @@ static void scsi_disk_release(struct device *dev)
> {
> struct scsi_disk *sdkp = to_scsi_disk(dev);
>
> - ida_free(&sd_index_ida, sdkp->index);
> + if (sdkp->index >= 0)
> + ida_free(&sd_index_ida, sdkp->index);
> put_device(&sdkp->device->sdev_gendev);
> free_opal_dev(sdkp->opal_dev);
>
> @@ -4021,6 +4040,90 @@ static int sd_format_disk_name(char *prefix, int index, char *buf, int buflen)
[Severity: Low]
Is sdkp->index >= 0 always true here?
struct scsi_disk declares index as u32 in drivers/scsi/sd.h, while
sd_mpath_probe() sets sdkp->index = -1. This triggers a -Wtype-limits
warning under W=1 and still calls ida_free(&sd_index_ida, 0xffffffffU) on
multipath per-path disks. Changing struct scsi_disk::index to int matches
sd_mpath_disk::disk_index and local index in sd_probe().
> return 0;
> }
>
> +#ifdef CONFIG_SCSI_MULTIPATH
> +static int sd_mpath_revalidate_head(struct scsi_disk *sdkp)
> +{
> + struct sd_mpath_disk *sd_mpath_disk = sdkp->sd_mpath_disk;
> + struct scsi_mpath_head *scsi_mpath_head = sd_mpath_disk->scsi_mpath_head;
> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> + struct gendisk *disk = mpath_head->disk;
> + struct queue_limits *sdkp_lim = &sdkp->disk->queue->limits;
> + struct queue_limits lim;
> + unsigned int memflags;
> + int ret;
> +
> + lim = queue_limits_start_update(disk->queue);
> + memflags = blk_mq_freeze_queue(disk->queue);
> +
> + lim.logical_block_size = sdkp_lim->logical_block_size;
> + lim.physical_block_size = sdkp_lim->physical_block_size;
> + lim.io_min = sdkp_lim->io_min;
> + lim.io_opt = sdkp_lim->io_opt;
> +
> + queue_limits_stack_bdev(&lim, sdkp->disk->part0, 0,
> + disk->disk_name);
> +
> + /* TODO: setup integrity and zoned limits */
> + lim.max_write_streams = sdkp_lim->max_write_streams;
> + lim.write_stream_granularity = sdkp_lim->write_stream_granularity;
> + ret = queue_limits_commit_update(disk->queue, &lim);
> +
> + set_capacity_and_notify(disk, get_capacity(sdkp->disk));
> +
> + mpath_revalidate_paths(mpath_head);
> +
> + blk_mq_unfreeze_queue(disk->queue, memflags);
> +
> + return ret;
> +}
[Severity: High]
Can probing a standby ALUA path overwrite the multipath head disk capacity
with 0?
On ALUA arrays, standby or unavailable paths succeed INQUIRY VPD page 0x83
and attach to scsi_mpath_head, but fail READ CAPACITY in
sd_revalidate_disk(), leaving get_capacity(sdkp->disk) at 0. Calling
sd_mpath_revalidate_head() unconditionally from sd_probe() sets the live
mpath_head->disk capacity to 0, causing subsequent I/O on the multipath disk
to fail bio_check_eod() with -EIO.
In addition, sd_mpath_revalidate_head() has several other state sync issues:
- It is only called from sd_probe() and never from sd_revalidate_disk() or
sd_rescan(), so LUN resizes or standby-to-active transitions do not update
mpath_head->disk.
- It never calls set_disk_ro(disk, sdkp->write_prot), leaving write-protected
multipath disks openable for writes via mpath_ops.
- When queue_limits_commit_update() fails, sd_mpath_revalidate_head() still
updates capacity and paths, and sd_probe() only logs a warning before
calling sd_mpath_add_disk(sdkp) with uncommitted queue limits.
- For TYPE_ZBC disks, queue_limits_stack_bdev() inherits BLK_FEAT_ZONED onto
mpath_head->disk->queue even though zoned multipath support is not set up.
[ ... ]
> @@ -4050,13 +4153,260 @@ static const struct attribute_group sd_mpath_disk_attr_group = {
[ ... ]
> +static void sd_mpath_remove(struct scsi_disk *sdkp)
> +{
> + struct sd_mpath_disk *sd_mpath_disk = sdkp->sd_mpath_disk;
> + struct scsi_device *sdp = sdkp->device;
> + struct scsi_mpath_device *scsi_mpath_dev = sdp->scsi_mpath_dev;
> + struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
> + struct scsi_mpath_head *scsi_mpath_head = sd_mpath_disk->scsi_mpath_head;
> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> + bool remove = false;
> +
> + mpath_synchronize(mpath_head);
> +
> + if (mpath_clear_current_path(mpath_device))
> + mpath_synchronize(mpath_head);
> +
> + mpath_delete_device(mpath_device);
[Severity: Medium]
Can mpath_device be re-cached into mpath_head->current_path[node] when
mpath_clear_current_path() runs before mpath_delete_device()?
When sd_remove() is triggered by driver unbind or module unload,
sdev->sdev_state remains SDEV_RUNNING, so scsi_mpath_is_disabled() returns
false. Because mpath_device is still on mpath_head->dev_list until
mpath_delete_device() unlinks it, a concurrent path lookup can repopulate
current_path[node]:
CPU0 CPU1
sd_mpath_remove()
mpath_clear_current_path(mpath_device)
mpath_bdev_submit_bio()
mpath_find_path()
__mpath_find_path()
rcu_assign_pointer(
current_path[node],
mpath_device)
mpath_delete_device(mpath_device)
...
mpath_device->disk = NULL;
mpath_bdev_submit_bio()
mpath_find_path()
/* returns cached mpath_device */
mpath_device->disk->part0
<-- NULL pointer dereference
Calling mpath_delete_device(mpath_device) before mpath_clear_current_path()
prevents __mpath_find_path() from finding mpath_device on dev_list after
current_path is cleared.
> +
> + mutex_lock(&sd_mpath_disks_lock);
> + sd_mpath_disk->disk_count--;
> + if (!sd_mpath_disk->disk_count && mpath_can_remove_head(mpath_head)) {
> + list_del_init(&sd_mpath_disk->entry);
> + remove = true;
> + }
> + mutex_unlock(&sd_mpath_disks_lock);
[Severity: Medium]
Can unlinking sd_mpath_disk and dropping sd_mpath_disks_lock before teardown
completes race with a concurrent sd_mpath_probe() for the same
scsi_mpath_head?
In sd_mpath_remove(), sd_mpath_remove_head(), and sd_mpath_fail_probe(),
sd_mpath_disk is removed from sd_mpath_disks_list and sd_mpath_disks_lock is
released before device_del(), mpath_remove_disk(), and sd_mpath_put_disk()
finish tearing down the shared mpath_head:
CPU0 CPU1
sd_mpath_remove()
mutex_lock(&sd_mpath_disks_lock)
list_del_init(&sd_mpath_disk->entry)
mutex_unlock(&sd_mpath_disks_lock)
sd_mpath_probe()
mutex_lock(&sd_mpath_disks_lock)
sd_mpath_find_disk() -> NULL
mpath_alloc_head_disk(mpath_head)
/* fails with -EINVAL, or races
* with old sd_mpath_disk_release()
* calling mpath_put_disk() */
device_del(&sd_mpath_disk->dev)
mpath_remove_disk(mpath_head)
sd_mpath_put_disk(sd_mpath_disk)
Depending on timing, mpath_alloc_head_disk() fails with -EINVAL because
mpath_head->disk is still non-NULL, device_add() fails with -EEXIST on the
duplicate "scsi_mpath_disk%d" name, or a delayed sd_mpath_disk_release()
calls mpath_put_disk(mpath_head) after the new probe has allocated a new
mpath_head->disk.
> + mpath_remove_sysfs_link(mpath_device);
> + mpath_device->disk = NULL;
> +
> + if (remove) {
> + device_del(&sd_mpath_disk->dev);
> + mpath_remove_disk(mpath_head);
> + }
> + sd_mpath_put_disk(sd_mpath_disk);
> +}
[Severity: Medium]
Does calling device_del(&sd_mpath_disk->dev) before
mpath_remove_disk(mpath_head) delete the parent device before its child
gendisk?
sd_mpath_probe() sets mpath_head->parent = &sd_mpath_disk->dev, and
mpath_device_set_live() registers mpath_head->disk as a child of that
device. In sd_mpath_remove(), sd_mpath_remove_head(), and
sd_mpath_fail_probe(), calling device_del(&sd_mpath_disk->dev) first clears
sd_mpath_disk->dev.kobj.parent and tears down the parent sysfs directory
while mpath_head->disk is still registered, causing del_gendisk() inside
mpath_remove_disk() to emit child KOBJ_REMOVE uevents with truncated
DEVPATHs. Also, in sd_mpath_remove(), mpath_remove_sysfs_link(mpath_device)
runs after dropping sd_mpath_disks_lock, so a concurrent last-path removal
or sd_mpath_remove_head() can remove mpath_head->disk while
mpath_remove_sysfs_link() is still accessing disk_to_dev(mpath_head->disk).
[ ... ]
> +static void sd_mpath_fail_probe(struct scsi_disk *sdkp)
> +{
> + struct sd_mpath_disk *sd_mpath_disk = sdkp->sd_mpath_disk;
> + struct scsi_mpath_device *scsi_mpath_dev;
> + struct mpath_device *mpath_device;
> + struct scsi_device *sdp = sdkp->device;
> + struct scsi_mpath_head *scsi_mpath_head;
> + struct mpath_head *mpath_head;
> + bool remove = false;
> +
> + if (!sd_mpath_disk)
> + return;
> +
> + scsi_mpath_dev = sdp->scsi_mpath_dev;
> + mpath_device = &scsi_mpath_dev->mpath_device;
> + scsi_mpath_head = sd_mpath_disk->scsi_mpath_head;
> + mpath_head = &scsi_mpath_head->mpath_head;
> +
> + mutex_lock(&sd_mpath_disks_lock);
> + sd_mpath_disk->disk_count--;
> + if (!sd_mpath_disk->disk_count) {
> + list_del_init(&sd_mpath_disk->entry);
> + remove = true;
> + }
> + mutex_unlock(&sd_mpath_disks_lock);
> + mpath_device->disk = NULL;
> +
> + if (remove) {
> + device_del(&sd_mpath_disk->dev);
> + mpath_remove_disk(mpath_head);
> + }
> + sd_mpath_put_disk(sd_mpath_disk);
> +}
[Severity: Medium]
Can sd_mpath_fail_probe() free scsi_mpath_head while mpath_head->remove_work
is still pending?
When delayed_removal_secs > 0 and the last path is removed in
sd_mpath_remove(), mpath_can_remove_head() schedules mpath_head->remove_work
and leaves sd_mpath_disk on sd_mpath_disks_list with disk_count == 0. If a
new path starts probing before remove_work fires, sd_mpath_probe() increments
disk_count to 1, but remove_work is not cancelled until mpath_add_device() in
sd_mpath_add_disk(). If sd_probe() fails earlier and calls
sd_mpath_fail_probe(), sd_mpath_disk is torn down without cancelling
remove_work:
CPU0 kworker
sd_mpath_remove() (last path)
mpath_can_remove_head()
mod_delayed_work(...,
&mpath_head->remove_work, ...)
sd_probe() (new path)
sd_mpath_probe()
sd_mpath_disk->disk_count++
device_add_disk() -> fails
sd_mpath_fail_probe()
sd_mpath_put_disk()
scsi_mpath_put_head()
scsi_mpath_remove_device()
scsi_mpath_put_head()
kfree(scsi_mpath_head)
mpath_remove_head_work()
sd_mpath_remove_head()
<-- use-after-free
[ ... ]
> @@ -4348,6 +4728,9 @@ static void sd_remove(struct scsi_device *sdp)
> if (scsi_autopm_get_device(sdkp->device) < 0)
> blk_mark_disk_dead(sdkp->disk);
>
> + if (sdp->scsi_mpath_dev)
> + sd_mpath_remove(sdkp);
> +
> device_del(&sdkp->disk_dev);
> del_gendisk(sdkp->disk);
> if (!sdkp->suspended)
[Severity: High]
Can calling sd_mpath_remove() before del_gendisk(sdkp->disk) cause a NULL
pointer dereference or use-after-free while REQ_MPATH requests are still
queued on sdkp->disk->queue?
Cloned multipath bios submitted to sdkp->disk->part0 remain queued or in
flight on sdkp->disk->queue until del_gendisk(sdkp->disk) freezes and drains
that queue. When the last path is removed, sd_mpath_remove() drops the last
reference to sd_mpath_disk->dev, and sd_mpath_disk_release() calls
mpath_put_disk(mpath_head) (setting mpath_head->disk = NULL) and frees
sd_mpath_disk before del_gendisk(sdkp->disk) runs:
CPU0 CPU1
sd_remove()
sd_mpath_remove(sdkp)
mpath_remove_disk(mpath_head)
sd_mpath_put_disk(sd_mpath_disk)
sd_mpath_disk_release()
mpath_put_disk(mpath_head)
mpath_head->disk = NULL;
kfree(sd_mpath_disk)
sd_init_command()
scsi_mpath_start_request(req)
disk = mpath_head->disk;
blk_queue_io_stat(disk->queue)
<-- NULL pointer dereference
del_gendisk(sdkp->disk)
Completing REQ_MPATH requests in scsi_mpath_end_request() or
scsi_mpath_clone_end_io() also access the released mpath_head->disk or
requeue work after mpath_put_disk() has already flushed it. Furthermore,
because __scsi_remove_device() still holds sdev->scsi_mpath_dev's reference
on scsi_mpath_head until after sd_remove() returns, scsi_mpath_put_head() in
sd_mpath_disk_release() does not wait in mpath_head_uninit() when the
multipath disk is still open, freeing sd_mpath_disk while mpath_head->parent
still points to &sd_mpath_disk->dev (used by dev_warn_ratelimited() in
mpath_bdev_submit_bio()).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261108204733.557995-1-john.garry@linux.dev?part=26
More information about the Linux-nvme
mailing list