* [PATCH v3 01/17] scsi-multipath: introduce basic SCSI device support
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 10:53 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 02/17] scsi-multipath: introduce scsi_device head structure John Garry
` (15 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
For a scsi_device to support multipath, introduce structure
scsi_mpath_device to hold multipath-specific details.
Like NS structure for NVME, scsi_mpath_device holds the mpath_device
structure to device management and path selection.
A module param are introduced to enable multipath - the following modes
are available:
- on
- off
- always
SCSI multipath will only be available until the following conditions:
- scsi_multipath enabled and ALUA supported and unique ID available in
VPD page 83.
- scsi_multipath always mode and unique ID available in VPD page 83
The scsi_device structure contains a pointer to scsi_mpath_device; having
this pointer set or unset indicates whether multipath is enabled or
disabled for the scsi_device.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/Kconfig | 10 +++
drivers/scsi/Makefile | 1 +
drivers/scsi/scsi.c | 8 +-
drivers/scsi/scsi_multipath.c | 146 ++++++++++++++++++++++++++++++++++
drivers/scsi/scsi_scan.c | 4 +
drivers/scsi/scsi_sysfs.c | 2 +
include/scsi/scsi_device.h | 2 +
include/scsi/scsi_multipath.h | 55 +++++++++++++
8 files changed, 227 insertions(+), 1 deletion(-)
create mode 100644 drivers/scsi/scsi_multipath.c
create mode 100644 include/scsi/scsi_multipath.h
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index c3042393af234..d6c31df454825 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -76,6 +76,16 @@ config SCSI_LIB_KUNIT_TEST
If unsure say N.
+config SCSI_MULTIPATH
+ bool "SCSI multipath support (EXPERIMENTAL)"
+ depends on SCSI_MOD
+ select LIBMULTIPATH
+ help
+ This option enables support for native SCSI multipath support for
+ SCSI host.
+
+ If unsure say N.
+
comment "SCSI support type (disk, tape, CD-ROM)"
depends on SCSI
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 842c254bb2269..04f9c2e51fcbd 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -168,6 +168,7 @@ scsi_mod-y += scsi_trace.o scsi_logging.o
scsi_mod-$(CONFIG_PM) += scsi_pm.o
scsi_mod-$(CONFIG_SCSI_DH) += scsi_dh.o
scsi_mod-$(CONFIG_BLK_DEV_BSG) += scsi_bsg.o
+scsi_mod-$(CONFIG_SCSI_MULTIPATH) += scsi_multipath.o
hv_storvsc-y := storvsc_drv.o
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 76cdad063f7bc..70aa1dbeacebf 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -64,6 +64,7 @@
#include <scsi/scsi_driver.h>
#include <scsi/scsi_eh.h>
#include <scsi/scsi_host.h>
+#include <scsi/scsi_multipath.h>
#include <scsi/scsi_tcq.h>
#include "scsi_priv.h"
@@ -1042,12 +1043,16 @@ static int __init init_scsi(void)
error = scsi_sysfs_register();
if (error)
goto cleanup_sysctl;
+ error = scsi_multipath_init();
+ if (error)
+ goto cleanup_sysfs;
scsi_netlink_init();
printk(KERN_NOTICE "SCSI subsystem initialized\n");
return 0;
-
+cleanup_sysfs:
+ scsi_sysfs_unregister();
cleanup_sysctl:
scsi_exit_sysctl();
cleanup_hosts:
@@ -1066,6 +1071,7 @@ static int __init init_scsi(void)
static void __exit exit_scsi(void)
{
scsi_netlink_exit();
+ scsi_multipath_exit();
scsi_sysfs_unregister();
scsi_exit_sysctl();
scsi_exit_hosts();
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
new file mode 100644
index 0000000000000..ff37cfdf2f9d1
--- /dev/null
+++ b/drivers/scsi/scsi_multipath.c
@@ -0,0 +1,146 @@
+// SPDX-License-Indentifier: GPL-2.0
+/*
+ * Copyright (c) 2026 Oracle Corp
+ *
+ */
+
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_driver.h>
+#include <scsi/scsi_proto.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_multipath.h>
+
+#include "scsi_priv.h"
+
+enum {
+ SCSI_MULTIPATH_OFF,
+ SCSI_MULTIPATH_ON,
+ SCSI_MULTIPATH_ALWAYS,
+};
+
+static const char *scsi_multipath_modes[] = {
+ [SCSI_MULTIPATH_OFF] = "off",
+ [SCSI_MULTIPATH_ON] = "on",
+ [SCSI_MULTIPATH_ALWAYS] = "always",
+};
+
+static int scsi_multipath = SCSI_MULTIPATH_OFF;
+
+static int scsi_multipath_param_set(const char *val, const struct kernel_param *kp)
+{
+ if (!val)
+ return -EINVAL;
+ if (!strncmp(val, "on", 2))
+ scsi_multipath = SCSI_MULTIPATH_ON;
+ else if (!strncmp(val, "always", 6))
+ scsi_multipath = SCSI_MULTIPATH_ALWAYS;
+ else if (!strncmp(val, "off", 3))
+ scsi_multipath = SCSI_MULTIPATH_OFF;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
+static int scsi_multipath_param_get(char *buf, const struct kernel_param *kp)
+{
+ return sprintf(buf, "%s\n", scsi_multipath_modes[scsi_multipath]);
+}
+
+static const struct kernel_param_ops multipath_param_ops = {
+ .set = scsi_multipath_param_set,
+ .get = scsi_multipath_param_get,
+};
+
+module_param_cb(multipath, &multipath_param_ops, &scsi_multipath, 0444);
+MODULE_PARM_DESC(multipath, "turn on native multipath support, options: on, off, always");
+
+static int scsi_mpath_unique_lun_id(struct scsi_device *sdev)
+{
+ struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
+ int ret;
+
+ ret = scsi_vpd_lun_id(sdev, scsi_mpath_dev->device_id_str,
+ SCSI_MPATH_DEVICE_ID_LEN);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int scsi_multipath_sdev_init(struct scsi_device *sdev)
+{
+ struct Scsi_Host *shost = sdev->host;
+ struct scsi_mpath_device *scsi_mpath_dev;
+ struct mpath_device *mpath_device;
+
+ scsi_mpath_dev = kzalloc(sizeof(*scsi_mpath_dev), GFP_KERNEL);
+ if (!scsi_mpath_dev)
+ return -ENOMEM;
+ scsi_mpath_dev->sdev = sdev;
+ sdev->scsi_mpath_dev = scsi_mpath_dev;
+
+ mpath_device = &scsi_mpath_dev->mpath_device;
+ mpath_device->numa_node = dev_to_node(shost->dma_dev);
+ mpath_device->access_state = MPATH_STATE_OPTIMIZED;
+
+ return 0;
+}
+
+static void scsi_multipath_sdev_uninit(struct scsi_device *sdev)
+{
+ kfree(sdev->scsi_mpath_dev);
+ sdev->scsi_mpath_dev = NULL;
+}
+
+int scsi_mpath_dev_alloc(struct scsi_device *sdev)
+{
+ int ret;
+
+ if (scsi_multipath == SCSI_MULTIPATH_OFF)
+ return 0;
+
+ if (!scsi_device_tpgs(sdev) && (scsi_multipath != SCSI_MULTIPATH_ALWAYS)) {
+ sdev_printk(KERN_DEBUG, sdev, "IMPLICIT TPGS are required for multipath support\n");
+ return 0;
+ }
+
+ ret = scsi_multipath_sdev_init(sdev);
+ if (ret)
+ return ret;
+
+ ret = scsi_mpath_unique_lun_id(sdev);
+ if (ret < 0) {
+ ret = 0;
+ goto out_uninit;
+ }
+
+ return 0;
+
+out_uninit:
+ scsi_multipath_sdev_uninit(sdev);
+ return ret;
+}
+
+void scsi_mpath_dev_release(struct scsi_device *sdev)
+{
+ struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
+
+ if (!scsi_mpath_dev)
+ return;
+
+ scsi_multipath_sdev_uninit(sdev);
+}
+
+int __init scsi_multipath_init(void)
+{
+ return 0;
+}
+
+void __exit scsi_multipath_exit(void)
+{
+}
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("scsi_multipath");
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e27da038603a2..3f1ac302719c2 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -46,6 +46,7 @@
#include <scsi/scsi_transport.h>
#include <scsi/scsi_dh.h>
#include <scsi/scsi_eh.h>
+#include <scsi/scsi_multipath.h>
#include "scsi_priv.h"
#include "scsi_logging.h"
@@ -1130,6 +1131,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
sdev->max_queue_depth = sdev->queue_depth;
WARN_ON_ONCE(sdev->max_queue_depth > sdev->budget_map.depth);
+ if (scsi_mpath_dev_alloc(sdev))
+ return SCSI_SCAN_NO_RESPONSE;
+
/*
* Ok, the device is now all set up, we can
* register it and tell the rest of the kernel
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dfc3559e7e04f..2f80d703ce640 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -23,6 +23,7 @@
#include <scsi/scsi_transport.h>
#include <scsi/scsi_driver.h>
#include <scsi/scsi_devinfo.h>
+#include <scsi/scsi_multipath.h>
#include "scsi_priv.h"
#include "scsi_logging.h"
@@ -455,6 +456,7 @@ static void scsi_device_dev_release(struct device *dev)
might_sleep();
scsi_dh_release_device(sdev);
+ scsi_mpath_dev_release(sdev);
parent = sdev->sdev_gendev.parent;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 029f5115b2ea0..40a435e66b34e 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -280,6 +280,8 @@ struct scsi_device {
struct device sdev_gendev,
sdev_dev;
+ struct scsi_mpath_device *scsi_mpath_dev;
+
struct work_struct requeue_work;
struct scsi_device_handler *handler;
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
new file mode 100644
index 0000000000000..d3d410dafd17a
--- /dev/null
+++ b/include/scsi/scsi_multipath.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _SCSI_SCSI_MULTIPATH_H
+#define _SCSI_SCSI_MULTIPATH_H
+
+#include <linux/list.h>
+#include <linux/types.h>
+#include <linux/rcupdate.h>
+#include <linux/workqueue.h>
+#include <linux/mutex.h>
+#include <linux/blk-mq.h>
+#include <linux/multipath.h>
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_dbg.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_devinfo.h>
+#include <scsi/scsi_driver.h>
+
+#ifdef CONFIG_SCSI_MULTIPATH
+#define SCSI_MPATH_DEVICE_ID_LEN 256
+
+struct scsi_mpath_device {
+ struct mpath_device mpath_device;
+ struct scsi_device *sdev;
+
+ char device_id_str[SCSI_MPATH_DEVICE_ID_LEN];
+};
+#define to_scsi_mpath_device(d) \
+ container_of(d, struct scsi_mpath_device, mpath_device)
+
+int scsi_mpath_dev_alloc(struct scsi_device *sdev);
+void scsi_mpath_dev_release(struct scsi_device *sdev);
+int scsi_multipath_init(void);
+void scsi_multipath_exit(void);
+#else /* CONFIG_SCSI_MULTIPATH */
+
+struct scsi_mpath_device {
+};
+
+static inline int scsi_mpath_dev_alloc(struct scsi_device *sdev)
+{
+ return 0;
+}
+static inline void scsi_mpath_dev_release(struct scsi_device *sdev)
+{
+}
+static inline int scsi_multipath_init(void)
+{
+ return 0;
+}
+static inline void scsi_multipath_exit(void)
+{
+}
+#endif /* CONFIG_SCSI_MULTIPATH */
+#endif /* _SCSI_SCSI_MULTIPATH_H */
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 01/17] scsi-multipath: introduce basic SCSI device support
2026-07-03 10:33 ` [PATCH v3 01/17] scsi-multipath: introduce basic SCSI device support John Garry
@ 2026-07-03 10:53 ` sashiko-bot
2026-07-06 14:39 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 10:53 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The module parameter parsing logic loosely matches string prefixes instead of exact values, mistakenly accepting unintended input.
- [Low] The Kconfig symbol SCSI_MULTIPATH selects a non-existent symbol LIBMULTIPATH.
- [Low] The patch includes a non-existent header <linux/multipath.h> and references undefined structures, unconditionally breaking the build.
- [Low] The debug log message incorrectly states that 'IMPLICIT TPGS' is required, while the code accepts any ALUA TPGS support.
--
commit ad9f5e786299d2bbf9325dfe336051204d3f7369
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: introduce basic SCSI device support
For a scsi_device to support multipath, introduce structure
scsi_mpath_device to hold multipath-specific details.
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index c3042393af234..d6c31df454825 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
[ ... ]
> @@ -76,6 +76,16 @@ config SCSI_LIB_KUNIT_TEST
>
> If unsure say N.
>
> +config SCSI_MULTIPATH
> + bool "SCSI multipath support (EXPERIMENTAL)"
> + depends on SCSI_MOD
> + select LIBMULTIPATH
[Severity: Low]
Does the config symbol LIBMULTIPATH exist in the tree? Selecting a
non-existent symbol causes unmet dependency warnings and breaks the
dependency inheritance chain.
> + help
> + This option enables support for native SCSI multipath support for
> + SCSI host.
> +
> + If unsure say N.
> +
> comment "SCSI support type (disk, tape, CD-ROM)"
> depends on SCSI
[ ... ]
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> new file mode 100644
> index 0000000000000..ff37cfdf2f9d1
> --- /dev/null
> +++ b/drivers/scsi/scsi_multipath.c
> @@ -0,0 +1,146 @@
[ ... ]
> +static int scsi_multipath_param_set(const char *val, const struct kernel_param *kp)
> +{
> + if (!val)
> + return -EINVAL;
> + if (!strncmp(val, "on", 2))
> + scsi_multipath = SCSI_MULTIPATH_ON;
> + else if (!strncmp(val, "always", 6))
> + scsi_multipath = SCSI_MULTIPATH_ALWAYS;
> + else if (!strncmp(val, "off", 3))
> + scsi_multipath = SCSI_MULTIPATH_OFF;
[Severity: Medium]
Since scsi_multipath_param_set() uses strncmp() with exact lengths, won't it
silently accept strings like "once" or "office" instead of returning -EINVAL?
Should this use sysfs_streq() or strcmp() to ensure exact matches?
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
[ ... ]
> +int scsi_mpath_dev_alloc(struct scsi_device *sdev)
> +{
> + int ret;
> +
> + if (scsi_multipath == SCSI_MULTIPATH_OFF)
> + return 0;
> +
> + if (!scsi_device_tpgs(sdev) && (scsi_multipath != SCSI_MULTIPATH_ALWAYS)) {
> + sdev_printk(KERN_DEBUG, sdev, "IMPLICIT TPGS are required for multipath support\n");
[Severity: Low]
This isn't a bug, but the log message says IMPLICIT TPGS is required, while
the check for scsi_device_tpgs(sdev) accepts any TPGS support (implicit,
explicit, or both). Could the message be updated to match the check?
> + return 0;
> + }
> +
> + ret = scsi_multipath_sdev_init(sdev);
> + if (ret)
> + return ret;
[ ... ]
> diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
> new file mode 100644
> index 0000000000000..d3d410dafd17a
> --- /dev/null
> +++ b/include/scsi/scsi_multipath.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _SCSI_SCSI_MULTIPATH_H
> +#define _SCSI_SCSI_MULTIPATH_H
> +
> +#include <linux/list.h>
> +#include <linux/types.h>
> +#include <linux/rcupdate.h>
> +#include <linux/workqueue.h>
> +#include <linux/mutex.h>
> +#include <linux/blk-mq.h>
> +#include <linux/multipath.h>
[Severity: Low]
Does linux/multipath.h exist in the tree? Including a non-existent header
will unconditionally break the build for any file including this header.
> +#include <scsi/scsi.h>
> +#include <scsi/scsi_cmnd.h>
> +#include <scsi/scsi_dbg.h>
> +#include <scsi/scsi_device.h>
> +#include <scsi/scsi_devinfo.h>
> +#include <scsi/scsi_driver.h>
> +
> +#ifdef CONFIG_SCSI_MULTIPATH
> +#define SCSI_MPATH_DEVICE_ID_LEN 256
> +
> +struct scsi_mpath_device {
> + struct mpath_device mpath_device;
[Severity: Low]
Because linux/multipath.h is missing, struct mpath_device is undefined,
which results in an incomplete type error when CONFIG_SCSI_MULTIPATH is
enabled. Is there a missing patch series this depends on?
> + struct scsi_device *sdev;
> +
> + char device_id_str[SCSI_MPATH_DEVICE_ID_LEN];
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=1
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 01/17] scsi-multipath: introduce basic SCSI device support
2026-07-03 10:53 ` sashiko-bot
@ 2026-07-06 14:39 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 14:39 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 11:53, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [Medium] The module parameter parsing logic loosely matches string prefixes instead of exact values, mistakenly accepting unintended input.
> - [Low] The Kconfig symbol SCSI_MULTIPATH selects a non-existent symbol LIBMULTIPATH.
> - [Low] The patch includes a non-existent header <linux/multipath.h> and references undefined structures, unconditionally breaking the build.
> - [Low] The debug log message incorrectly states that 'IMPLICIT TPGS' is required, while the code accepts any ALUA TPGS support.
> --
>
I will address all below.
> commit ad9f5e786299d2bbf9325dfe336051204d3f7369
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi-multipath: introduce basic SCSI device support
>
> For a scsi_device to support multipath, introduce structure
> scsi_mpath_device to hold multipath-specific details.
>
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index c3042393af234..d6c31df454825 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
> [ ... ]
>> @@ -76,6 +76,16 @@ config SCSI_LIB_KUNIT_TEST
>>
>> If unsure say N.
>>
>> +config SCSI_MULTIPATH
>> + bool "SCSI multipath support (EXPERIMENTAL)"
>> + depends on SCSI_MOD
>> + select LIBMULTIPATH
>
> [Severity: Low]
> Does the config symbol LIBMULTIPATH exist in the tree? Selecting a
> non-existent symbol causes unmet dependency warnings and breaks the
> dependency inheritance chain.
>
>> + help
>> + This option enables support for native SCSI multipath support for
>> + SCSI host.
>> +
>> + If unsure say N.
>> +
>> comment "SCSI support type (disk, tape, CD-ROM)"
>> depends on SCSI
>
> [ ... ]
>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> new file mode 100644
>> index 0000000000000..ff37cfdf2f9d1
>> --- /dev/null
>> +++ b/drivers/scsi/scsi_multipath.c
>> @@ -0,0 +1,146 @@
> [ ... ]
>> +static int scsi_multipath_param_set(const char *val, const struct kernel_param *kp)
>> +{
>> + if (!val)
>> + return -EINVAL;
>> + if (!strncmp(val, "on", 2))
>> + scsi_multipath = SCSI_MULTIPATH_ON;
>> + else if (!strncmp(val, "always", 6))
>> + scsi_multipath = SCSI_MULTIPATH_ALWAYS;
>> + else if (!strncmp(val, "off", 3))
>> + scsi_multipath = SCSI_MULTIPATH_OFF;
>
> [Severity: Medium]
> Since scsi_multipath_param_set() uses strncmp() with exact lengths, won't it
> silently accept strings like "once" or "office" instead of returning -EINVAL?
> Should this use sysfs_streq() or strcmp() to ensure exact matches?
>
>> + else
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>
> [ ... ]
>
>> +int scsi_mpath_dev_alloc(struct scsi_device *sdev)
>> +{
>> + int ret;
>> +
>> + if (scsi_multipath == SCSI_MULTIPATH_OFF)
>> + return 0;
>> +
>> + if (!scsi_device_tpgs(sdev) && (scsi_multipath != SCSI_MULTIPATH_ALWAYS)) {
>> + sdev_printk(KERN_DEBUG, sdev, "IMPLICIT TPGS are required for multipath support\n");
>
> [Severity: Low]
> This isn't a bug, but the log message says IMPLICIT TPGS is required, while
> the check for scsi_device_tpgs(sdev) accepts any TPGS support (implicit,
> explicit, or both). Could the message be updated to match the check?
>
>> + return 0;
>> + }
>> +
>> + ret = scsi_multipath_sdev_init(sdev);
>> + if (ret)
>> + return ret;
>
> [ ... ]
>
>> diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
>> new file mode 100644
>> index 0000000000000..d3d410dafd17a
>> --- /dev/null
>> +++ b/include/scsi/scsi_multipath.h
>> @@ -0,0 +1,55 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _SCSI_SCSI_MULTIPATH_H
>> +#define _SCSI_SCSI_MULTIPATH_H
>> +
>> +#include <linux/list.h>
>> +#include <linux/types.h>
>> +#include <linux/rcupdate.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/mutex.h>
>> +#include <linux/blk-mq.h>
>> +#include <linux/multipath.h>
>
> [Severity: Low]
> Does linux/multipath.h exist in the tree? Including a non-existent header
> will unconditionally break the build for any file including this header.
>
>> +#include <scsi/scsi.h>
>> +#include <scsi/scsi_cmnd.h>
>> +#include <scsi/scsi_dbg.h>
>> +#include <scsi/scsi_device.h>
>> +#include <scsi/scsi_devinfo.h>
>> +#include <scsi/scsi_driver.h>
>> +
>> +#ifdef CONFIG_SCSI_MULTIPATH
>> +#define SCSI_MPATH_DEVICE_ID_LEN 256
>> +
>> +struct scsi_mpath_device {
>> + struct mpath_device mpath_device;
>
> [Severity: Low]
> Because linux/multipath.h is missing, struct mpath_device is undefined,
> which results in an incomplete type error when CONFIG_SCSI_MULTIPATH is
> enabled. Is there a missing patch series this depends on?
>
>> + struct scsi_device *sdev;
>> +
>> + char device_id_str[SCSI_MPATH_DEVICE_ID_LEN];
>> +};
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 02/17] scsi-multipath: introduce scsi_device head structure
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
2026-07-03 10:33 ` [PATCH v3 01/17] scsi-multipath: introduce basic SCSI device support John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 10:49 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 03/17] scsi-multipath: provide sysfs link from to scsi_device John Garry
` (14 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Introduce a scsi_device head structure - scsi_mpath_head - to manage
multipathing for a scsi_device. This is similar to nvme_ns_head structure.
There is no reference in scsi_mpath_head to any disk, as this would be
mananged by the scsi_disk driver.
A list of scsi_mpath_head structures is managed to lookup for matching
multipathed scsi_device's. Matching is done through the scsi_device
unique id.
A new class for multipathed devices is added, scsi_mpath_device_class.
The purpose of this class is for managing the scsi_mpath_head.dev member.
The naming for the scsi_device structure is in form H:C:I:L,
where H is host, C is channel, I is ID, and L is lun.
However, for a multipathed scsi_device, all the naming members may be
different between member scsi_device's. As such, just use a simple
single-number naming index for each scsi_mpath_head.
The sysfs device folder will have links to the scsi_device's so, it will
be possible to lookup the member scsi_device's.
An example sysfs entry is as follows:
# ls -l /sys/class/scsi_mpath_device/scsi_mpath_device0/
total 0
drwxr-xr-x 2 root root 0 Apr 13 15:48 power
lrwxrwxrwx 1 root root 0 Apr 13 15:48 subsystem -> ../../../../class/scsi_mpath_device
-rw-r--r-- 1 root root 4096 Apr 13 15:48 uevent
-r--r--r-- 1 root root 4096 Apr 13 15:48 vpd_id
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_multipath.c | 207 +++++++++++++++++++++++++++++++++-
drivers/scsi/scsi_sysfs.c | 3 +
include/scsi/scsi_multipath.h | 31 +++++
3 files changed, 239 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index ff37cfdf2f9d1..65ee3da5cc7fc 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -27,6 +27,10 @@ static const char *scsi_multipath_modes[] = {
static int scsi_multipath = SCSI_MULTIPATH_OFF;
+static LIST_HEAD(scsi_mpath_heads_list);
+static DEFINE_MUTEX(scsi_mpath_heads_lock);
+static DEFINE_IDA(scsi_multipath_dev_ida);
+
static int scsi_multipath_param_set(const char *val, const struct kernel_param *kp)
{
if (!val)
@@ -69,6 +73,60 @@ static int scsi_mpath_unique_lun_id(struct scsi_device *sdev)
return 0;
}
+static void scsi_mpath_head_release(struct device *dev)
+{
+ struct scsi_mpath_head *scsi_mpath_head =
+ container_of(dev, struct scsi_mpath_head, dev);
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+
+ ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
+ mpath_head_uninit(mpath_head);
+ kfree(scsi_mpath_head);
+}
+
+static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct scsi_mpath_head *scsi_mpath_head =
+ container_of(dev, struct scsi_mpath_head, dev);
+
+ return sysfs_emit(buf, "%s\n", scsi_mpath_head->vpd_id);
+}
+static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
+
+static struct attribute *scsi_mpath_device_attrs[] = {
+ &dev_attr_vpd_id.attr,
+ NULL
+};
+
+static const struct attribute_group scsi_mpath_device_attrs_group = {
+ .attrs = scsi_mpath_device_attrs,
+};
+
+static bool scsi_multipath_sysfs_group_visible(struct kobject *kobj)
+{
+ return true;
+}
+
+static bool scsi_multipath_sysfs_attr_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ return false;
+}
+DEFINE_SYSFS_GROUP_VISIBLE(scsi_multipath_sysfs)
+
+static const struct attribute_group *scsi_mpath_device_groups[] = {
+ &scsi_mpath_device_attrs_group,
+ NULL
+};
+
+static const struct class scsi_mpath_device_class = {
+ .name = "scsi_mpath_device",
+ .dev_groups = scsi_mpath_device_groups,
+ .dev_release = scsi_mpath_head_release,
+};
+
static int scsi_multipath_sdev_init(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->host;
@@ -88,6 +146,71 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
return 0;
}
+static struct mpath_head_template smpdt = {
+};
+
+static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
+{
+ struct scsi_mpath_head *scsi_mpath_head;
+ int ret;
+
+ scsi_mpath_head = kzalloc(sizeof(*scsi_mpath_head), GFP_KERNEL);
+ if (!scsi_mpath_head)
+ return NULL;
+
+ ida_init(&scsi_mpath_head->ida);
+
+ if (mpath_head_init(&scsi_mpath_head->mpath_head))
+ goto out_free;
+ scsi_mpath_head->mpath_head.mpdt = &smpdt;
+
+ scsi_mpath_head->index = ida_alloc(&scsi_multipath_dev_ida, GFP_KERNEL);
+ if (scsi_mpath_head->index < 0)
+ goto out_put_head;
+ kref_init(&scsi_mpath_head->ref);
+
+ device_initialize(&scsi_mpath_head->dev);
+ scsi_mpath_head->dev.class = &scsi_mpath_device_class;
+ ret = dev_set_name(&scsi_mpath_head->dev, "scsi_mpath_device%d",
+ scsi_mpath_head->index);
+ if (ret) {
+ put_device(&scsi_mpath_head->dev);
+ goto out_free_ida;
+ }
+
+ return scsi_mpath_head;
+
+out_free_ida:
+ ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
+out_put_head:
+ mpath_put_head(&scsi_mpath_head->mpath_head);
+out_free:
+ kfree(scsi_mpath_head);
+ return NULL;
+}
+
+static struct scsi_mpath_head *scsi_mpath_find_head(
+ struct scsi_mpath_device *scsi_mpath_dev)
+{
+ struct scsi_mpath_head *scsi_mpath_head;
+ int ret;
+
+ list_for_each_entry(scsi_mpath_head, &scsi_mpath_heads_list, entry) {
+ ret = scsi_mpath_get_head(scsi_mpath_head);
+ if (ret)
+ continue;
+ if (strncmp(scsi_mpath_head->vpd_id,
+ scsi_mpath_dev->device_id_str,
+ SCSI_MPATH_DEVICE_ID_LEN) == 0) {
+
+ return scsi_mpath_head;
+ }
+ scsi_mpath_put_head(scsi_mpath_head);
+ }
+
+ return NULL;
+}
+
static void scsi_multipath_sdev_uninit(struct scsi_device *sdev)
{
kfree(sdev->scsi_mpath_dev);
@@ -96,6 +219,7 @@ static void scsi_multipath_sdev_uninit(struct scsi_device *sdev)
int scsi_mpath_dev_alloc(struct scsi_device *sdev)
{
+ struct scsi_mpath_head *scsi_mpath_head;
int ret;
if (scsi_multipath == SCSI_MULTIPATH_OFF)
@@ -116,13 +240,58 @@ int scsi_mpath_dev_alloc(struct scsi_device *sdev)
goto out_uninit;
}
- return 0;
+ mutex_lock(&scsi_mpath_heads_lock);
+ scsi_mpath_head = scsi_mpath_find_head(sdev->scsi_mpath_dev);
+ if (scsi_mpath_head)
+ goto found;
+ scsi_mpath_head = scsi_mpath_alloc_head();
+ if (!scsi_mpath_head) {
+ sdev_printk(KERN_NOTICE, sdev, "could not allocate multipath head, device multipathing disabled\n");
+ mutex_unlock(&scsi_mpath_heads_lock);
+ goto out_uninit;
+ }
+ strscpy(scsi_mpath_head->vpd_id, sdev->scsi_mpath_dev->device_id_str,
+ SCSI_MPATH_DEVICE_ID_LEN);
+
+ ret = device_add(&scsi_mpath_head->dev);
+ if (ret) {
+ mutex_unlock(&scsi_mpath_heads_lock);
+ goto out_put_head;
+ }
+
+ list_add_tail(&scsi_mpath_head->entry, &scsi_mpath_heads_list);
+found:
+ mutex_unlock(&scsi_mpath_heads_lock);
+ ret = ida_alloc(&scsi_mpath_head->ida, GFP_KERNEL);
+ if (ret < 0)
+ goto out_put_head;
+ sdev->scsi_mpath_dev->index = ret;
+
+ sdev->scsi_mpath_dev->scsi_mpath_head = scsi_mpath_head;
+ return 0;
+out_put_head:
+ scsi_mpath_put_head(scsi_mpath_head);
out_uninit:
scsi_multipath_sdev_uninit(sdev);
return ret;
}
+static void scsi_mpath_remove_head(struct scsi_mpath_device *scsi_mpath_dev)
+{
+ scsi_mpath_put_head(scsi_mpath_dev->scsi_mpath_head);
+ scsi_mpath_dev->scsi_mpath_head = NULL;
+}
+
+void scsi_mpath_remove_device(struct scsi_mpath_device *scsi_mpath_dev)
+{
+ struct scsi_mpath_head *scsi_mpath_head = scsi_mpath_dev->scsi_mpath_head;
+
+ ida_free(&scsi_mpath_head->ida, scsi_mpath_dev->index);
+
+ scsi_mpath_remove_head(scsi_mpath_dev);
+}
+
void scsi_mpath_dev_release(struct scsi_device *sdev)
{
struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
@@ -133,13 +302,47 @@ void scsi_mpath_dev_release(struct scsi_device *sdev)
scsi_multipath_sdev_uninit(sdev);
}
-int __init scsi_multipath_init(void)
+int scsi_mpath_get_head(struct scsi_mpath_head *scsi_mpath_head)
{
+ if (!kref_get_unless_zero(&scsi_mpath_head->ref))
+ return -ENXIO;
return 0;
}
+EXPORT_SYMBOL_GPL(scsi_mpath_get_head);
+
+static void scsi_mpath_free_head(struct kref *ref)
+{
+ struct scsi_mpath_head *scsi_mpath_head =
+ container_of(ref, struct scsi_mpath_head, ref);
+
+ /*
+ * If we race with scsi_mpath_find_head(), then that function may
+ * find this scsi_mpath_head in the heads list; however we would fail
+ * to take a reference to this scsi_mpath_head and continue the search.
+ * As such, it is safe to call device_unregister (and free
+ * scsi_mpath_head) after we delete this head from the list.
+ */
+ mutex_lock(&scsi_mpath_heads_lock);
+ list_del_init(&scsi_mpath_head->entry);
+ mutex_unlock(&scsi_mpath_heads_lock);
+
+ device_unregister(&scsi_mpath_head->dev);
+}
+
+void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head)
+{
+ kref_put(&scsi_mpath_head->ref, scsi_mpath_free_head);
+}
+EXPORT_SYMBOL_GPL(scsi_mpath_put_head);
+
+int __init scsi_multipath_init(void)
+{
+ return class_register(&scsi_mpath_device_class);
+}
void __exit scsi_multipath_exit(void)
{
+ class_unregister(&scsi_mpath_device_class);
}
MODULE_LICENSE("GPL");
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 2f80d703ce640..d6bbaf424bd4a 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1501,6 +1501,9 @@ void __scsi_remove_device(struct scsi_device *sdev)
} else
put_device(&sdev->sdev_dev);
+ if (sdev->scsi_mpath_dev)
+ scsi_mpath_remove_device(sdev->scsi_mpath_dev);
+
/*
* Stop accepting new requests and wait until all queuecommand() and
* scsi_run_queue() invocations have finished before tearing down the
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
index d3d410dafd17a..50298056181f9 100644
--- a/include/scsi/scsi_multipath.h
+++ b/include/scsi/scsi_multipath.h
@@ -19,12 +19,25 @@
#ifdef CONFIG_SCSI_MULTIPATH
#define SCSI_MPATH_DEVICE_ID_LEN 256
+struct scsi_mpath_head {
+ struct mpath_head mpath_head;
+ char vpd_id[SCSI_MPATH_DEVICE_ID_LEN];
+ struct list_head entry;
+ struct ida ida;
+ struct kref ref;
+ struct device dev;
+ int index;
+};
+
struct scsi_mpath_device {
struct mpath_device mpath_device;
struct scsi_device *sdev;
+ int index;
+ struct scsi_mpath_head *scsi_mpath_head;
char device_id_str[SCSI_MPATH_DEVICE_ID_LEN];
};
+
#define to_scsi_mpath_device(d) \
container_of(d, struct scsi_mpath_device, mpath_device)
@@ -32,8 +45,13 @@ int scsi_mpath_dev_alloc(struct scsi_device *sdev);
void scsi_mpath_dev_release(struct scsi_device *sdev);
int scsi_multipath_init(void);
void scsi_multipath_exit(void);
+void scsi_mpath_remove_device(struct scsi_mpath_device *scsi_mpath_dev);
+int scsi_mpath_get_head(struct scsi_mpath_head *scsi_mpath_head);
+void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head);
#else /* CONFIG_SCSI_MULTIPATH */
+struct scsi_mpath_head {
+};
struct scsi_mpath_device {
};
@@ -51,5 +69,18 @@ static inline int scsi_multipath_init(void)
static inline void scsi_multipath_exit(void)
{
}
+static inline
+void scsi_mpath_remove_device(struct scsi_mpath_device *scsi_mpath_dev)
+{
+}
+static inline
+int scsi_mpath_get_head(struct scsi_mpath_head *scsi_mpath_head)
+{
+ return 0;
+}
+static inline
+void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head)
+{
+}
#endif /* CONFIG_SCSI_MULTIPATH */
#endif /* _SCSI_SCSI_MULTIPATH_H */
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 02/17] scsi-multipath: introduce scsi_device head structure
2026-07-03 10:33 ` [PATCH v3 02/17] scsi-multipath: introduce scsi_device head structure John Garry
@ 2026-07-03 10:49 ` sashiko-bot
2026-07-06 14:45 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 10:49 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Self-deadlock in scsi_mpath_find_head() due to kref_put() triggering release while holding scsi_mpath_heads_lock.
- [High] Double-free and use-after-free of scsi_mpath_head in the error path of scsi_mpath_alloc_head().
- [High] NULL pointer dereference and invalid device_unregister() execution when tearing down a failed device_add() in scsi_mpath_dev_alloc().
--
commit d7eb5e4fb506178f4a58d8aa264acf8230ebb5fa
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: introduce scsi_device head structure
This patch introduces a scsi_mpath_head structure to manage multipathing
for a scsi_device. It maintains a list of scsi_mpath_head structures
to look up matching multipathed devices by their unique ID. A new sysfs
device class is also added to manage the scsi_mpath_head device members.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index ff37cfdf2f9d1..65ee3da5cc7fc 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
> @@ -88,6 +146,71 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
[ ... ]
> +static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
> +{
[ ... ]
> + ret = dev_set_name(&scsi_mpath_head->dev, "scsi_mpath_device%d",
> + scsi_mpath_head->index);
> + if (ret) {
> + put_device(&scsi_mpath_head->dev);
> + goto out_free_ida;
> + }
> +
> + return scsi_mpath_head;
> +
> +out_free_ida:
> + ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
> +out_put_head:
> + mpath_put_head(&scsi_mpath_head->mpath_head);
> +out_free:
> + kfree(scsi_mpath_head);
> + return NULL;
> +}
[Severity: High]
Will this cause a double-free on the error path?
If dev_set_name() fails, put_device() is called which triggers the
scsi_mpath_head_release() callback, freeing the IDA and the structure itself.
The code then falls through to the out_free_ida label which calls ida_free()
and kfree() a second time on the already freed pointer.
> +
> +static struct scsi_mpath_head *scsi_mpath_find_head(
> + struct scsi_mpath_device *scsi_mpath_dev)
> +{
[ ... ]
> + list_for_each_entry(scsi_mpath_head, &scsi_mpath_heads_list, entry) {
> + ret = scsi_mpath_get_head(scsi_mpath_head);
> + if (ret)
> + continue;
> + if (strncmp(scsi_mpath_head->vpd_id,
> + scsi_mpath_dev->device_id_str,
> + SCSI_MPATH_DEVICE_ID_LEN) == 0) {
> +
> + return scsi_mpath_head;
> + }
> + scsi_mpath_put_head(scsi_mpath_head);
> + }
> +
> + return NULL;
> +}
[ ... ]
> @@ -116,13 +240,58 @@ int scsi_mpath_dev_alloc(struct scsi_device *sdev)
[ ... ]
> + mutex_lock(&scsi_mpath_heads_lock);
> + scsi_mpath_head = scsi_mpath_find_head(sdev->scsi_mpath_dev);
> + if (scsi_mpath_head)
> + goto found;
> + scsi_mpath_head = scsi_mpath_alloc_head();
> + if (!scsi_mpath_head) {
> + sdev_printk(KERN_NOTICE, sdev, "could not allocate multipath head, device multipathing disabled\n");
> + mutex_unlock(&scsi_mpath_heads_lock);
> + goto out_uninit;
> + }
> +
> + strscpy(scsi_mpath_head->vpd_id, sdev->scsi_mpath_dev->device_id_str,
> + SCSI_MPATH_DEVICE_ID_LEN);
> +
> + ret = device_add(&scsi_mpath_head->dev);
> + if (ret) {
> + mutex_unlock(&scsi_mpath_heads_lock);
> + goto out_put_head;
> + }
> +
> + list_add_tail(&scsi_mpath_head->entry, &scsi_mpath_heads_list);
> +found:
> + mutex_unlock(&scsi_mpath_heads_lock);
[ ... ]
> +out_put_head:
> + scsi_mpath_put_head(scsi_mpath_head);
> out_uninit:
> scsi_multipath_sdev_uninit(sdev);
> return ret;
> }
[ ... ]
> +static void scsi_mpath_free_head(struct kref *ref)
> +{
> + struct scsi_mpath_head *scsi_mpath_head =
> + container_of(ref, struct scsi_mpath_head, ref);
> +
[ ... ]
> + mutex_lock(&scsi_mpath_heads_lock);
> + list_del_init(&scsi_mpath_head->entry);
> + mutex_unlock(&scsi_mpath_heads_lock);
> +
> + device_unregister(&scsi_mpath_head->dev);
> +}
[Severity: High]
Could this lead to an AB-BA self-deadlock?
The scsi_mpath_dev_alloc() function acquires scsi_mpath_heads_lock before
calling scsi_mpath_find_head(). During iteration, scsi_mpath_find_head()
takes a temporary reference to each head.
If a concurrent thread drops its reference to a head just as we process it,
our scsi_mpath_put_head() call might drop the refcount to 0.
This would trigger scsi_mpath_free_head(), which unconditionally tries to
acquire scsi_mpath_heads_lock. Since the non-recursive mutex is already held
by the current thread in scsi_mpath_dev_alloc(), it appears this would freeze
the thread.
[Severity: High]
Is there a risk of a NULL pointer dereference and invalid device unregistration
here if device_add() fails?
If device_add() fails in scsi_mpath_dev_alloc(), we jump to out_put_head and
call scsi_mpath_put_head(). This drops the refcount to 0 and triggers this
cleanup function.
However, the scsi_mpath_head was just allocated with kzalloc and never added
to the list, so entry.next and entry.prev are NULL. Calling list_del_init()
would dereference these NULL pointers.
Furthermore, calling device_unregister() will invoke device_del() on a device
that failed device_add(), which violates the driver core API.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=2
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 02/17] scsi-multipath: introduce scsi_device head structure
2026-07-03 10:49 ` sashiko-bot
@ 2026-07-06 14:45 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 14:45 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 11:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Self-deadlock in scsi_mpath_find_head() due to kref_put() triggering release while holding scsi_mpath_heads_lock.
> - [High] Double-free and use-after-free of scsi_mpath_head in the error path of scsi_mpath_alloc_head().
> - [High] NULL pointer dereference and invalid device_unregister() execution when tearing down a failed device_add() in scsi_mpath_dev_alloc().
> --
>
> commit d7eb5e4fb506178f4a58d8aa264acf8230ebb5fa
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi-multipath: introduce scsi_device head structure
>
> This patch introduces a scsi_mpath_head structure to manage multipathing
> for a scsi_device. It maintains a list of scsi_mpath_head structures
> to look up matching multipathed devices by their unique ID. A new sysfs
> device class is also added to manage the scsi_mpath_head device members.
>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index ff37cfdf2f9d1..65ee3da5cc7fc 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
> [ ... ]
>> @@ -88,6 +146,71 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
> [ ... ]
>> +static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
>> +{
> [ ... ]
>> + ret = dev_set_name(&scsi_mpath_head->dev, "scsi_mpath_device%d",
>> + scsi_mpath_head->index);
>> + if (ret) {
>> + put_device(&scsi_mpath_head->dev);
>> + goto out_free_ida;
>> + }
>> +
>> + return scsi_mpath_head;
>> +
>> +out_free_ida:
>> + ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
>> +out_put_head:
>> + mpath_put_head(&scsi_mpath_head->mpath_head);
>> +out_free:
>> + kfree(scsi_mpath_head);
>> + return NULL;
>> +}
>
> [Severity: High]
> Will this cause a double-free on the error path?
>
> If dev_set_name() fails, put_device() is called which triggers the
> scsi_mpath_head_release() callback, freeing the IDA and the structure itself.
>
> The code then falls through to the out_free_ida label which calls ida_free()
> and kfree() a second time on the already freed pointer.
I will fix it
>
>> +
>> +static struct scsi_mpath_head *scsi_mpath_find_head(
>> + struct scsi_mpath_device *scsi_mpath_dev)
>> +{
> [ ... ]
>> + list_for_each_entry(scsi_mpath_head, &scsi_mpath_heads_list, entry) {
>> + ret = scsi_mpath_get_head(scsi_mpath_head);
>> + if (ret)
>> + continue;
>> + if (strncmp(scsi_mpath_head->vpd_id,
>> + scsi_mpath_dev->device_id_str,
>> + SCSI_MPATH_DEVICE_ID_LEN) == 0) {
>> +
>> + return scsi_mpath_head;
>> + }
>> + scsi_mpath_put_head(scsi_mpath_head);
>> + }
>> +
>> + return NULL;
>> +}
> [ ... ]
>> @@ -116,13 +240,58 @@ int scsi_mpath_dev_alloc(struct scsi_device *sdev)
> [ ... ]
>> + mutex_lock(&scsi_mpath_heads_lock);
>> + scsi_mpath_head = scsi_mpath_find_head(sdev->scsi_mpath_dev);
>> + if (scsi_mpath_head)
>> + goto found;
>> + scsi_mpath_head = scsi_mpath_alloc_head();
>> + if (!scsi_mpath_head) {
>> + sdev_printk(KERN_NOTICE, sdev, "could not allocate multipath head, device multipathing disabled\n");
>> + mutex_unlock(&scsi_mpath_heads_lock);
>> + goto out_uninit;
>> + }
>> +
>> + strscpy(scsi_mpath_head->vpd_id, sdev->scsi_mpath_dev->device_id_str,
>> + SCSI_MPATH_DEVICE_ID_LEN);
>> +
>> + ret = device_add(&scsi_mpath_head->dev);
>> + if (ret) {
>> + mutex_unlock(&scsi_mpath_heads_lock);
>> + goto out_put_head;
>> + }
>> +
>> + list_add_tail(&scsi_mpath_head->entry, &scsi_mpath_heads_list);
>> +found:
>> + mutex_unlock(&scsi_mpath_heads_lock);
> [ ... ]
>> +out_put_head:
>> + scsi_mpath_put_head(scsi_mpath_head);
>> out_uninit:
>> scsi_multipath_sdev_uninit(sdev);
>> return ret;
>> }
> [ ... ]
>> +static void scsi_mpath_free_head(struct kref *ref)
>> +{
>> + struct scsi_mpath_head *scsi_mpath_head =
>> + container_of(ref, struct scsi_mpath_head, ref);
>> +
> [ ... ]
>> + mutex_lock(&scsi_mpath_heads_lock);
>> + list_del_init(&scsi_mpath_head->entry);
>> + mutex_unlock(&scsi_mpath_heads_lock);
>> +
>> + device_unregister(&scsi_mpath_head->dev);
>> +}
>
> [Severity: High]
> Could this lead to an AB-BA self-deadlock?
>
> The scsi_mpath_dev_alloc() function acquires scsi_mpath_heads_lock before
> calling scsi_mpath_find_head(). During iteration, scsi_mpath_find_head()
> takes a temporary reference to each head.
>
> If a concurrent thread drops its reference to a head just as we process it,
> our scsi_mpath_put_head() call might drop the refcount to 0.
>
> This would trigger scsi_mpath_free_head(), which unconditionally tries to
> acquire scsi_mpath_heads_lock. Since the non-recursive mutex is already held
> by the current thread in scsi_mpath_dev_alloc(), it appears this would freeze
> the thread.
I will check it
>
> [Severity: High]
> Is there a risk of a NULL pointer dereference and invalid device unregistration
> here if device_add() fails?
>
> If device_add() fails in scsi_mpath_dev_alloc(), we jump to out_put_head and
> call scsi_mpath_put_head(). This drops the refcount to 0 and triggers this
> cleanup function.
>
> However, the scsi_mpath_head was just allocated with kzalloc and never added
> to the list, so entry.next and entry.prev are NULL. Calling list_del_init()
> would dereference these NULL pointers.
>
> Furthermore, calling device_unregister() will invoke device_del() on a device
> that failed device_add(), which violates the driver core API.
>
I will check it.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 03/17] scsi-multipath: provide sysfs link from to scsi_device
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
2026-07-03 10:33 ` [PATCH v3 01/17] scsi-multipath: introduce basic SCSI device support John Garry
2026-07-03 10:33 ` [PATCH v3 02/17] scsi-multipath: introduce scsi_device head structure John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 11:04 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 04/17] scsi-multipath: support iopolicy John Garry
` (13 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry,
Hannes Reinecke
Provide a link in sysfs from a scsi_mpath_device to member scsi_device's.
An example is as follows:
# ls -l /sys/class/scsi_mpath_device/scsi_mpath_device0/multipath/
total 0
lrwxrwxrwx 1 root root 0 Feb 24 12:01 8:0:0:0 -> ../../../../platform/host8/session1/target8:0:0/8:0:0:0
lrwxrwxrwx 1 root root 0 Feb 24 12:01 9:0:0:0 -> ../../../../platform/host9/session2/target9:0:0/9:0:0:0
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_multipath.c | 51 ++++++++++++++++++++++++++++++-----
drivers/scsi/scsi_sysfs.c | 5 ++++
include/scsi/scsi_multipath.h | 8 ++++++
3 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index 65ee3da5cc7fc..cb433a028dbff 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -104,23 +104,62 @@ static const struct attribute_group scsi_mpath_device_attrs_group = {
.attrs = scsi_mpath_device_attrs,
};
+static struct attribute dummy_attr = {
+ .name = "dummy",
+};
+
+static struct attribute *scsi_mpath_attrs[] = {
+ &dummy_attr,
+ NULL
+};
+
static bool scsi_multipath_sysfs_group_visible(struct kobject *kobj)
{
return true;
}
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(scsi_multipath_sysfs)
-static bool scsi_multipath_sysfs_attr_visible(struct kobject *kobj,
- struct attribute *attr, int n)
-{
- return false;
-}
-DEFINE_SYSFS_GROUP_VISIBLE(scsi_multipath_sysfs)
+static const struct attribute_group scsi_mpath_attr_group = {
+ .name = "multipath",
+ .attrs = scsi_mpath_attrs,
+ .is_visible = SYSFS_GROUP_VISIBLE(scsi_multipath_sysfs),
+};
static const struct attribute_group *scsi_mpath_device_groups[] = {
&scsi_mpath_device_attrs_group,
+ &scsi_mpath_attr_group,
NULL
};
+void scsi_mpath_add_sysfs_link(struct scsi_device *sdev)
+{
+ struct device *target = &sdev->sdev_gendev;
+ struct scsi_mpath_head *scsi_mpath_head =
+ sdev->scsi_mpath_dev->scsi_mpath_head;
+ struct device *source = &scsi_mpath_head->dev;
+ int error;
+
+ error = sysfs_add_link_to_group(&source->kobj, "multipath",
+ &target->kobj, dev_name(target));
+ if (error) {
+ sdev_printk(KERN_INFO, sdev, "Failed to create mpath sysfs link, error=%d\n",
+ error);
+ }
+}
+EXPORT_SYMBOL_GPL(scsi_mpath_add_sysfs_link);
+
+void scsi_mpath_remove_sysfs_link(struct scsi_device *sdev)
+{
+ struct device *target = &sdev->sdev_gendev;
+ struct scsi_mpath_head *scsi_mpath_head =
+ sdev->scsi_mpath_dev->scsi_mpath_head;
+ struct device *source = &scsi_mpath_head->dev;
+
+ sysfs_remove_link_from_group(&source->kobj, "multipath",
+ dev_name(target));
+}
+EXPORT_SYMBOL_GPL(scsi_mpath_remove_sysfs_link);
+
static const struct class scsi_mpath_device_class = {
.name = "scsi_mpath_device",
.dev_groups = scsi_mpath_device_groups,
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d6bbaf424bd4a..9c4f8d4a6f42c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1443,6 +1443,9 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
transport_add_device(&sdev->sdev_gendev);
sdev->is_visible = 1;
+ if (sdev->scsi_mpath_dev)
+ scsi_mpath_add_sysfs_link(sdev);
+
if (IS_ENABLED(CONFIG_BLK_DEV_BSG)) {
sdev->bsg_dev = scsi_bsg_register_queue(sdev);
if (IS_ERR(sdev->bsg_dev)) {
@@ -1495,6 +1498,8 @@ void __scsi_remove_device(struct scsi_device *sdev)
if (IS_ENABLED(CONFIG_BLK_DEV_BSG) && sdev->bsg_dev)
bsg_unregister_queue(sdev->bsg_dev);
+ if (sdev->scsi_mpath_dev)
+ scsi_mpath_remove_sysfs_link(sdev);
device_unregister(&sdev->sdev_dev);
transport_remove_device(dev);
device_del(dev);
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
index 50298056181f9..a9fd02bc42371 100644
--- a/include/scsi/scsi_multipath.h
+++ b/include/scsi/scsi_multipath.h
@@ -46,6 +46,8 @@ void scsi_mpath_dev_release(struct scsi_device *sdev);
int scsi_multipath_init(void);
void scsi_multipath_exit(void);
void scsi_mpath_remove_device(struct scsi_mpath_device *scsi_mpath_dev);
+void scsi_mpath_add_sysfs_link(struct scsi_device *sdev);
+void scsi_mpath_remove_sysfs_link(struct scsi_device *sdev);
int scsi_mpath_get_head(struct scsi_mpath_head *scsi_mpath_head);
void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head);
#else /* CONFIG_SCSI_MULTIPATH */
@@ -82,5 +84,11 @@ static inline
void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head)
{
}
+static inline void scsi_mpath_add_sysfs_link(struct scsi_device *sdev)
+{
+}
+static inline void scsi_mpath_remove_sysfs_link(struct scsi_device *sdev)
+{
+}
#endif /* CONFIG_SCSI_MULTIPATH */
#endif /* _SCSI_SCSI_MULTIPATH_H */
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 03/17] scsi-multipath: provide sysfs link from to scsi_device
2026-07-03 10:33 ` [PATCH v3 03/17] scsi-multipath: provide sysfs link from to scsi_device John Garry
@ 2026-07-03 11:04 ` sashiko-bot
0 siblings, 0 replies; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 11:04 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double free and use-after-free of `scsi_mpath_head` on `dev_set_name()` error path.
- [High] NULL pointer dereference from uninitialized `list_head` on `device_add()` error path.
--
commit d7eb5e4fb506178f4a58d8aa264acf8230ebb5fa
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: introduce scsi_device head structure
This commit introduces the scsi_mpath_head structure to manage multipathing
for SCSI devices. It adds a list to look up matching multipathed SCSI devices
using their unique IDs and creates a new scsi_mpath_device_class for managing
the head devices.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index ff37cfdf2f9d1..65ee3da5cc7fc 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
> @@ -88,6 +146,71 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
[ ... ]
> +static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
> +{
> + struct scsi_mpath_head *scsi_mpath_head;
> + int ret;
> +
> + scsi_mpath_head = kzalloc(sizeof(*scsi_mpath_head), GFP_KERNEL);
> + if (!scsi_mpath_head)
> + return NULL;
[ ... ]
> + device_initialize(&scsi_mpath_head->dev);
> + scsi_mpath_head->dev.class = &scsi_mpath_device_class;
> + ret = dev_set_name(&scsi_mpath_head->dev, "scsi_mpath_device%d",
> + scsi_mpath_head->index);
> + if (ret) {
> + put_device(&scsi_mpath_head->dev);
> + goto out_free_ida;
> + }
> +
> + return scsi_mpath_head;
> +
> +out_free_ida:
> + ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
> +out_put_head:
> + mpath_put_head(&scsi_mpath_head->mpath_head);
> +out_free:
> + kfree(scsi_mpath_head);
> + return NULL;
> +}
[Severity: High]
Does this error path cause a use-after-free and double free?
If dev_set_name() fails, put_device() drops the kobject refcount to zero,
which triggers scsi_mpath_head_release() to free the IDA index and kfree
the scsi_mpath_head structure.
When the code falls through to out_free_ida, it attempts to free the index
again using the already-freed pointer, and finally calls kfree() a second
time.
> @@ -96,6 +219,7 @@ static void scsi_multipath_sdev_uninit(struct scsi_device *sdev)
[ ... ]
> int scsi_mpath_dev_alloc(struct scsi_device *sdev)
> {
> + struct scsi_mpath_head *scsi_mpath_head;
> int ret;
[ ... ]
> + ret = device_add(&scsi_mpath_head->dev);
> + if (ret) {
> + mutex_unlock(&scsi_mpath_heads_lock);
> + goto out_put_head;
> + }
> +
> + list_add_tail(&scsi_mpath_head->entry, &scsi_mpath_heads_list);
> +found:
> + mutex_unlock(&scsi_mpath_heads_lock);
> + ret = ida_alloc(&scsi_mpath_head->ida, GFP_KERNEL);
> + if (ret < 0)
> + goto out_put_head;
> + sdev->scsi_mpath_dev->index = ret;
> +
> + sdev->scsi_mpath_dev->scsi_mpath_head = scsi_mpath_head;
> + return 0;
> +out_put_head:
> + scsi_mpath_put_head(scsi_mpath_head);
> out_uninit:
> scsi_multipath_sdev_uninit(sdev);
> return ret;
> }
> @@ -133,13 +302,47 @@ void scsi_mpath_dev_release(struct scsi_device *sdev)
[ ... ]
> +static void scsi_mpath_free_head(struct kref *ref)
> +{
> + struct scsi_mpath_head *scsi_mpath_head =
> + container_of(ref, struct scsi_mpath_head, ref);
> +
> + /*
> + * If we race with scsi_mpath_find_head(), then that function may
> + * find this scsi_mpath_head in the heads list; however we would fail
> + * to take a reference to this scsi_mpath_head and continue the search.
> + * As such, it is safe to call device_unregister (and free
> + * scsi_mpath_head) after we delete this head from the list.
> + */
> + mutex_lock(&scsi_mpath_heads_lock);
> + list_del_init(&scsi_mpath_head->entry);
> + mutex_unlock(&scsi_mpath_heads_lock);
> +
> + device_unregister(&scsi_mpath_head->dev);
> +}
[Severity: High]
Can this list_del_init() and device_unregister() crash if called from the
device_add() error path in scsi_mpath_dev_alloc()?
If device_add() fails, scsi_mpath_put_head() is called, which eventually
invokes this cleanup function. However, because device_add() failed before
list_add_tail() was called, and the list head was never initialized on the
kzalloc-ed structure, the list pointers are NULL. This would cause a NULL
pointer dereference in list_del_init().
Additionally, device_unregister() expects the device to have been
successfully added, which isn't the case on this error path.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=3
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 04/17] scsi-multipath: support iopolicy
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (2 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 03/17] scsi-multipath: provide sysfs link from to scsi_device John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 11:10 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 05/17] scsi-multipath: clone each bio John Garry
` (12 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add support to set the multipath iopolicy.
The iopolicy member is per scsi_mpath_head structure.
A module param is added so that the default iopolicy may be set.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_multipath.c | 47 +++++++++++++++++++++++++++++++++++
include/scsi/scsi_multipath.h | 3 +++
2 files changed, 50 insertions(+)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index cb433a028dbff..6159803d4cbb8 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -60,6 +60,23 @@ static const struct kernel_param_ops multipath_param_ops = {
module_param_cb(multipath, &multipath_param_ops, &scsi_multipath, 0444);
MODULE_PARM_DESC(multipath, "turn on native multipath support, options: on, off, always");
+static enum mpath_iopolicy_e iopolicy = MPATH_IOPOLICY_NUMA;
+
+static int scsi_mpath_set_iopolicy_param(const char *val, const struct kernel_param *kp)
+{
+ return mpath_set_iopolicy(val, &iopolicy);
+}
+
+static int scsi_mpath_get_iopolicy_param(char *buf, const struct kernel_param *kp)
+{
+ return mpath_get_iopolicy(buf, iopolicy);
+}
+
+module_param_call(multipath_iopolicy, scsi_mpath_set_iopolicy_param,
+ scsi_mpath_get_iopolicy_param, &iopolicy, 0644);
+MODULE_PARM_DESC(multipath_iopolicy,
+ "Default multipath I/O policy; 'numa' (default), 'round-robin' or 'queue-depth'");
+
static int scsi_mpath_unique_lun_id(struct scsi_device *sdev)
{
struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
@@ -95,8 +112,36 @@ static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
}
static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
+static ssize_t scsi_mpath_device_iopolicy_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct scsi_mpath_head *scsi_mpath_head =
+ container_of(dev, struct scsi_mpath_head, dev);
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+
+ if (!mpath_iopolicy_store(&scsi_mpath_head->iopolicy, buf, count))
+ return -EINVAL;
+
+ mpath_clear_paths(mpath_head);
+ mpath_schedule_requeue_work(mpath_head);
+ return count;
+}
+
+static ssize_t scsi_mpath_device_iopolicy_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct scsi_mpath_head *scsi_mpath_head =
+ container_of(dev, struct scsi_mpath_head, dev);
+
+ return mpath_iopolicy_show(&scsi_mpath_head->iopolicy, buf);
+}
+
+static DEVICE_ATTR(iopolicy, S_IRUGO | S_IWUSR,
+ scsi_mpath_device_iopolicy_show, scsi_mpath_device_iopolicy_store);
+
static struct attribute *scsi_mpath_device_attrs[] = {
&dev_attr_vpd_id.attr,
+ &dev_attr_iopolicy.attr,
NULL
};
@@ -201,7 +246,9 @@ static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
if (mpath_head_init(&scsi_mpath_head->mpath_head))
goto out_free;
+
scsi_mpath_head->mpath_head.mpdt = &smpdt;
+ scsi_mpath_head->mpath_head.iopolicy = &scsi_mpath_head->iopolicy;
scsi_mpath_head->index = ida_alloc(&scsi_multipath_dev_ida, GFP_KERNEL);
if (scsi_mpath_head->index < 0)
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
index a9fd02bc42371..2dc02313b0496 100644
--- a/include/scsi/scsi_multipath.h
+++ b/include/scsi/scsi_multipath.h
@@ -25,6 +25,7 @@ struct scsi_mpath_head {
struct list_head entry;
struct ida ida;
struct kref ref;
+ enum mpath_iopolicy_e iopolicy;
struct device dev;
int index;
};
@@ -40,6 +41,8 @@ struct scsi_mpath_device {
#define to_scsi_mpath_device(d) \
container_of(d, struct scsi_mpath_device, mpath_device)
+#define to_scsi_mpath_head(d) \
+ container_of(d, struct scsi_mpath_head, mpath_head)
int scsi_mpath_dev_alloc(struct scsi_device *sdev);
void scsi_mpath_dev_release(struct scsi_device *sdev);
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 04/17] scsi-multipath: support iopolicy
2026-07-03 10:33 ` [PATCH v3 04/17] scsi-multipath: support iopolicy John Garry
@ 2026-07-03 11:10 ` sashiko-bot
2026-07-06 14:49 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 11:10 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Medium] Newly allocated multipath heads ignore the module parameter default for `multipath_iopolicy`.
- [Medium] Lack of synchronization in `scsi_mpath_device_iopolicy_store()` allows concurrent writes to race and corrupt state.
- [Low] The patch calls several undefined core `mpath_` functions and uses undefined types, causing build failures.
Pre-existing issues:
- [Critical] The error path in `scsi_mpath_alloc_head()` performs manual cleanup after `put_device()`, leading to double-free bugs.
- [High] If `device_add()` fails in `scsi_mpath_dev_alloc()`, the error cleanup path crashes due to uninitialized list management and invalid device unregistration.
--
commit dcaa6fb2d7264e46f58295ab1af4b1021442b793
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: support iopolicy
Add support to set the multipath iopolicy. The iopolicy member is per
scsi_mpath_head structure. A module param is added so that the default
iopolicy may be set.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index cb433a028dbff..6159803d4cbb8 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
> @@ -95,8 +112,36 @@ static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
> }
> static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
>
> +static ssize_t scsi_mpath_device_iopolicy_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct scsi_mpath_head *scsi_mpath_head =
> + container_of(dev, struct scsi_mpath_head, dev);
> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> +
> + if (!mpath_iopolicy_store(&scsi_mpath_head->iopolicy, buf, count))
> + return -EINVAL;
[Severity: Low]
Are the functions mpath_iopolicy_store(), mpath_clear_paths(), and
mpath_schedule_requeue_work() defined anywhere in the kernel tree?
It looks like calling these will cause build failures.
> +
> + mpath_clear_paths(mpath_head);
> + mpath_schedule_requeue_work(mpath_head);
[Severity: Medium]
Are these sequential operations safe from race conditions when called
without holding any locks?
If multiple threads write to the sysfs attribute concurrently, could they
race and potentially duplicate path clearing or corrupt the I/O policy
state?
> + return count;
> +}
[ ... ]
> @@ -201,7 +246,9 @@ static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
>
> if (mpath_head_init(&scsi_mpath_head->mpath_head))
> goto out_free;
> +
> scsi_mpath_head->mpath_head.mpdt = &smpdt;
> + scsi_mpath_head->mpath_head.iopolicy = &scsi_mpath_head->iopolicy;
[Severity: Medium]
Will newly allocated multipath heads ignore the module parameter default
for multipath_iopolicy?
Since scsi_mpath_head is kzalloc'd, iopolicy defaults to zero
(MPATH_IOPOLICY_NUMA). This assignment hooks up the pointer but doesn't
initialize it with the global iopolicy module parameter.
[Severity: Critical]
This is a pre-existing issue, but does the error path in this function
cause a double-free?
If dev_set_name() fails further down in scsi_mpath_alloc_head():
if (ret) {
put_device(&scsi_mpath_head->dev);
goto out_free_ida;
}
The put_device() call drops the refcount to 0 and synchronously invokes
scsi_mpath_head_release(), which frees the IDA slot, uninitializes the
mpath head, and calls kfree(scsi_mpath_head).
The code then jumps to out_free_ida:
out_free_ida:
ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
out_put_head:
mpath_put_head(&scsi_mpath_head->mpath_head);
out_free:
kfree(scsi_mpath_head);
This manually repeats all of these steps, leading to a double-free and
use-after-free memory corruption.
[Severity: High]
This is a pre-existing issue, but can the device addition error path lead
to a NULL pointer dereference?
If device_add() fails in scsi_mpath_dev_alloc(), it jumps to out_put_head
and calls scsi_mpath_put_head(), which invokes scsi_mpath_free_head():
static void scsi_mpath_free_head(struct kref *ref)
{
...
mutex_lock(&scsi_mpath_heads_lock);
list_del_init(&scsi_mpath_head->entry);
Because device_add() failed, the object was never added to the list via
list_add_tail(). Since scsi_mpath_head was allocated with kzalloc(),
entry.next and entry.prev are NULL, and list_del_init() will dereference
these NULL pointers.
Additionally, device_unregister() is called on a device that failed
device_add(), which violates driver core API expectations.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=4
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 04/17] scsi-multipath: support iopolicy
2026-07-03 11:10 ` sashiko-bot
@ 2026-07-06 14:49 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 14:49 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 12:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
>
> New issues:
> - [Medium] Newly allocated multipath heads ignore the module parameter default for `multipath_iopolicy`.
> - [Medium] Lack of synchronization in `scsi_mpath_device_iopolicy_store()` allows concurrent writes to race and corrupt state.
> - [Low] The patch calls several undefined core `mpath_` functions and uses undefined types, causing build failures.
>
> Pre-existing issues:
> - [Critical] The error path in `scsi_mpath_alloc_head()` performs manual cleanup after `put_device()`, leading to double-free bugs.
> - [High] If `device_add()` fails in `scsi_mpath_dev_alloc()`, the error cleanup path crashes due to uninitialized list management and invalid device unregistration.
> --
>
> commit dcaa6fb2d7264e46f58295ab1af4b1021442b793
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi-multipath: support iopolicy
>
> Add support to set the multipath iopolicy. The iopolicy member is per
> scsi_mpath_head structure. A module param is added so that the default
> iopolicy may be set.
>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index cb433a028dbff..6159803d4cbb8 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
> [ ... ]
>> @@ -95,8 +112,36 @@ static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
>> }
>> static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
>>
>> +static ssize_t scsi_mpath_device_iopolicy_store(struct device *dev,
>> + struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> + struct scsi_mpath_head *scsi_mpath_head =
>> + container_of(dev, struct scsi_mpath_head, dev);
>> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
>> +
>> + if (!mpath_iopolicy_store(&scsi_mpath_head->iopolicy, buf, count))
>> + return -EINVAL;
>
> [Severity: Low]
> Are the functions mpath_iopolicy_store(), mpath_clear_paths(), and
> mpath_schedule_requeue_work() defined anywhere in the kernel tree?
> It looks like calling these will cause build failures.
>
>> +
>> + mpath_clear_paths(mpath_head);
>> + mpath_schedule_requeue_work(mpath_head);
>
> [Severity: Medium]
> Are these sequential operations safe from race conditions when called
> without holding any locks?
>
> If multiple threads write to the sysfs attribute concurrently, could they
> race and potentially duplicate path clearing or corrupt the I/O policy
> state?
>
>> + return count;
>> +}
> [ ... ]
>> @@ -201,7 +246,9 @@ static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
>>
>> if (mpath_head_init(&scsi_mpath_head->mpath_head))
>> goto out_free;
>> +
>> scsi_mpath_head->mpath_head.mpdt = &smpdt;
>> + scsi_mpath_head->mpath_head.iopolicy = &scsi_mpath_head->iopolicy;
>
> [Severity: Medium]
> Will newly allocated multipath heads ignore the module parameter default
> for multipath_iopolicy?
>
> Since scsi_mpath_head is kzalloc'd, iopolicy defaults to zero
> (MPATH_IOPOLICY_NUMA). This assignment hooks up the pointer but doesn't
> initialize it with the global iopolicy module parameter.
I need to check what is missing here, as this was working...
>
> [Severity: Critical]
> This is a pre-existing issue, but does the error path in this function
> cause a double-free?
>
> If dev_set_name() fails further down in scsi_mpath_alloc_head():
>
> if (ret) {
> put_device(&scsi_mpath_head->dev);
> goto out_free_ida;
> }
>
> The put_device() call drops the refcount to 0 and synchronously invokes
> scsi_mpath_head_release(), which frees the IDA slot, uninitializes the
> mpath head, and calls kfree(scsi_mpath_head).
>
> The code then jumps to out_free_ida:
>
> out_free_ida:
> ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
> out_put_head:
> mpath_put_head(&scsi_mpath_head->mpath_head);
> out_free:
> kfree(scsi_mpath_head);
>
> This manually repeats all of these steps, leading to a double-free and
> use-after-free memory corruption.
>
> [Severity: High]
> This is a pre-existing issue, but can the device addition error path lead
> to a NULL pointer dereference?
It would be nice to stop repeating review comments from earlier patches...
>
> If device_add() fails in scsi_mpath_dev_alloc(), it jumps to out_put_head
> and calls scsi_mpath_put_head(), which invokes scsi_mpath_free_head():
>
> static void scsi_mpath_free_head(struct kref *ref)
> {
> ...
> mutex_lock(&scsi_mpath_heads_lock);
> list_del_init(&scsi_mpath_head->entry);
>
> Because device_add() failed, the object was never added to the list via
> list_add_tail(). Since scsi_mpath_head was allocated with kzalloc(),
> entry.next and entry.prev are NULL, and list_del_init() will dereference
> these NULL pointers.
>
> Additionally, device_unregister() is called on a device that failed
> device_add(), which violates driver core API expectations.
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 05/17] scsi-multipath: clone each bio
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (3 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 04/17] scsi-multipath: support iopolicy John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 11:26 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 06/17] scsi-multipath: clear path when device is blocked John Garry
` (11 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
For failover handling, we will take the approach to resubmit each
bio.
However, unlike NVMe, for SCSI there is no guarantee that any bio submitted
is either all or none completed.
As such, for SCSI, for failover handling we will take the approach to
just re-submit the original bio. For this, clone and submit each bio.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_multipath.c | 36 +++++++++++++++++++++++++++++++++--
include/scsi/scsi_multipath.h | 1 +
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index 6159803d4cbb8..ca4ab720c19af 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -96,6 +96,7 @@ static void scsi_mpath_head_release(struct device *dev)
container_of(dev, struct scsi_mpath_head, dev);
struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+ bioset_exit(&scsi_mpath_head->bio_pool);
ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
mpath_head_uninit(mpath_head);
kfree(scsi_mpath_head);
@@ -230,7 +231,34 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
return 0;
}
+static void scsi_mpath_clone_end_io(struct bio *clone)
+{
+ struct bio *master_bio = clone->bi_private;
+
+ master_bio->bi_status = clone->bi_status;
+ bio_put(clone);
+ bio_endio(master_bio);
+}
+
+static struct bio *scsi_mpath_clone_bio(struct bio *bio)
+{
+ struct mpath_head *mpath_head = bio->bi_bdev->bd_disk->private_data;
+ struct scsi_mpath_head *scsi_mpath_head = to_scsi_mpath_head(mpath_head);
+ struct bio *clone;
+
+ clone = bio_alloc_clone(bio->bi_bdev, bio, GFP_NOIO,
+ &scsi_mpath_head->bio_pool);
+ if (!clone)
+ return NULL;
+
+ clone->bi_end_io = scsi_mpath_clone_end_io;
+ clone->bi_private = bio;
+
+ return clone;
+}
+
static struct mpath_head_template smpdt = {
+ .clone_bio = scsi_mpath_clone_bio,
};
static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
@@ -244,9 +272,11 @@ static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
ida_init(&scsi_mpath_head->ida);
- if (mpath_head_init(&scsi_mpath_head->mpath_head))
+ if (bioset_init(&scsi_mpath_head->bio_pool, BIO_POOL_SIZE,
+ 0, BIOSET_PERCPU_CACHE))
goto out_free;
-
+ if (mpath_head_init(&scsi_mpath_head->mpath_head))
+ goto out_bioset_exit;
scsi_mpath_head->mpath_head.mpdt = &smpdt;
scsi_mpath_head->mpath_head.iopolicy = &scsi_mpath_head->iopolicy;
@@ -270,6 +300,8 @@ static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
out_put_head:
mpath_put_head(&scsi_mpath_head->mpath_head);
+out_bioset_exit:
+ bioset_exit(&scsi_mpath_head->bio_pool);
out_free:
kfree(scsi_mpath_head);
return NULL;
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
index 2dc02313b0496..5cc37acaa664f 100644
--- a/include/scsi/scsi_multipath.h
+++ b/include/scsi/scsi_multipath.h
@@ -26,6 +26,7 @@ struct scsi_mpath_head {
struct ida ida;
struct kref ref;
enum mpath_iopolicy_e iopolicy;
+ struct bio_set bio_pool;
struct device dev;
int index;
};
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 05/17] scsi-multipath: clone each bio
2026-07-03 10:33 ` [PATCH v3 05/17] scsi-multipath: clone each bio John Garry
@ 2026-07-03 11:26 ` sashiko-bot
2026-07-06 14:53 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 11:26 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Destroying the bio pool before draining inflight IOs during teardown leads to memory corruption.
- [High] Manual cleanup on the dev_set_name() error path after put_device() causes deterministic double-free and use-after-free vulnerabilities.
--
commit 31da3ef05c72ae055a7ebbed278c14967c624ea3
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: clone each bio
For failover handling, we will take the approach to resubmit each
bio. Unlike NVMe, for SCSI there is no guarantee that any bio submitted
is either all or none completed. As such, for failover handling clone
and submit each bio.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 6159803d4cbb8..ca4ab720c19af 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
> @@ -96,6 +96,7 @@ static void scsi_mpath_head_release(struct device *dev)
> container_of(dev, struct scsi_mpath_head, dev);
> struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
>
> + bioset_exit(&scsi_mpath_head->bio_pool);
[Severity: High]
Does destroying the bio pool here before draining inflight IOs lead to memory
corruption?
If a device is removed, the final reference to the scsi_mpath_head might be
dropped before the underlying request queue is frozen. Because the queue is
not yet drained, inflight cloned bios can still complete after the pool is
destroyed. When they do, scsi_mpath_clone_end_io() calls bio_put(), which
will attempt to return the clone to this destroyed bio_pool, causing a
use-after-free.
> ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
> mpath_head_uninit(mpath_head);
[ ... ]
> @@ -270,6 +300,8 @@ static struct scsi_mpath_head *scsi_mpath_alloc_head(void)
> ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
> out_put_head:
> mpath_put_head(&scsi_mpath_head->mpath_head);
> +out_bioset_exit:
> + bioset_exit(&scsi_mpath_head->bio_pool);
> out_free:
> kfree(scsi_mpath_head);
[Severity: High]
Can this error cleanup path cause a double-free vulnerability?
If dev_set_name() fails earlier in scsi_mpath_alloc_head(), the code will
call put_device() which drops the kobject reference count and synchronously
invokes scsi_mpath_head_release().
scsi_mpath_head_release() completely tears down and frees the scsi_mpath_head
structure, but the error path then continues through these manual cleanup
steps, executing bioset_exit() and kfree() on the already freed pointer.
> return NULL;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=5
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v3 05/17] scsi-multipath: clone each bio
2026-07-03 11:26 ` sashiko-bot
@ 2026-07-06 14:53 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 14:53 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 12:26, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Destroying the bio pool before draining inflight IOs during teardown leads to memory corruption.
> - [High] Manual cleanup on the dev_set_name() error path after put_device() causes deterministic double-free and use-after-free vulnerabilities.
> --
>
> commit 31da3ef05c72ae055a7ebbed278c14967c624ea3
> Author: John Garry<john.g.garry@oracle.com>
>
> scsi-multipath: clone each bio
>
> For failover handling, we will take the approach to resubmit each
> bio. Unlike NVMe, for SCSI there is no guarantee that any bio submitted
> is either all or none completed. As such, for failover handling clone
> and submit each bio.
>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index 6159803d4cbb8..ca4ab720c19af 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
>> @@ -96,6 +96,7 @@ static void scsi_mpath_head_release(struct device *dev)
>> container_of(dev, struct scsi_mpath_head, dev);
>> struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
>>
>> + bioset_exit(&scsi_mpath_head->bio_pool);
> [Severity: High]
> Does destroying the bio pool here before draining inflight IOs lead to memory
> corruption?
>
> If a device is removed, the final reference to the scsi_mpath_head might be
> dropped before the underlying request queue is frozen.
We remove the gendisk in sd_mpath_remove(), and this must be done prior
to tidying up the scsi_mpath_head.
> Because the queue is
> not yet drained, inflight cloned bios can still complete after the pool is
> destroyed. When they do, scsi_mpath_clone_end_io() calls bio_put(), which
> will attempt to return the clone to this destroyed bio_pool, causing a
> use-after-free.
>
>> ida_free(&scsi_multipath_dev_ida, scsi_mpath_head->index);
>> mpath_head_uninit(mpath_head);
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 06/17] scsi-multipath: clear path when device is blocked
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (4 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 05/17] scsi-multipath: clone each bio John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 11:32 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 07/17] scsi-multipath: failover handling John Garry
` (10 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add scsi_mpath_dev_clear_path() to clear a device path when it becomes
blocked, and call from __scsi_internal_device_block_nowait().
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_lib.c | 3 +++
drivers/scsi/scsi_multipath.c | 11 +++++++++++
include/scsi/scsi_multipath.h | 5 +++++
3 files changed, 19 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 22e2e3223440d..cc34253c467a2 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -34,6 +34,7 @@
#include <scsi/scsi_eh.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_transport.h> /* scsi_init_limits() */
+#include <scsi/scsi_multipath.h>
#include <scsi/scsi_dh.h>
#include <trace/events/scsi.h>
@@ -2931,6 +2932,8 @@ EXPORT_SYMBOL(scsi_target_resume);
static int __scsi_internal_device_block_nowait(struct scsi_device *sdev)
{
+ if (sdev->scsi_mpath_dev)
+ scsi_mpath_dev_clear_path(sdev->scsi_mpath_dev);
if (scsi_device_set_state(sdev, SDEV_BLOCK))
return scsi_device_set_state(sdev, SDEV_CREATED_BLOCK);
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index ca4ab720c19af..0f3f3f9fa5fae 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -113,6 +113,17 @@ static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
}
static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
+void scsi_mpath_dev_clear_path(struct scsi_mpath_device *scsi_mpath_dev)
+{
+ struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
+ struct scsi_mpath_head *scsi_mpath_head = scsi_mpath_dev->scsi_mpath_head;
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+
+ if (mpath_clear_current_path(mpath_device))
+ mpath_synchronize(mpath_head);
+}
+EXPORT_SYMBOL_GPL(scsi_mpath_dev_clear_path);
+
static ssize_t scsi_mpath_device_iopolicy_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
index 5cc37acaa664f..d8644d6261992 100644
--- a/include/scsi/scsi_multipath.h
+++ b/include/scsi/scsi_multipath.h
@@ -49,6 +49,7 @@ int scsi_mpath_dev_alloc(struct scsi_device *sdev);
void scsi_mpath_dev_release(struct scsi_device *sdev);
int scsi_multipath_init(void);
void scsi_multipath_exit(void);
+void scsi_mpath_dev_clear_path(struct scsi_mpath_device *scsi_mpath_dev);
void scsi_mpath_remove_device(struct scsi_mpath_device *scsi_mpath_dev);
void scsi_mpath_add_sysfs_link(struct scsi_device *sdev);
void scsi_mpath_remove_sysfs_link(struct scsi_device *sdev);
@@ -80,6 +81,10 @@ void scsi_mpath_remove_device(struct scsi_mpath_device *scsi_mpath_dev)
{
}
static inline
+void scsi_mpath_dev_clear_path(struct scsi_mpath_device *scsi_mpath_dev)
+{
+}
+static inline
int scsi_mpath_get_head(struct scsi_mpath_head *scsi_mpath_head)
{
return 0;
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 06/17] scsi-multipath: clear path when device is blocked
2026-07-03 10:33 ` [PATCH v3 06/17] scsi-multipath: clear path when device is blocked John Garry
@ 2026-07-03 11:32 ` sashiko-bot
2026-07-06 15:04 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 11:32 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Critical] Unsafe lockless access to `scsi_mpath_head` in `scsi_mpath_dev_clear_path` races with device removal, causing a NULL pointer dereference or Use-After-Free.
- [High] Adding a blocking synchronization call to the `nowait` context of `__scsi_internal_device_block_nowait` violates non-blocking guarantees.
- [High] Publishing `sdev->scsi_mpath_dev` before `scsi_mpath_head` is initialized leads to NULL pointer dereferences during concurrent block requests.
Pre-existing issues:
- [High] The error path in `scsi_mpath_alloc_head()` double-frees the `scsi_mpath_head` structure and its associated resources.
- [High] The error path for `device_add()` failure in `scsi_mpath_dev_alloc()` causes a NULL pointer dereference and an invalid `device_unregister()`.
--
commit 848d562af61faa57ddead6bc3e5e7a75ce63ea4e
Author: John Garry <john.g.garry@oracle.com>
Subject: scsi-multipath: clear path when device is blocked
This patch adds scsi_mpath_dev_clear_path() to clear a device path when it
becomes blocked. It updates __scsi_internal_device_block_nowait() to call
this new function when a path is available.
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index daeb3693fe559..3c493068adb98 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
[ ... ]
> @@ -2923,6 +2924,8 @@ EXPORT_SYMBOL(scsi_target_resume);
>
> static int __scsi_internal_device_block_nowait(struct scsi_device *sdev)
> {
> + if (sdev->scsi_mpath_dev)
[Severity: High]
Does this early access race with device probe?
If scsi_multipath_sdev_init() publishes sdev->scsi_mpath_dev before
allocating scsi_mpath_head, could an asynchronous block request see this as
true and invoke scsi_mpath_dev_clear_path(), leading to a NULL pointer
dereference on the head?
> + scsi_mpath_dev_clear_path(sdev->scsi_mpath_dev);
> if (scsi_device_set_state(sdev, SDEV_BLOCK))
> return scsi_device_set_state(sdev, SDEV_CREATED_BLOCK);
>
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index ca4ab720c19af..0f3f3f9fa5fae 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
> @@ -113,6 +113,17 @@ static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
> }
> static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
>
> +void scsi_mpath_dev_clear_path(struct scsi_mpath_device *scsi_mpath_dev)
> +{
> + struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
> + struct scsi_mpath_head *scsi_mpath_head = scsi_mpath_dev->scsi_mpath_head;
[Severity: Critical]
Is this lockless access safe against concurrent device removal?
If scsi_mpath_remove_head() drops the head reference locklessly and sets:
scsi_mpath_dev->scsi_mpath_head = NULL;
Could a concurrent event hitting __scsi_internal_device_block_nowait()
unconditionally call this and blindly dereference the NULL or freed
scsi_mpath_head?
> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> +
> + if (mpath_clear_current_path(mpath_device))
> + mpath_synchronize(mpath_head);
[Severity: High]
Does this introduce a sleeping call into a nowait context?
__scsi_internal_device_block_nowait() is designed not to sleep to preserve
batching optimizations. Since mpath_synchronize() typically sleeps to wait
for grace periods or draining I/O, will calling it here break those
optimizations or cause scheduling while atomic panics?
> +}
> +EXPORT_SYMBOL_GPL(scsi_mpath_dev_clear_path);
> +
> static ssize_t scsi_mpath_device_iopolicy_store(struct device *dev,
While reviewing this patch, I also noticed a couple of pre-existing issues
in the earlier patches of this series:
[Severity: High]
This is a pre-existing issue, but does the error path in
scsi_mpath_alloc_head() double-free the structure?
If dev_set_name() fails:
if (dev_set_name(&scsi_mpath_head->dev, ...)) {
put_device(&scsi_mpath_head->dev);
goto out_free_ida;
}
put_device() drops the refcount to 0, which invokes the release function
scsi_mpath_head_release() to free the structure and its resources.
The code then falls through to out_free_ida where it manually executes the
IDA free, bioset exit, and a second kfree() on the already freed struct.
[Severity: High]
This is a pre-existing issue, but does the error path for device_add()
cause a NULL pointer dereference in scsi_mpath_dev_alloc()?
If device_add() fails, it jumps to out_put_head:
ret = device_add(&scsi_mpath_head->dev);
if (ret) {
mutex_unlock(&scsi_mpath_heads_lock);
goto out_put_head;
}
This ultimately calls scsi_mpath_free_head(), which unconditionally
executes:
list_del_init(&scsi_mpath_head->entry);
Since the device was never added to the list on this error path, entry is
still zero-initialized. Will list_del_init() dereference a NULL pointer
here? Also, would the cleanup code call device_unregister() on a device
that failed device_add()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=6
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 06/17] scsi-multipath: clear path when device is blocked
2026-07-03 11:32 ` sashiko-bot
@ 2026-07-06 15:04 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:04 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 12:32, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
>
> New issues:
> - [Critical] Unsafe lockless access to `scsi_mpath_head` in `scsi_mpath_dev_clear_path` races with device removal, causing a NULL pointer dereference or Use-After-Free.
> - [High] Adding a blocking synchronization call to the `nowait` context of `__scsi_internal_device_block_nowait` violates non-blocking guarantees.
> - [High] Publishing `sdev->scsi_mpath_dev` before `scsi_mpath_head` is initialized leads to NULL pointer dereferences during concurrent block requests.
>
> Pre-existing issues:
> - [High] The error path in `scsi_mpath_alloc_head()` double-frees the `scsi_mpath_head` structure and its associated resources.
> - [High] The error path for `device_add()` failure in `scsi_mpath_dev_alloc()` causes a NULL pointer dereference and an invalid `device_unregister()`.
> --
>
> commit 848d562af61faa57ddead6bc3e5e7a75ce63ea4e
> Author: John Garry <john.g.garry@oracle.com>
> Subject: scsi-multipath: clear path when device is blocked
>
> This patch adds scsi_mpath_dev_clear_path() to clear a device path when it
> becomes blocked. It updates __scsi_internal_device_block_nowait() to call
> this new function when a path is available.
>
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index daeb3693fe559..3c493068adb98 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
> [ ... ]
>> @@ -2923,6 +2924,8 @@ EXPORT_SYMBOL(scsi_target_resume);
>>
>> static int __scsi_internal_device_block_nowait(struct scsi_device *sdev)
>> {
>> + if (sdev->scsi_mpath_dev)
>
> [Severity: High]
> Does this early access race with device probe?
>
> If scsi_multipath_sdev_init() publishes sdev->scsi_mpath_dev before
> allocating scsi_mpath_head, could an asynchronous block request see this as
> true and invoke scsi_mpath_dev_clear_path(), leading to a NULL pointer
> dereference on the head?
this should not be a problem, but I will change to alloc head before
publishing sdev->scsi_mpath_dev
>
>> + scsi_mpath_dev_clear_path(sdev->scsi_mpath_dev);
>> if (scsi_device_set_state(sdev, SDEV_BLOCK))
>> return scsi_device_set_state(sdev, SDEV_CREATED_BLOCK);
>>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index ca4ab720c19af..0f3f3f9fa5fae 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
> [ ... ]
>> @@ -113,6 +113,17 @@ static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
>> }
>> static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
>>
>> +void scsi_mpath_dev_clear_path(struct scsi_mpath_device *scsi_mpath_dev)
>> +{
>> + struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
>> + struct scsi_mpath_head *scsi_mpath_head = scsi_mpath_dev->scsi_mpath_head;
>
> [Severity: Critical]
> Is this lockless access safe against concurrent device removal?
>
> If scsi_mpath_remove_head() drops the head reference locklessly and sets:
> scsi_mpath_dev->scsi_mpath_head = NULL;
>
> Could a concurrent event hitting __scsi_internal_device_block_nowait()
> unconditionally call this and blindly dereference the NULL or freed
> scsi_mpath_head?
this should not happen as the sdev should be nearly removed when the
scsi mpath head reference is put
>
>> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
>> +
>> + if (mpath_clear_current_path(mpath_device))
>> + mpath_synchronize(mpath_head);
>
> [Severity: High]
> Does this introduce a sleeping call into a nowait context?
>
> __scsi_internal_device_block_nowait() is designed not to sleep to preserve
> batching optimizations. Since mpath_synchronize() typically sleeps to wait
> for grace periods or draining I/O, will calling it here break those
> optimizations or cause scheduling while atomic panics?
I need to check that
>
>> +}
>> +EXPORT_SYMBOL_GPL(scsi_mpath_dev_clear_path);
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 07/17] scsi-multipath: failover handling
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (5 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 06/17] scsi-multipath: clear path when device is blocked John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 11:40 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 08/17] scsi-multipath: provide callbacks for path state John Garry
` (9 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Failover occurs when the scsi_cmnd has failed and it is discovered that the
target scsi_device has transport down.
For a scsi command which suffers failover, requeue the master bio of each
bio attached to its request.
A bio which for which failover occurs is handled in
scsi_mpath_clone_end_io(). Failover is detected for blk_path_error()
occuring, same as how dm-mpath detects this.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_multipath.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index 0f3f3f9fa5fae..f22e3677cf2ad 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -242,11 +242,44 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
return 0;
}
+static inline void bio_list_add_clone(struct bio_list *bl,
+ struct bio *clone)
+{
+ struct bio *master_bio = clone->bi_private;
+
+ if (bl->tail)
+ bl->tail->bi_next = master_bio;
+ else
+ bl->head = master_bio;
+ bl->tail = master_bio;
+ bio_put(clone);
+}
+
static void scsi_mpath_clone_end_io(struct bio *clone)
{
struct bio *master_bio = clone->bi_private;
master_bio->bi_status = clone->bi_status;
+
+ if (clone->bi_status && blk_path_error(clone->bi_status)) {
+ struct block_device *bi_bdev = clone->bi_bdev;
+ struct request_queue *q = bi_bdev->bd_queue;
+ struct scsi_device *sdev = scsi_device_from_queue(q);
+ struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
+ struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
+ struct mpath_head *mpath_head = mpath_device->mpath_head;
+ unsigned long flags;
+
+ scsi_mpath_dev_clear_path(scsi_mpath_dev);
+
+ spin_lock_irqsave(&mpath_head->requeue_lock, flags);
+ bio_list_add_clone(&mpath_head->requeue_list, clone);
+ spin_unlock_irqrestore(&mpath_head->requeue_lock, flags);
+
+ mpath_schedule_requeue_work(mpath_head);
+ return;
+ }
+
bio_put(clone);
bio_endio(master_bio);
}
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 07/17] scsi-multipath: failover handling
2026-07-03 10:33 ` [PATCH v3 07/17] scsi-multipath: failover handling John Garry
@ 2026-07-03 11:40 ` sashiko-bot
2026-07-06 15:32 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 11:40 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unchecked dereference of `sdev` returned by `scsi_device_from_queue()` leading to an immediate kernel panic.
- [High] Leaking a reference to `scsi_device` acquired by `scsi_device_from_queue()`.
- [High] Calling a sleepable synchronization function in `bio_end_io` atomic context.
--
commit 8a70735872bb7cb2125e8980a60e0bae2a38b040
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: failover handling
This commit implements failover handling for scsi commands that fail due to
the target device's transport going down. It requeues the master bio of each
bio attached to the request by handling the failover inside the
scsi_mpath_clone_end_io() completion handler.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 0f3f3f9fa5fae..f22e3677cf2ad 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
> @@ -242,11 +242,44 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
> static void scsi_mpath_clone_end_io(struct bio *clone)
> {
> struct bio *master_bio = clone->bi_private;
>
> master_bio->bi_status = clone->bi_status;
> +
> + if (clone->bi_status && blk_path_error(clone->bi_status)) {
> + struct block_device *bi_bdev = clone->bi_bdev;
> + struct request_queue *q = bi_bdev->bd_queue;
> + struct scsi_device *sdev = scsi_device_from_queue(q);
> + struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
[Severity: High]
Can scsi_device_from_queue() return NULL here? If a path error occurs
concurrently with a device removal or hot-unplug, get_device() inside
scsi_device_from_queue() could fail. If sdev is NULL, wouldn't accessing
sdev->scsi_mpath_dev on the very next line cause an immediate NULL pointer
dereference?
> + struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
> + struct mpath_head *mpath_head = mpath_device->mpath_head;
> + unsigned long flags;
> +
> + scsi_mpath_dev_clear_path(scsi_mpath_dev);
[Severity: High]
Is it safe to call scsi_mpath_dev_clear_path() from a bio completion handler?
Since scsi_mpath_clone_end_io() runs in atomic softirq context, and
scsi_mpath_dev_clear_path() can call mpath_synchronize() which might sleep
to wait for grace periods, could this trigger a sleeping in atomic context
BUG?
> +
> + spin_lock_irqsave(&mpath_head->requeue_lock, flags);
> + bio_list_add_clone(&mpath_head->requeue_list, clone);
> + spin_unlock_irqrestore(&mpath_head->requeue_lock, flags);
> +
> + mpath_schedule_requeue_work(mpath_head);
> + return;
[Severity: High]
Does this return path leak the reference to the scsi_device?
When scsi_device_from_queue() is called earlier, it increments the device's
kobject reference count via get_device(). There doesn't appear to be a
corresponding put_device() before returning here, which might permanently
prevent device cleanup on unplug if repeated failovers occur.
> + }
> +
> bio_put(clone);
> bio_endio(master_bio);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=7
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 07/17] scsi-multipath: failover handling
2026-07-03 11:40 ` sashiko-bot
@ 2026-07-06 15:32 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 12:40, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Unchecked dereference of `sdev` returned by `scsi_device_from_queue()` leading to an immediate kernel panic.
> - [High] Leaking a reference to `scsi_device` acquired by `scsi_device_from_queue()`.
> - [High] Calling a sleepable synchronization function in `bio_end_io` atomic context.
> --
>
> commit 8a70735872bb7cb2125e8980a60e0bae2a38b040
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi-multipath: failover handling
>
> This commit implements failover handling for scsi commands that fail due to
> the target device's transport going down. It requeues the master bio of each
> bio attached to the request by handling the failover inside the
> scsi_mpath_clone_end_io() completion handler.
>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index 0f3f3f9fa5fae..f22e3677cf2ad 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
> [ ... ]
>> @@ -242,11 +242,44 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
>> static void scsi_mpath_clone_end_io(struct bio *clone)
>> {
>> struct bio *master_bio = clone->bi_private;
>>
>> master_bio->bi_status = clone->bi_status;
>> +
>> + if (clone->bi_status && blk_path_error(clone->bi_status)) {
>> + struct block_device *bi_bdev = clone->bi_bdev;
>> + struct request_queue *q = bi_bdev->bd_queue;
>> + struct scsi_device *sdev = scsi_device_from_queue(q);
>> + struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
>
> [Severity: High]
> Can scsi_device_from_queue() return NULL here? If a path error occurs
> concurrently with a device removal or hot-unplug, get_device() inside
> scsi_device_from_queue() could fail. If sdev is NULL, wouldn't accessing
> sdev->scsi_mpath_dev on the very next line cause an immediate NULL pointer
> dereference?
will fix
>
>> + struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
>> + struct mpath_head *mpath_head = mpath_device->mpath_head;
>> + unsigned long flags;
>> +
>> + scsi_mpath_dev_clear_path(scsi_mpath_dev);
>
> [Severity: High]
> Is it safe to call scsi_mpath_dev_clear_path() from a bio completion handler?
> Since scsi_mpath_clone_end_io() runs in atomic softirq context, and
> scsi_mpath_dev_clear_path() can call mpath_synchronize() which might sleep
> to wait for grace periods, could this trigger a sleeping in atomic context
> BUG?
I'd like more info for when this is called from softirq context. From my
experience we the failover completion call from blk_mq_work_fn in ...
scsi_queue_rq() -> blk_update_request()
>
>> +
>> + spin_lock_irqsave(&mpath_head->requeue_lock, flags);
>> + bio_list_add_clone(&mpath_head->requeue_list, clone);
>> + spin_unlock_irqrestore(&mpath_head->requeue_lock, flags);
>> +
>> + mpath_schedule_requeue_work(mpath_head);
>> + return;
>
> [Severity: High]
> Does this return path leak the reference to the scsi_device?
>
> When scsi_device_from_queue() is called earlier, it increments the device's
> kobject reference count via get_device(). There doesn't appear to be a
> corresponding put_device() before returning here, which might permanently
> prevent device cleanup on unplug if repeated failovers occur.
will fix
>
>> + }
>> +
>> bio_put(clone);
>> bio_endio(master_bio);
>> }
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 08/17] scsi-multipath: provide callbacks for path state
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (6 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 07/17] scsi-multipath: failover handling John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 11:49 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 09/17] scsi-multipath: add scsi_mpath_{start,end}_request() John Garry
` (8 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Provide callbacks for .is_disabled, .is_optimized, and .available_path.
These all use scsi_device.sdev_state and scsi_device.access_state.
Member scsi_device.access_state will be driven by ALUA. Currently
only device handlers support this, and in future we will have core
SCSI support for implicit ALUA (not relying on device handlers).
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_multipath.c | 46 +++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index f22e3677cf2ad..d8ea9ffe8942c 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -301,7 +301,53 @@ static struct bio *scsi_mpath_clone_bio(struct bio *bio)
return clone;
}
+static bool scsi_mpath_is_disabled(struct mpath_device *mpath_device)
+{
+ struct scsi_mpath_device *scsi_mpath_dev =
+ to_scsi_mpath_device(mpath_device);
+ struct scsi_device *sdev = scsi_mpath_dev->sdev;
+ unsigned char access_state = READ_ONCE(sdev->access_state);
+
+ if (sdev->sdev_state != SDEV_RUNNING)
+ return true;
+
+ if (access_state == SCSI_ACCESS_STATE_OPTIMAL ||
+ access_state == SCSI_ACCESS_STATE_ACTIVE)
+ return false;
+
+ return true;
+}
+
+static bool scsi_mpath_is_optimized(struct mpath_device *mpath_device)
+{
+ struct scsi_mpath_device *scsi_mpath_dev =
+ to_scsi_mpath_device(mpath_device);
+ struct scsi_device *sdev = scsi_mpath_dev->sdev;
+
+ if (sdev->sdev_state != SDEV_RUNNING)
+ return false;
+
+ return READ_ONCE(sdev->access_state) == SCSI_ACCESS_STATE_OPTIMAL;
+}
+
+static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
+{
+ struct scsi_mpath_device *scsi_mpath_dev =
+ to_scsi_mpath_device(mpath_device);
+ struct scsi_device *sdev = scsi_mpath_dev->sdev;
+ enum scsi_device_state sdev_state = sdev->sdev_state;
+
+ if (sdev_state == SDEV_RUNNING || sdev_state == SDEV_QUIESCE ||
+ sdev_state == SDEV_BLOCK || sdev_state == SDEV_CREATED_BLOCK)
+ return true;
+
+ return false;
+}
+
static struct mpath_head_template smpdt = {
+ .is_disabled = scsi_mpath_is_disabled,
+ .is_optimized = scsi_mpath_is_optimized,
+ .available_path = scsi_mpath_available_path,
.clone_bio = scsi_mpath_clone_bio,
};
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 08/17] scsi-multipath: provide callbacks for path state
2026-07-03 10:33 ` [PATCH v3 08/17] scsi-multipath: provide callbacks for path state John Garry
@ 2026-07-03 11:49 ` sashiko-bot
2026-07-06 15:38 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 11:49 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `scsi_mpath_is_disabled` and `scsi_mpath_is_optimized` fail to mask out the `SCSI_ACCESS_STATE_PREFERRED` flag from `sdev->access_state`.
- [High] `scsi_mpath_available_path()` incorrectly treats `SDEV_CREATED_BLOCK` as an available path.
--
commit 71967b8cc1b7e6dc50c814782c317913564b9ee9
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: provide callbacks for path state
This patch provides callbacks for path states in SCSI multipath, including
scsi_mpath_is_disabled, scsi_mpath_is_optimized, and
scsi_mpath_available_path. These rely on the SCSI device's sdev_state and
access_state to determine path availability.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index f22e3677cf2ad..d8ea9ffe8942c 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
> @@ -301,7 +301,53 @@ static struct bio *scsi_mpath_clone_bio(struct bio *bio)
[ ... ]
> +static bool scsi_mpath_is_disabled(struct mpath_device *mpath_device)
> +{
> + struct scsi_mpath_device *scsi_mpath_dev =
> + to_scsi_mpath_device(mpath_device);
> + struct scsi_device *sdev = scsi_mpath_dev->sdev;
> + unsigned char access_state = READ_ONCE(sdev->access_state);
> +
> + if (sdev->sdev_state != SDEV_RUNNING)
> + return true;
> +
> + if (access_state == SCSI_ACCESS_STATE_OPTIMAL ||
> + access_state == SCSI_ACCESS_STATE_ACTIVE)
[Severity: High]
Does this comparison correctly handle ALUA preferred paths?
If a device handler sets sdev->access_state to include the preferred bit
(SCSI_ACCESS_STATE_OPTIMAL | SCSI_ACCESS_STATE_PREFERRED), this raw byte
comparison will fail. This would cause the multipath layer to incorrectly
classify valid, optimal preferred paths as disabled, potentially discarding
them from I/O consideration.
Should this mask the access_state with SCSI_ACCESS_STATE_MASK before
comparing?
> + return false;
> +
> + return true;
> +}
> +
> +static bool scsi_mpath_is_optimized(struct mpath_device *mpath_device)
> +{
> + struct scsi_mpath_device *scsi_mpath_dev =
> + to_scsi_mpath_device(mpath_device);
> + struct scsi_device *sdev = scsi_mpath_dev->sdev;
> +
> + if (sdev->sdev_state != SDEV_RUNNING)
> + return false;
> +
> + return READ_ONCE(sdev->access_state) == SCSI_ACCESS_STATE_OPTIMAL;
[Severity: High]
Similar to the check in scsi_mpath_is_disabled(), will this strict equality
check misclassify optimal paths that also have the
SCSI_ACCESS_STATE_PREFERRED flag set?
> +}
> +
> +static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
> +{
> + struct scsi_mpath_device *scsi_mpath_dev =
> + to_scsi_mpath_device(mpath_device);
> + struct scsi_device *sdev = scsi_mpath_dev->sdev;
> + enum scsi_device_state sdev_state = sdev->sdev_state;
> +
> + if (sdev_state == SDEV_RUNNING || sdev_state == SDEV_QUIESCE ||
> + sdev_state == SDEV_BLOCK || sdev_state == SDEV_CREATED_BLOCK)
[Severity: High]
Is it intended to route user I/O to a device in the SDEV_CREATED_BLOCK state?
Devices in SDEV_CREATED and SDEV_CREATED_BLOCK are newly created, not fully
initialized, and generally only accept internal commands. If a newly created
device is temporarily blocked, this function returns true and routes user I/O
to it, which could lead to I/O rejection or indefinite blocking.
Furthermore, when the device unblocks and returns to SDEV_CREATED, this
function would suddenly return false. Should SDEV_CREATED_BLOCK be excluded
from the available paths list?
> + return true;
> +
> + return false;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=8
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 08/17] scsi-multipath: provide callbacks for path state
2026-07-03 11:49 ` sashiko-bot
@ 2026-07-06 15:38 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:38 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 12:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] `scsi_mpath_is_disabled` and `scsi_mpath_is_optimized` fail to mask out the `SCSI_ACCESS_STATE_PREFERRED` flag from `sdev->access_state`.
> - [High] `scsi_mpath_available_path()` incorrectly treats `SDEV_CREATED_BLOCK` as an available path.
> --
>
> commit 71967b8cc1b7e6dc50c814782c317913564b9ee9
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi-multipath: provide callbacks for path state
>
> This patch provides callbacks for path states in SCSI multipath, including
> scsi_mpath_is_disabled, scsi_mpath_is_optimized, and
> scsi_mpath_available_path. These rely on the SCSI device's sdev_state and
> access_state to determine path availability.
>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index f22e3677cf2ad..d8ea9ffe8942c 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
>> @@ -301,7 +301,53 @@ static struct bio *scsi_mpath_clone_bio(struct bio *bio)
>
> [ ... ]
>
>> +static bool scsi_mpath_is_disabled(struct mpath_device *mpath_device)
>> +{
>> + struct scsi_mpath_device *scsi_mpath_dev =
>> + to_scsi_mpath_device(mpath_device);
>> + struct scsi_device *sdev = scsi_mpath_dev->sdev;
>> + unsigned char access_state = READ_ONCE(sdev->access_state);
>> +
>> + if (sdev->sdev_state != SDEV_RUNNING)
>> + return true;
>> +
>> + if (access_state == SCSI_ACCESS_STATE_OPTIMAL ||
>> + access_state == SCSI_ACCESS_STATE_ACTIVE)
>
> [Severity: High]
> Does this comparison correctly handle ALUA preferred paths?
>
> If a device handler sets sdev->access_state to include the preferred bit
> (SCSI_ACCESS_STATE_OPTIMAL | SCSI_ACCESS_STATE_PREFERRED), this raw byte
> comparison will fail. This would cause the multipath layer to incorrectly
> classify valid, optimal preferred paths as disabled, potentially discarding
> them from I/O consideration.
>
> Should this mask the access_state with SCSI_ACCESS_STATE_MASK before
> comparing?
will fix
>
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static bool scsi_mpath_is_optimized(struct mpath_device *mpath_device)
>> +{
>> + struct scsi_mpath_device *scsi_mpath_dev =
>> + to_scsi_mpath_device(mpath_device);
>> + struct scsi_device *sdev = scsi_mpath_dev->sdev;
>> +
>> + if (sdev->sdev_state != SDEV_RUNNING)
>> + return false;
>> +
>> + return READ_ONCE(sdev->access_state) == SCSI_ACCESS_STATE_OPTIMAL;
>
> [Severity: High]
> Similar to the check in scsi_mpath_is_disabled(), will this strict equality
> check misclassify optimal paths that also have the
> SCSI_ACCESS_STATE_PREFERRED flag set?
will fix
>
>> +}
>> +
>> +static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
>> +{
>> + struct scsi_mpath_device *scsi_mpath_dev =
>> + to_scsi_mpath_device(mpath_device);
>> + struct scsi_device *sdev = scsi_mpath_dev->sdev;
>> + enum scsi_device_state sdev_state = sdev->sdev_state;
>> +
>> + if (sdev_state == SDEV_RUNNING || sdev_state == SDEV_QUIESCE ||
>> + sdev_state == SDEV_BLOCK || sdev_state == SDEV_CREATED_BLOCK)
>
> [Severity: High]
> Is it intended to route user I/O to a device in the SDEV_CREATED_BLOCK state?
>
> Devices in SDEV_CREATED and SDEV_CREATED_BLOCK are newly created, not fully
> initialized, and generally only accept internal commands. If a newly created
> device is temporarily blocked, this function returns true and routes user I/O
> to it, which could lead to I/O rejection or indefinite blocking.
It does not specifically route IO to it, but allows IO to be requeued
for the head to be issued later
>
> Furthermore, when the device unblocks and returns to SDEV_CREATED, this
> function would suddenly return false. Should SDEV_CREATED_BLOCK be excluded
> from the available paths list?
probably
>
>> + return true;
>> +
>> + return false;
>> +}
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 09/17] scsi-multipath: add scsi_mpath_{start,end}_request()
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (7 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 08/17] scsi-multipath: provide callbacks for path state John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 11:53 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 10/17] scsi-multipath: block PR commands John Garry
` (7 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add scsi_mpath_{start,end}_request() to handle updating private multipath
request data, like nvme_mpath_{start,end}_request().
New member Scsi_Host.mpath_nr_active is added. This is required for queue
depth multipath iopolicy.
For NVMe, this count is per controller. The reason is that many NSes may
be connected to a controller, so congestion should be judged at
controller level.
SCSI has no definition of a controller, but SCSI host is a comparable
concept.
Indeed, many SCSI disks may be connected to the same SCSI host, so it
makes sense to count number of active requests at this point. However,
for a transport like iSCSI Initiator over TCP/IP, we have a separate SCSI
host per SCSI device (so there the count would be same at SCSI device
level).
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_lib.c | 6 +++++
drivers/scsi/scsi_multipath.c | 51 +++++++++++++++++++++++++++++++++++
include/scsi/scsi_cmnd.h | 5 ++++
include/scsi/scsi_host.h | 4 +++
include/scsi/scsi_multipath.h | 8 ++++++
5 files changed, 74 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index cc34253c467a2..f8b389fda2537 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -669,6 +669,9 @@ static bool scsi_end_request(struct request *req, blk_status_t error,
struct scsi_device *sdev = cmd->device;
struct request_queue *q = sdev->request_queue;
+ if (is_mpath_request(req))
+ scsi_mpath_end_request(req);
+
if (blk_update_request(req, error, bytes))
return true;
@@ -1917,6 +1920,9 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
cmd->submitter = SUBMITTED_BY_BLOCK_LAYER;
+ if (is_mpath_request(req))
+ scsi_mpath_start_request(req);
+
blk_mq_start_request(req);
if (blk_mq_is_reserved_rq(req)) {
reason = shost->hostt->queue_reserved_command(shost, cmd);
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index d8ea9ffe8942c..61fa2e4cdfdab 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -543,6 +543,57 @@ void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head)
}
EXPORT_SYMBOL_GPL(scsi_mpath_put_head);
+void scsi_mpath_start_request(struct request *req)
+{
+ struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
+ struct scsi_device *sdev = scmd->device;
+ struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
+ struct scsi_mpath_head *scsi_mpath_head =
+ scsi_mpath_dev->scsi_mpath_head;
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+ struct gendisk *disk = mpath_head->disk;
+
+ if (mpath_qd_iopolicy(&scsi_mpath_head->iopolicy) &&
+ !(scmd->flags & SCMD_MPATH_CNT_ACTIVE)) {
+ struct Scsi_Host *shost = sdev->host;
+
+ atomic_inc(&shost->mpath_nr_active);
+ scmd->flags |= SCMD_MPATH_CNT_ACTIVE;
+ }
+
+ if (!blk_queue_io_stat(disk->queue) || blk_rq_is_passthrough(req) ||
+ (scmd->flags & SCMD_MPATH_IO_STATS))
+ return;
+
+ scmd->flags |= SCMD_MPATH_IO_STATS;
+ scmd->start_time = bdev_start_io_acct(disk->part0, req_op(req),
+ jiffies);
+}
+
+void scsi_mpath_end_request(struct request *req)
+{
+ struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
+ struct scsi_device *sdev = scmd->device;
+ struct scsi_mpath_device *scsi_mpath_dev =
+ sdev->scsi_mpath_dev;
+ struct scsi_mpath_head *scsi_mpath_head =
+ scsi_mpath_dev->scsi_mpath_head;
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+ struct gendisk *disk = mpath_head->disk;
+
+ if (scmd->flags & SCMD_MPATH_CNT_ACTIVE) {
+ struct Scsi_Host *shost = sdev->host;
+
+ atomic_dec_if_positive(&shost->mpath_nr_active);
+ }
+
+ if (!(scmd->flags & SCMD_MPATH_IO_STATS))
+ return;
+ bdev_end_io_acct(disk->part0, req_op(req),
+ blk_rq_bytes(req) >> SECTOR_SHIFT,
+ scmd->start_time);
+}
+
int __init scsi_multipath_init(void)
{
return class_register(&scsi_mpath_device_class);
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 8ecfb94049db5..c6571a36e577b 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -60,6 +60,8 @@ struct scsi_pointer {
#define SCMD_FAIL_IF_RECOVERING (1 << 4)
/* flags preserved across unprep / reprep */
#define SCMD_PRESERVED_FLAGS (SCMD_INITIALIZED | SCMD_FAIL_IF_RECOVERING)
+#define SCMD_MPATH_IO_STATS (1 << 5)
+#define SCMD_MPATH_CNT_ACTIVE (1 << 6)
/* for scmd->state */
#define SCMD_STATE_COMPLETE 0
@@ -139,6 +141,9 @@ struct scsi_cmnd {
* to release this memory. (The memory
* obtained by scsi_malloc is guaranteed
* to be at an address < 16Mb). */
+ #ifdef CONFIG_SCSI_MULTIPATH
+ unsigned long start_time;
+ #endif
int result; /* Status code from lower level driver */
};
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 7e2011830ba4b..cf4fff3ea1a8e 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -753,6 +753,10 @@ struct Scsi_Host {
/* Delay for runtime autosuspend */
int rpm_autosuspend_delay;
+ #ifdef CONFIG_SCSI_MULTIPATH
+ atomic_t mpath_nr_active;
+ #endif
+
/*
* We should ensure that this is aligned, both for better performance
* and also because some compilers (m68k) don't automatically force
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
index d8644d6261992..ed5c77d568843 100644
--- a/include/scsi/scsi_multipath.h
+++ b/include/scsi/scsi_multipath.h
@@ -55,6 +55,8 @@ void scsi_mpath_add_sysfs_link(struct scsi_device *sdev);
void scsi_mpath_remove_sysfs_link(struct scsi_device *sdev);
int scsi_mpath_get_head(struct scsi_mpath_head *scsi_mpath_head);
void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head);
+void scsi_mpath_start_request(struct request *req);
+void scsi_mpath_end_request(struct request *req);
#else /* CONFIG_SCSI_MULTIPATH */
struct scsi_mpath_head {
@@ -93,6 +95,12 @@ static inline
void scsi_mpath_put_head(struct scsi_mpath_head *scsi_mpath_head)
{
}
+static inline void scsi_mpath_start_request(struct request *req)
+{
+}
+static inline void scsi_mpath_end_request(struct request *req)
+{
+}
static inline void scsi_mpath_add_sysfs_link(struct scsi_device *sdev)
{
}
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 09/17] scsi-multipath: add scsi_mpath_{start,end}_request()
2026-07-03 10:33 ` [PATCH v3 09/17] scsi-multipath: add scsi_mpath_{start,end}_request() John Garry
@ 2026-07-03 11:53 ` sashiko-bot
2026-07-06 15:44 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 11:53 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Requeueing a command causes permanent leaks of `mpath_nr_active` and block device in-flight I/O statistics because `SCMD_MPATH_*` flags are not preserved.
- [High] Premature termination of I/O accounting and double-accounting on partial request completions because `scsi_mpath_end_request()` is called before `blk_update_request()`.
- [Low] Implicit function declarations of `is_mpath_request` and `mpath_qd_iopolicy` cause build failures.
--
commit 6d78b641877657fb26302d1c7965b47fd751383b
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: add scsi_mpath_{start,end}_request()
This patch adds scsi_mpath_start_request() and scsi_mpath_end_request() to
handle updating private multipath request data. It also introduces
mpath_nr_active to Scsi_Host to support queue depth multipath iopolicy.
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 3c493068adb98..72ba066235911 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -669,6 +669,9 @@ static bool scsi_end_request(struct request *req, blk_status_t error,
> struct scsi_device *sdev = cmd->device;
> struct request_queue *q = sdev->request_queue;
>
> + if (is_mpath_request(req))
> + scsi_mpath_end_request(req);
> +
[Severity: Low]
Are the functions is_mpath_request() and mpath_qd_iopolicy() defined anywhere?
It looks like they are used in this patch but not declared, which would cause
a build failure.
[Severity: High]
Does calling scsi_mpath_end_request() before blk_update_request() cause double
accounting on partial completions?
If blk_update_request() returns true for a partial completion, the request is
retried. Since scsi_mpath_end_request() has already decremented
mpath_nr_active and called bdev_end_io_acct() without clearing the state
flags, a later completion will decrement these again. Should this be called
after blk_update_request() finishes the entire request?
> if (blk_update_request(req, error, bytes))
> return true;
>
[ ... ]
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 8ecfb94049db5..c6571a36e577b 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -60,6 +60,8 @@ struct scsi_pointer {
> #define SCMD_FAIL_IF_RECOVERING (1 << 4)
> /* flags preserved across unprep / reprep */
> #define SCMD_PRESERVED_FLAGS (SCMD_INITIALIZED | SCMD_FAIL_IF_RECOVERING)
> +#define SCMD_MPATH_IO_STATS (1 << 5)
> +#define SCMD_MPATH_CNT_ACTIVE (1 << 6)
[Severity: High]
Can we leak mpath_nr_active and in-flight I/O statistics when a command is
requeued?
In scsi_queue_rq(), cmd->flags is masked with SCMD_PRESERVED_FLAGS. Because
SCMD_MPATH_IO_STATS and SCMD_MPATH_CNT_ACTIVE are not added to
SCMD_PRESERVED_FLAGS, they are cleared on every retry.
When scsi_mpath_start_request() runs again on a retry, it will see the flags
as unset, increment mpath_nr_active, and call bdev_start_io_acct() a second
time. Upon completion, scsi_mpath_end_request() only cleans up once, leaving a
permanent leak. Should these new flags be added to SCMD_PRESERVED_FLAGS?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=9
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 09/17] scsi-multipath: add scsi_mpath_{start,end}_request()
2026-07-03 11:53 ` sashiko-bot
@ 2026-07-06 15:44 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:44 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 12:53, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Requeueing a command causes permanent leaks of `mpath_nr_active` and block device in-flight I/O statistics because `SCMD_MPATH_*` flags are not preserved.
> - [High] Premature termination of I/O accounting and double-accounting on partial request completions because `scsi_mpath_end_request()` is called before `blk_update_request()`.
> - [Low] Implicit function declarations of `is_mpath_request` and `mpath_qd_iopolicy` cause build failures.
> --
>
> commit 6d78b641877657fb26302d1c7965b47fd751383b
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi-multipath: add scsi_mpath_{start,end}_request()
>
> This patch adds scsi_mpath_start_request() and scsi_mpath_end_request() to
> handle updating private multipath request data. It also introduces
> mpath_nr_active to Scsi_Host to support queue depth multipath iopolicy.
>
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index 3c493068adb98..72ba066235911 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -669,6 +669,9 @@ static bool scsi_end_request(struct request *req, blk_status_t error,
>> struct scsi_device *sdev = cmd->device;
>> struct request_queue *q = sdev->request_queue;
>>
>> + if (is_mpath_request(req))
>> + scsi_mpath_end_request(req);
>> +
>
> [Severity: Low]
> Are the functions is_mpath_request() and mpath_qd_iopolicy() defined anywhere?
> It looks like they are used in this patch but not declared, which would cause
> a build failure.
>
> [Severity: High]
> Does calling scsi_mpath_end_request() before blk_update_request() cause double
> accounting on partial completions?
>
> If blk_update_request() returns true for a partial completion, the request is
> retried. Since scsi_mpath_end_request() has already decremented
> mpath_nr_active and called bdev_end_io_acct() without clearing the state
> flags, a later completion will decrement these again. Should this be called
> after blk_update_request() finishes the entire request?
There was an ordering issue previously, in that calling after
blk_update_request() meant that the request size was then zero. I'll
check this further.
>
>> if (blk_update_request(req, error, bytes))
>> return true;
>>
> [ ... ]
>> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
>> index 8ecfb94049db5..c6571a36e577b 100644
>> --- a/include/scsi/scsi_cmnd.h
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 10/17] scsi-multipath: block PR commands
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (8 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 09/17] scsi-multipath: add scsi_mpath_{start,end}_request() John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 12:03 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 11/17] scsi-multipath: add delayed disk removal support John Garry
` (6 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
As described by Benjamin in the following link, PR support for SCSI is
quite complicated:
https://lore.kernel.org/linux-scsi/aaHecneNg9Q8EtiS@redhat.com/
For initial scsi-multipath support, just don't support PRs. This means that
we need to intercept PR SCSI commands for passthrough and reject them.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_lib.c | 6 ++++++
drivers/scsi/scsi_multipath.c | 11 +++++++++++
include/scsi/scsi_multipath.h | 5 +++++
3 files changed, 22 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f8b389fda2537..42f6065c7d48b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1320,6 +1320,12 @@ static blk_status_t scsi_setup_scsi_cmnd(struct scsi_device *sdev,
{
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
+ if (sdev->scsi_mpath_dev) {
+ blk_status_t ret = scsi_mpath_setup_scsi_cmnd(cmd);
+ if (ret)
+ return ret;
+ }
+
/*
* Passthrough requests may transfer data, in which case they must
* a bio attached to them. Or they might contain a SCSI command
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index 61fa2e4cdfdab..b4d9d6518b4fe 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -242,6 +242,17 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
return 0;
}
+blk_status_t scsi_mpath_setup_scsi_cmnd(struct scsi_cmnd *scmd)
+{
+ switch (scmd->cmnd[0]) {
+ /* Special handling required which is not yet supported */
+ case PERSISTENT_RESERVE_IN:
+ case PERSISTENT_RESERVE_OUT:
+ return BLK_STS_NOTSUPP;
+ }
+ return BLK_STS_OK;
+}
+
static inline void bio_list_add_clone(struct bio_list *bl,
struct bio *clone)
{
diff --git a/include/scsi/scsi_multipath.h b/include/scsi/scsi_multipath.h
index ed5c77d568843..520e4761fa0f1 100644
--- a/include/scsi/scsi_multipath.h
+++ b/include/scsi/scsi_multipath.h
@@ -45,6 +45,7 @@ struct scsi_mpath_device {
#define to_scsi_mpath_head(d) \
container_of(d, struct scsi_mpath_head, mpath_head)
+blk_status_t scsi_mpath_setup_scsi_cmnd(struct scsi_cmnd *scmd);
int scsi_mpath_dev_alloc(struct scsi_device *sdev);
void scsi_mpath_dev_release(struct scsi_device *sdev);
int scsi_multipath_init(void);
@@ -64,6 +65,10 @@ struct scsi_mpath_head {
struct scsi_mpath_device {
};
+static inline blk_status_t scsi_mpath_setup_scsi_cmnd(struct scsi_cmnd *scmd)
+{
+ return BLK_STS_OK;
+}
static inline int scsi_mpath_dev_alloc(struct scsi_device *sdev)
{
return 0;
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 10/17] scsi-multipath: block PR commands
2026-07-03 10:33 ` [PATCH v3 10/17] scsi-multipath: block PR commands John Garry
@ 2026-07-03 12:03 ` sashiko-bot
0 siblings, 0 replies; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 12:03 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Double-free and use-after-free of `scsi_mpath_head` and its resources on the `dev_set_name` failure path.
- [High] Calling `list_del_init` on an uninitialized list_head and `device_unregister` on a failed device addition.
--
commit 3be4a33456416864c9e49c5cfb76773fb5375009
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: block PR commands
For initial scsi-multipath support, do not support Persistent Reservations
(PRs). This change intercepts PR SCSI commands for passthrough and rejects
them since PR support for SCSI is quite complicated.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 61fa2e4cdfdab..b4d9d6518b4fe 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
While reviewing this file, I noticed a couple of pre-existing issues:
drivers/scsi/scsi_multipath.c:scsi_mpath_alloc_head() {
...
ret = dev_set_name(&scsi_mpath_head->dev, "scsi_mpath_device%d",
scsi_mpath_head->index);
if (ret) {
put_device(&scsi_mpath_head->dev);
goto out_free_ida;
}
...
}
[Severity: High]
This is a pre-existing issue, but can this lead to a double-free?
If dev_set_name() fails, the error path calls put_device(). Since
device_initialize() sets the kref count to 1, this drops the count to 0 and
immediately invokes the release function scsi_mpath_head_release().
The scsi_mpath_head_release() function will free the scsi_mpath_head memory
via kfree() as well as associated resources like the IDA and bioset.
The code then incorrectly falls through to out_free_ida, executing ida_free(),
mpath_put_head(), bioset_exit(), and a second kfree() on the already freed
scsi_mpath_head pointer. Will this result in a double-free and memory
corruption?
A similar issue appears in a different error path:
drivers/scsi/scsi_multipath.c:scsi_mpath_dev_alloc() {
...
ret = device_add(&scsi_mpath_head->dev);
if (ret) {
mutex_unlock(&scsi_mpath_heads_lock);
goto out_put_head;
}
...
}
Which eventually calls the release function:
drivers/scsi/scsi_multipath.c:scsi_mpath_free_head() {
...
list_del_init(&scsi_mpath_head->entry);
mutex_unlock(&scsi_mpath_heads_lock);
device_unregister(&scsi_mpath_head->dev);
...
}
[Severity: High]
This is a pre-existing issue, but does this release function operate on an
unlinked list head and a partially initialized device?
In scsi_mpath_dev_alloc(), if device_add() fails, it jumps to out_put_head and
calls scsi_mpath_put_head(). This drops the reference to 0 and invokes
scsi_mpath_free_head().
When scsi_mpath_free_head() calls list_del_init(&scsi_mpath_head->entry), the
entry is uninitialized (zeroed by kzalloc) because device_add() failed before
list_add_tail() was ever called. Can this result in a NULL pointer dereference
when __list_del_entry() attempts to link the pointers?
Furthermore, it then unconditionally calls device_unregister(). Does this
incorrectly attempt to device_del() a device that was never successfully added,
leading to driver core warnings or crashes?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=10
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 11/17] scsi-multipath: add delayed disk removal support
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (9 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 10/17] scsi-multipath: block PR commands John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 12:08 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 12/17] scsi: sd: add multipath disk class John Garry
` (5 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add support in core code for delayed disk removal support. In this, the
callback calls into the scsi_driver to do the necessary removal work.
The scsi_disk driver (sd) must ensure that the scsi_mpath_device does not
go away while the delayed removal work is active, i.e. it must keep a
reference.
No reference to the scsi_disk multipath structures are kept outside that
driver, so that driver needs to provide a scsi_driver.mpath_remove_head
callback to do the necessary work.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_multipath.c | 18 ++++++++++++++++++
include/scsi/scsi_driver.h | 4 ++++
2 files changed, 22 insertions(+)
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index b4d9d6518b4fe..c947c71d15ef1 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -355,7 +355,25 @@ static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
return false;
}
+static int scsi_mpath_remove_head_drv(struct device_driver *drv, void *data)
+{
+ struct scsi_mpath_head *scsi_mpath_head = data;
+ struct scsi_driver *scsi_driver = to_scsi_driver(drv);
+
+ if (scsi_driver->mpath_remove_head)
+ scsi_driver->mpath_remove_head(scsi_mpath_head);
+
+ return 0;
+}
+
+static void scsi_mpath_remove_head_work(struct mpath_head *mpath_head)
+{
+ bus_for_each_drv(&scsi_bus_type, NULL, to_scsi_mpath_head(mpath_head),
+ scsi_mpath_remove_head_drv);
+}
+
static struct mpath_head_template smpdt = {
+ .remove_head = scsi_mpath_remove_head_work,
.is_disabled = scsi_mpath_is_disabled,
.is_optimized = scsi_mpath_is_optimized,
.available_path = scsi_mpath_available_path,
diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
index 249cea724abd1..d92b63d357f2a 100644
--- a/include/scsi/scsi_driver.h
+++ b/include/scsi/scsi_driver.h
@@ -8,6 +8,7 @@
struct module;
struct request;
+struct scsi_mpath_head;
struct scsi_driver {
struct device_driver gendrv;
@@ -22,6 +23,9 @@ struct scsi_driver {
int (*done)(struct scsi_cmnd *);
int (*eh_action)(struct scsi_cmnd *, int);
void (*eh_reset)(struct scsi_cmnd *);
+ #ifdef CONFIG_SCSI_MULTIPATH
+ void (*mpath_remove_head)(struct scsi_mpath_head *);
+ #endif
};
#define to_scsi_driver(drv) \
container_of((drv), struct scsi_driver, gendrv)
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 11/17] scsi-multipath: add delayed disk removal support
2026-07-03 10:33 ` [PATCH v3 11/17] scsi-multipath: add delayed disk removal support John Garry
@ 2026-07-03 12:08 ` sashiko-bot
2026-07-06 15:49 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 12:08 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Memory leak during module unload due to flawed bus driver broadcast for delayed removal.
- [Medium] Use-After-Free (UAF) hazard during SCSI driver iteration because the iterator continues after the structure is freed.
--
commit b2a49437af44776426456e634c1ad321526ae49c
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: add delayed disk removal support
This commit adds core support for delayed disk removal in SCSI multipath.
It introduces a new scsi_driver callback to handle the removal work,
ensuring that the driver maintains a reference to the multipath device while
active.
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
> @@ -355,7 +355,25 @@ static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
> +static int scsi_mpath_remove_head_drv(struct device_driver *drv, void *data)
> +{
> + struct scsi_mpath_head *scsi_mpath_head = data;
> + struct scsi_driver *scsi_driver = to_scsi_driver(drv);
> +
> + if (scsi_driver->mpath_remove_head)
> + scsi_driver->mpath_remove_head(scsi_mpath_head);
> +
> + return 0;
[Severity: Medium]
Is there a potential use-after-free hazard during the bus iteration here?
When a device hot-unplug triggers delayed head removal, the sd driver will
process it and drop the final reference to the scsi_mpath_head structure,
freeing it.
Because this iterator callback unconditionally returns 0, bus_for_each_drv()
will continue iterating through other registered SCSI drivers (like sr).
Could this result in passing a freed scsi_mpath_head pointer to subsequent
drivers in the iteration? Should the iteration abort once the target driver
successfully handles the removal?
> +}
> +
> +static void scsi_mpath_remove_head_work(struct mpath_head *mpath_head)
> +{
> + bus_for_each_drv(&scsi_bus_type, NULL, to_scsi_mpath_head(mpath_head),
> + scsi_mpath_remove_head_drv);
[Severity: High]
Can this leak memory during module unload?
When the sd module is unloaded, driver_unregister() calls bus_remove_driver(),
which removes the driver from the bus's klist_drivers list before calling
driver_detach() to unbind devices.
If detaching the devices triggers a delayed removal of the multipath head,
this workqueue executes and attempts to find the responsible driver by
broadcasting to all drivers on scsi_bus_type. Since the sd driver was already
removed from the bus list, the iterator fails to find it.
Does this cause the targeted mpath_remove_head callback to be missed,
permanently leaking the scsi_mpath_head and sd_mpath_disk structures?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=11
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 11/17] scsi-multipath: add delayed disk removal support
2026-07-03 12:08 ` sashiko-bot
@ 2026-07-06 15:49 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:49 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 13:08, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Memory leak during module unload due to flawed bus driver broadcast for delayed removal.
> - [Medium] Use-After-Free (UAF) hazard during SCSI driver iteration because the iterator continues after the structure is freed.
> --
>
> commit b2a49437af44776426456e634c1ad321526ae49c
> Author: John Garry <john.g.garry@oracle.com>
> scsi-multipath: add delayed disk removal support
>
> This commit adds core support for delayed disk removal in SCSI multipath.
> It introduces a new scsi_driver callback to handle the removal work,
> ensuring that the driver maintains a reference to the multipath device while
> active.
>
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
>
> [ ... ]
>
>> @@ -355,7 +355,25 @@ static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
>> +static int scsi_mpath_remove_head_drv(struct device_driver *drv, void *data)
>> +{
>> + struct scsi_mpath_head *scsi_mpath_head = data;
>> + struct scsi_driver *scsi_driver = to_scsi_driver(drv);
>> +
>> + if (scsi_driver->mpath_remove_head)
>> + scsi_driver->mpath_remove_head(scsi_mpath_head);
>> +
>> + return 0;
>
> [Severity: Medium]
> Is there a potential use-after-free hazard during the bus iteration here?
>
> When a device hot-unplug triggers delayed head removal, the sd driver will
> process it and drop the final reference to the scsi_mpath_head structure,
> freeing it.
scsi_mpath_head should can or should not be free at that point.
>
> Because this iterator callback unconditionally returns 0, bus_for_each_drv()
> will continue iterating through other registered SCSI drivers (like sr).
>
> Could this result in passing a freed scsi_mpath_head pointer to subsequent
> drivers in the iteration? Should the iteration abort once the target driver
> successfully handles the removal?
>
>> +}
>> +
>> +static void scsi_mpath_remove_head_work(struct mpath_head *mpath_head)
>> +{
>> + bus_for_each_drv(&scsi_bus_type, NULL, to_scsi_mpath_head(mpath_head),
>> + scsi_mpath_remove_head_drv);
>
> [Severity: High]
> Can this leak memory during module unload?
>
> When the sd module is unloaded, driver_unregister() calls bus_remove_driver(),
> which removes the driver from the bus's klist_drivers list before calling
> driver_detach() to unbind devices.
>
> If detaching the devices triggers a delayed removal of the multipath head,
> this workqueue executes and attempts to find the responsible driver by
> broadcasting to all drivers on scsi_bus_type. Since the sd driver was already
> removed from the bus list, the iterator fails to find it.
>
> Does this cause the targeted mpath_remove_head callback to be missed,
> permanently leaking the scsi_mpath_head and sd_mpath_disk structures?
if the sd driver is remove, then the scsi_mpath_head and sd_mpath_disk
must be dropped via normal unloading paths
>
>> +}
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 12/17] scsi: sd: add multipath disk class
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (10 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 11/17] scsi-multipath: add delayed disk removal support John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 10:33 ` [PATCH v3 13/17] scsi: sd: support multipath disk John Garry
` (4 subsequent siblings)
16 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add a new class, sd_mpath_disk_class, which is the multipath version of
the scsi_disk class.
Structure sd_mpath_disk is introduced to manage the multipath gendisk.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/sd.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
drivers/scsi/sd.h | 3 +++
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 599e75f333343..242a15bc2c5bb 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -70,6 +70,7 @@
#include <scsi/scsi_ioctl.h>
#include <scsi/scsicam.h>
#include <scsi/scsi_common.h>
+#include <scsi/scsi_multipath.h>
#include "sd.h"
#include "scsi_priv.h"
@@ -113,6 +114,39 @@ static mempool_t *sd_page_pool;
static mempool_t *sd_large_page_pool;
static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
static struct lock_class_key sd_bio_compl_lkclass;
+#ifdef CONFIG_SCSI_MULTIPATH
+struct sd_mpath_disk {
+ struct scsi_mpath_head *scsi_mpath_head;
+};
+
+static void sd_mpath_disk_release(struct device *dev)
+{
+}
+
+static const struct class sd_mpath_disk_class = {
+ .name = "scsi_mpath_disk",
+ .dev_release = sd_mpath_disk_release,
+};
+
+static int sd_mpath_class_register(void)
+{
+ return class_register(&sd_mpath_disk_class);
+}
+
+static void sd_mpath_class_unregister(void)
+{
+ class_unregister(&sd_mpath_disk_class);
+}
+#else /* CONFIG_SCSI_MULTIPATH */
+static int sd_mpath_class_register(void)
+{
+ return 0;
+}
+
+static void sd_mpath_class_unregister(void)
+{
+}
+#endif
static const char *sd_cache_types[] = {
"write through", "none", "write back",
@@ -4453,11 +4487,15 @@ static int __init init_sd(void)
if (err)
goto err_out;
+ err = sd_mpath_class_register();
+ if (err)
+ goto err_out_class;
+
sd_page_pool = mempool_create_page_pool(SD_MEMPOOL_SIZE, 0);
if (!sd_page_pool) {
printk(KERN_ERR "sd: can't init discard page pool\n");
err = -ENOMEM;
- goto err_out_class;
+ goto err_out_mpath_class;
}
err = scsi_register_driver(&sd_template);
@@ -4468,6 +4506,8 @@ static int __init init_sd(void)
err_out_driver:
mempool_destroy(sd_page_pool);
+err_out_mpath_class:
+ sd_mpath_class_unregister();
err_out_class:
class_unregister(&sd_disk_class);
err_out:
@@ -4493,6 +4533,7 @@ static void __exit exit_sd(void)
mempool_destroy(sd_large_page_pool);
class_unregister(&sd_disk_class);
+ sd_mpath_class_unregister();
for (i = 0; i < SD_MAJORS; i++)
unregister_blkdev(sd_major(i), "sd");
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 574af82430169..304b24644d942 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -83,6 +83,9 @@ struct zoned_disk_info {
struct scsi_disk {
struct scsi_device *device;
+ #ifdef CONFIG_SCSI_MULTIPATH
+ struct sd_mpath_disk *sd_mpath_disk;
+ #endif
/*
* disk_dev is used to show attributes in /sys/class/scsi_disk/,
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3 13/17] scsi: sd: support multipath disk
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (11 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 12/17] scsi: sd: add multipath disk class John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 12:24 ` sashiko-bot
2026-07-03 10:33 ` [PATCH v3 14/17] scsi: sd: add multipath disk attr groups John Garry
` (3 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add support to attach a multipath disk.
We still allocate the gendisk per path, and this is required for the
per-path submission. However, those gendisks are marked as hidden. Those
disks are named sdX:Y, where X is the multipath disk index and Y is the
per-path index.
A global list of sd_mpath_disks is kept for matching scsi_device's.
The multipath gendisk has the name and disk->major/minor set to minic a
scsi_disk.
The following is an example of relevant scsi_disk and block sysfs
directories:
$ ls -l /sys/block/ | grep sdc
lrwxrwxrwx 1 root root 0 Feb 24 16:01 sdc -> ../devices/virtual/scsi_mpath_disk/0/sdc
lrwxrwxrwx 1 root root 0 Feb 24 16:01 sdc:0 -> ../devices/platform/host8/session1/target8:0:0/8:0:0:0/block/sdc:0
lrwxrwxrwx 1 root root 0 Feb 24 16:02 sdc:1 -> ../devices/platform/host9/session2/target9:0:0/9:0:0:0/block/sdc:1
$ ls -l /sys/class/scsi_mpath_disk/scsi_mpath_disk0/
total 0
drwxr-xr-x 2 root root 0 Feb 24 16:03 power
drwxr-xr-x 11 root root 0 Feb 24 16:01 sdc
lrwxrwxrwx 1 root root 0 Feb 24 16:01 subsystem -> ../../../../class/scsi_mpath_disk
-rw-r--r-- 1 root root 4096 Feb 24 16:01 uevent
$ ls -l /sys/class/scsi_mpath_disk/scsi_mpath_disk0/sdc/multipath/
total 0
lrwxrwxrwx 1 root root 0 Feb 24 16:20 sdc:0 -> ../../../../../platform/host8/session1/target8:0:0/8:0:0:0/block/sdc:0
lrwxrwxrwx 1 root root 0 Feb 24 16:20 sdc:1 -> ../../../../../platform/host9/session2/target9:0:0/9:0:0:0/block/sdc:1
$ ls -l /dev/sdc*
brw-rw---- 1 root disk 8, 32 Feb 24 16:01 /dev/sdc
brw-rw---- 1 root disk 8, 33 Feb 24 16:01 /dev/sdc1
brw-rw---- 1 root disk 8, 34 Feb 24 16:01 /dev/sdc2
$ lsblk /dev/sdc
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINTS
sdc 8:32 0 600M 0 disk
|-sdc1 8:33 0 9M 0 part
`-sdc2 8:34 0 568M 0 part
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/sd.c | 395 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 377 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 242a15bc2c5bb..3df70b24b688e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -115,12 +115,30 @@ static mempool_t *sd_large_page_pool;
static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
static struct lock_class_key sd_bio_compl_lkclass;
#ifdef CONFIG_SCSI_MULTIPATH
+static LIST_HEAD(sd_mpath_disks_list);
+static DEFINE_MUTEX(sd_mpath_disks_lock);
+
struct sd_mpath_disk {
+ struct device dev;
+ int disk_index;
+ int disk_count;
+ struct list_head entry;
struct scsi_mpath_head *scsi_mpath_head;
};
static void sd_mpath_disk_release(struct device *dev)
{
+ struct sd_mpath_disk *sd_mpath_disk =
+ container_of(dev, struct sd_mpath_disk, dev);
+ struct scsi_mpath_head *scsi_mpath_head =
+ sd_mpath_disk->scsi_mpath_head;
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+
+ mpath_put_disk(mpath_head);
+ ida_free(&sd_index_ida, sd_mpath_disk->disk_index);
+ scsi_mpath_put_head(scsi_mpath_head);
+
+ kfree(sd_mpath_disk);
}
static const struct class sd_mpath_disk_class = {
@@ -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);
@@ -4006,6 +4025,321 @@ static int sd_format_disk_name(char *prefix, int index, char *buf, int buflen)
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 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));
+
+ blk_mq_unfreeze_queue(disk->queue, memflags);
+
+ return ret;
+}
+static int sd_mpath_get_disk(struct sd_mpath_disk *sd_mpath_disk)
+{
+ if (!get_device(&sd_mpath_disk->dev))
+ return -ENXIO;
+ return 0;
+}
+
+static void sd_mpath_put_disk(struct sd_mpath_disk *sd_mpath_disk)
+{
+ put_device(&sd_mpath_disk->dev);
+}
+
+static struct sd_mpath_disk *sd_mpath_find_disk(
+ struct scsi_mpath_head *scsi_mpath_head)
+{
+ struct sd_mpath_disk *sd_mpath_disk;
+ int ret;
+
+ list_for_each_entry(sd_mpath_disk, &sd_mpath_disks_list, entry) {
+ ret = sd_mpath_get_disk(sd_mpath_disk);
+ if (ret)
+ continue;
+
+ if (sd_mpath_disk->scsi_mpath_head == scsi_mpath_head)
+ return sd_mpath_disk;
+
+ sd_mpath_put_disk(sd_mpath_disk);
+ }
+
+ return NULL;
+}
+
+static void sd_mpath_add_disk(struct scsi_disk *sdkp)
+{
+ 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 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;
+
+ mpath_add_device(mpath_device, mpath_head, sdkp->disk,
+ dev_to_node(sdp->host->dma_dev), &sdp->host->mpath_nr_active);
+ mpath_device_set_live(mpath_device);
+}
+
+static int sd_mpath_probe(struct scsi_disk *sdkp)
+{
+ struct scsi_device *sdp = sdkp->device;
+ struct scsi_mpath_device *scsi_mpath_dev = sdp->scsi_mpath_dev;
+ struct device *dma_dev = sdp->host->dma_dev;
+ struct scsi_mpath_head *scsi_mpath_head =
+ scsi_mpath_dev->scsi_mpath_head;
+ struct sd_mpath_disk *sd_mpath_disk;
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+ char disk_name[DISK_NAME_LEN - 2];
+ struct queue_limits lim;
+ struct gendisk *disk;
+ int error;
+
+ /*
+ * sd_mpath_disks_list is kept locked if no disk found.
+ * Otherwise an extra reference is taken.
+ */
+ mutex_lock(&sd_mpath_disks_lock);
+ sd_mpath_disk = sd_mpath_find_disk(scsi_mpath_head);
+ if (sd_mpath_disk) {
+ error = sized_strscpy(disk_name, mpath_head->disk->disk_name,
+ sizeof(disk_name));
+ if (error < 0) {
+ /*
+ * Should not happen as would fail for the same when
+ * allocating the sd_mpath_disk
+ */
+ sd_mpath_put_disk(sd_mpath_disk);
+ mutex_unlock(&sd_mpath_disks_lock);
+ return error;
+ }
+ sd_mpath_disk->disk_count++;
+ mutex_unlock(&sd_mpath_disks_lock);
+
+ goto found;
+ }
+
+ sd_mpath_disk = kzalloc(sizeof(*sd_mpath_disk), GFP_KERNEL);
+ if (!sd_mpath_disk) {
+ error = -ENOMEM;
+ goto out_unlock;
+ }
+
+ sd_mpath_disk->scsi_mpath_head = scsi_mpath_head;
+ device_initialize(&sd_mpath_disk->dev);
+ sd_mpath_disk->dev.class = &sd_mpath_disk_class;
+
+ blk_set_stacking_limits(&lim);
+ lim.dma_alignment = 3;
+ lim.features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT |
+ BLK_FEAT_POLL | BLK_FEAT_ATOMIC_WRITES;
+
+ mpath_head->parent = &sd_mpath_disk->dev;
+ mpath_head->drv_module = THIS_MODULE;
+ error = mpath_alloc_head_disk(mpath_head, &lim,
+ dev_to_node(dma_dev));
+ if (error)
+ goto out_free_disk;
+ disk = mpath_head->disk;
+
+ error = ida_alloc(&sd_index_ida, GFP_KERNEL);
+ if (error < 0) {
+ sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
+ goto out_put_disk;
+ }
+ sd_mpath_disk->disk_index = error;
+ error = sd_format_disk_name("sd", sd_mpath_disk->disk_index,
+ disk->disk_name, DISK_NAME_LEN);
+ if (error)
+ goto out_free_index;
+
+ error = sized_strscpy(disk_name, mpath_head->disk->disk_name,
+ sizeof(disk_name));
+ if (error < 0)
+ goto out_free_index;
+
+ error = dev_set_name(&sd_mpath_disk->dev, "scsi_mpath_disk%d",
+ scsi_mpath_head->index);
+ if (error)
+ goto out_free_index;
+
+ /* undone in sd_mpath_disk_release() */
+ scsi_mpath_get_head(scsi_mpath_head);
+
+ error = device_add(&sd_mpath_disk->dev);
+ if (error) {
+ put_device(&sd_mpath_disk->dev);
+ goto out_unlock;
+ }
+
+ list_add_tail(&sd_mpath_disk->entry, &sd_mpath_disks_list);
+ disk->major = sd_major((sd_mpath_disk->disk_index & 0xf0) >> 4);
+ disk->first_minor = ((sd_mpath_disk->disk_index & 0xf) << 4) |
+ (sd_mpath_disk->disk_index & 0xfff00);
+ disk->minors = SD_MINORS;
+
+ sd_mpath_disk->disk_count = 1;
+ mutex_unlock(&sd_mpath_disks_lock);
+found:
+ sdkp->sd_mpath_disk = sd_mpath_disk;
+ sdkp->disk->flags |= GENHD_FL_HIDDEN;
+ snprintf(sdkp->disk->disk_name, DISK_NAME_LEN, "%s:%d",
+ disk_name, scsi_mpath_dev->index);
+
+ sdkp->index = -1;
+ return 0;
+
+out_free_index:
+ ida_free(&sd_index_ida, sd_mpath_disk->disk_index);
+out_put_disk:
+ mpath_put_disk(mpath_head);
+out_free_disk:
+ kfree(sd_mpath_disk);
+out_unlock:
+ mutex_unlock(&sd_mpath_disks_lock);
+ return error;
+}
+
+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);
+
+ 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);
+ 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);
+}
+
+static void sd_mpath_remove_head(struct scsi_mpath_head *scsi_mpath_head)
+{
+ struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
+ struct sd_mpath_disk *sd_mpath_disk;
+ struct device *dev = &scsi_mpath_head->dev;
+
+ mutex_lock(&sd_mpath_disks_lock);
+ sd_mpath_disk = sd_mpath_find_disk(scsi_mpath_head);
+ if (!sd_mpath_disk) {
+ dev_warn(dev, "could not find mpath disk\n");
+ mutex_unlock(&sd_mpath_disks_lock);
+ return;
+ }
+
+ list_del_init(&sd_mpath_disk->entry);
+ mutex_unlock(&sd_mpath_disks_lock);
+
+ device_del(&sd_mpath_disk->dev);
+ mpath_remove_disk(mpath_head);
+ sd_mpath_put_disk(sd_mpath_disk);
+}
+
+/*
+ * Always calls for a failed probe, so we need to handle that some structures
+ * have not been setup.
+ */
+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);
+}
+
+#else /* CONFIG_SCSI_MULTIPATH */
+static int sd_mpath_probe(struct scsi_disk *sdkp)
+{
+ return 0;
+}
+static void sd_mpath_remove(struct scsi_disk *sdkp)
+{
+ return;
+}
+static void sd_mpath_fail_probe(struct scsi_disk *sdkp)
+{
+
+}
+static int sd_mpath_revalidate_head(struct scsi_disk *sdkp)
+{
+ return 0;
+}
+static void sd_mpath_add_disk(struct scsi_disk *sdkp)
+{
+}
+#endif
/**
* sd_probe - called during driver initialization and whenever a
* new scsi device is attached to the system. It is called once
@@ -4058,22 +4392,33 @@ static int sd_probe(struct scsi_device *sdp)
&sd_bio_compl_lkclass);
if (!gd)
goto out_free;
+ sdkp->disk = gd;
+ sdkp->device = sdp;
- index = ida_alloc(&sd_index_ida, GFP_KERNEL);
- if (index < 0) {
- sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
- goto out_put;
- }
+ if (sdp->scsi_mpath_dev) {
+ error = sd_mpath_probe(sdkp);
+ if (error)
+ goto out_put;
+ } else {
+ index = ida_alloc(&sd_index_ida, GFP_KERNEL);
+ if (index < 0) {
+ sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
+ goto out_put;
+ }
- error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
- if (error) {
- sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name length exceeded.\n");
- goto out_free_index;
+ error = sd_format_disk_name("sd", index, gd->disk_name,
+ DISK_NAME_LEN);
+ if (error) {
+ sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name length exceeded.\n");
+ goto out_free_index;
+ }
+ sdkp->index = index;
+
+ gd->major = sd_major((index & 0xf0) >> 4);
+ gd->first_minor = ((index & 0xf) << 4) | (index & 0xfff00);
+ gd->minors = SD_MINORS;
}
- sdkp->device = sdp;
- sdkp->disk = gd;
- sdkp->index = index;
sdkp->max_retries = SD_MAX_RETRIES;
atomic_set(&sdkp->openers, 0);
atomic_set(&sdkp->device->ioerr_cnt, 0);
@@ -4093,6 +4438,7 @@ static int sd_probe(struct scsi_device *sdp)
error = device_add(&sdkp->disk_dev);
if (error) {
+ sd_mpath_fail_probe(sdkp);
put_device(&sdkp->disk_dev);
put_disk(gd);
goto out;
@@ -4100,10 +4446,6 @@ static int sd_probe(struct scsi_device *sdp)
dev_set_drvdata(dev, sdkp);
- gd->major = sd_major((index & 0xf0) >> 4);
- gd->first_minor = ((index & 0xf) << 4) | (index & 0xfff00);
- gd->minors = SD_MINORS;
-
gd->fops = &sd_fops;
gd->private_data = sdkp;
@@ -4127,6 +4469,12 @@ static int sd_probe(struct scsi_device *sdp)
}
}
+ if (sdp->scsi_mpath_dev) {
+ error = sd_mpath_revalidate_head(sdkp);
+ if (error)
+ sdev_printk(KERN_WARNING, sdp, "could not revalidate multipath limits\n");
+ }
+
if (sdp->removable) {
gd->flags |= GENHD_FL_REMOVABLE;
gd->events |= DISK_EVENT_MEDIA_CHANGE;
@@ -4141,6 +4489,7 @@ static int sd_probe(struct scsi_device *sdp)
error = device_add_disk(dev, gd, NULL);
if (error) {
+ sd_mpath_fail_probe(sdkp);
device_unregister(&sdkp->disk_dev);
put_disk(gd);
if (sdp->sector_size > PAGE_SIZE)
@@ -4148,6 +4497,9 @@ static int sd_probe(struct scsi_device *sdp)
goto out;
}
+ if (sdp->scsi_mpath_dev)
+ sd_mpath_add_disk(sdkp);
+
if (sdkp->security) {
sdkp->opal_dev = init_opal_dev(sdkp, &sd_sec_submit);
if (sdkp->opal_dev)
@@ -4161,7 +4513,8 @@ static int sd_probe(struct scsi_device *sdp)
return 0;
out_free_index:
- ida_free(&sd_index_ida, index);
+ if (index >= 0)
+ ida_free(&sd_index_ida, index);
out_put:
put_disk(gd);
out_free:
@@ -4289,6 +4642,9 @@ static void sd_remove(struct scsi_device *sdp)
struct device *dev = &sdp->sdev_gendev;
struct scsi_disk *sdkp = dev_get_drvdata(dev);
+ if (sdp->scsi_mpath_dev)
+ sd_mpath_remove(sdkp);
+
scsi_autopm_get_device(sdkp->device);
device_del(&sdkp->disk_dev);
@@ -4457,6 +4813,9 @@ static struct scsi_driver sd_template = {
.resume = sd_resume,
.init_command = sd_init_command,
.uninit_command = sd_uninit_command,
+ #ifdef CONFIG_SCSI_MULTIPATH
+ .mpath_remove_head = sd_mpath_remove_head,
+ #endif
.done = sd_done,
.eh_action = sd_eh_action,
.eh_reset = sd_eh_reset,
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 13/17] scsi: sd: support multipath disk
2026-07-03 10:33 ` [PATCH v3 13/17] scsi: sd: support multipath disk John Garry
@ 2026-07-03 12:24 ` sashiko-bot
0 siblings, 0 replies; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 12:24 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] The local variable `index` is left uninitialized when probing a multipath disk in `sd_probe()`, which can lead to IDA corruption if an error occurs.
- [High] Error paths in `sd_mpath_probe()` call `kfree()` on a device object after `device_initialize()` has been called, leaking kobject resources and violating the driver core API.
- [High] Concurrent execution of `sd_mpath_remove_head()` and path removal (`sd_mpath_remove()` or `sd_mpath_fail_probe()`) causes `device_del()` and `mpath_remove_disk()` to be called twice on the same device.
- [High] A time-of-check to time-of-use (TOCTOU) race condition exists between `sd_probe()` and `sd_mpath_remove_head()`, leading to a Use-After-Free on the multipath gendisk.
- [Medium] The `sd_mpath_disk` reference is permanently leaked when `sd_large_pool_create()` fails for a multipath disk.
Pre-existing issues:
- [High] In `sd_probe()`, failing `sd_large_pool_create()` jumps to `out_free_index` which frees the `scsi_disk` object without unregistering its device from sysfs.
--
commit 1b5b9df2120152290853f43f41a0b4fe936be00b
Author: John Garry <john.g.garry@oracle.com>
scsi: sd: support multipath disk
Add support to attach a multipath disk. A global list of sd_mpath_disks
is kept for matching scsi_device's, and gendisks are allocated per-path
and marked as hidden.
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -4006,6 +4025,321 @@ static int sd_format_disk_name(char *prefix, int index, char *buf, int buflen)
[ ... ]
> +static int sd_mpath_probe(struct scsi_disk *sdkp)
> +{
[ ... ]
> + sd_mpath_disk->scsi_mpath_head = scsi_mpath_head;
> + device_initialize(&sd_mpath_disk->dev);
> + sd_mpath_disk->dev.class = &sd_mpath_disk_class;
> +
> + blk_set_stacking_limits(&lim);
> + lim.dma_alignment = 3;
> + lim.features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT |
> + BLK_FEAT_POLL | BLK_FEAT_ATOMIC_WRITES;
> +
> + mpath_head->parent = &sd_mpath_disk->dev;
> + mpath_head->drv_module = THIS_MODULE;
> + error = mpath_alloc_head_disk(mpath_head, &lim,
> + dev_to_node(dma_dev));
> + if (error)
> + goto out_free_disk;
[ ... ]
> +out_free_disk:
> + kfree(sd_mpath_disk);
[Severity: High]
Does this error path leak kobject resources?
Since device_initialize() has already been called on sd_mpath_disk->dev in
sd_mpath_probe(), shouldn't we use put_device() here instead of directly
calling kfree()?
It looks like sd_mpath_disk_release() currently assumes full initialization
(like unconditionally freeing disk_index), so a simple swap to put_device()
might also require updating the release function to handle partial
initialization safely.
> +out_unlock:
> + mutex_unlock(&sd_mpath_disks_lock);
> + return error;
> +}
> +
> +static void sd_mpath_remove(struct scsi_disk *sdkp)
> +{
[ ... ]
> + 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);
> + 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);
> +}
> +
> +static void sd_mpath_remove_head(struct scsi_mpath_head *scsi_mpath_head)
> +{
> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> + struct sd_mpath_disk *sd_mpath_disk;
> + struct device *dev = &scsi_mpath_head->dev;
> +
> + mutex_lock(&sd_mpath_disks_lock);
> + sd_mpath_disk = sd_mpath_find_disk(scsi_mpath_head);
> + if (!sd_mpath_disk) {
> + dev_warn(dev, "could not find mpath disk\n");
> + mutex_unlock(&sd_mpath_disks_lock);
> + return;
> + }
> +
> + list_del_init(&sd_mpath_disk->entry);
> + mutex_unlock(&sd_mpath_disks_lock);
> +
> + device_del(&sd_mpath_disk->dev);
> + mpath_remove_disk(mpath_head);
[Severity: High]
Can this lead to a double-free or sysfs splat if sd_mpath_remove() and
sd_mpath_remove_head() run concurrently?
If disk_count drops to 0 inside sd_mpath_remove(), it sets remove = true,
drops the lock, and unconditionally calls device_del() and
mpath_remove_disk().
Concurrently, sd_mpath_remove_head() could acquire the lock, remove the entry,
drop the lock, and also unconditionally call device_del() and
mpath_remove_disk() on the same device without mutual exclusion.
[ ... ]
> @@ -4058,22 +4392,33 @@ static int sd_probe(struct scsi_device *sdp)
> &sd_bio_compl_lkclass);
> if (!gd)
> goto out_free;
> + sdkp->disk = gd;
> + sdkp->device = sdp;
>
> - index = ida_alloc(&sd_index_ida, GFP_KERNEL);
> - if (index < 0) {
> - sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
> - goto out_put;
> - }
> + if (sdp->scsi_mpath_dev) {
> + error = sd_mpath_probe(sdkp);
> + if (error)
> + goto out_put;
> + } else {
> + index = ida_alloc(&sd_index_ida, GFP_KERNEL);
[Severity: High]
Does this leave the local variable index uninitialized when probing a
multipath disk?
If sdp->scsi_mpath_dev is true, index is bypassed and not initialized. If a
subsequent initialization step fails (like sd_large_pool_create()), the error
path jumps to out_free_index:
out_free_index:
if (index >= 0)
ida_free(&sd_index_ida, index);
Could this evaluate uninitialized stack garbage and corrupt the global
sd_index_ida allocator?
[Severity: Medium]
In this same scenario, if sd_large_pool_create() fails, we jump straight to
out_free_index. Doesn't this skip calling sd_mpath_fail_probe(sdkp) to clean
up the successfully allocated sd_mpath_disk?
This appears to permanently leak the sd_mpath_disk reference while freeing
the underlying scsi_disk object.
[Severity: High]
This is a pre-existing issue, but if sd_large_pool_create() fails and jumps
to out_free_index, wouldn't we free the scsi_disk (via out_free: kfree(sdkp))
while its embedded disk_dev is still actively linked into sysfs?
Earlier in the function, device_add(&sdkp->disk_dev) successfully registers
it, but this error path never calls device_del() or device_unregister(),
which leaves dangling pointers in driver core lists.
[ ... ]
> @@ -4127,6 +4469,12 @@ static int sd_probe(struct scsi_device *sdp)
> }
> }
>
> + if (sdp->scsi_mpath_dev) {
> + error = sd_mpath_revalidate_head(sdkp);
> + if (error)
> + sdev_printk(KERN_WARNING, sdp, "could not revalidate multipath limits\n");
> + }
> +
[Severity: High]
Is there a race condition between sd_probe() and a concurrent head removal?
Between the successful setup in sd_mpath_probe() and this call to
sd_mpath_revalidate_head(), an external event could trigger
sd_mpath_remove_head(). This would invoke mpath_remove_disk(mpath_head)
to delete and free the multipath gendisk.
If sd_probe() continues without locks and calls sd_mpath_revalidate_head(),
it dereferences mpath_head->disk and blindly calls
blk_mq_freeze_queue(disk->queue) inside sd_mpath_revalidate_head(), which
appears to be a use-after-free on the disk and its request queue.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=13
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 14/17] scsi: sd: add multipath disk attr groups
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (12 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 13/17] scsi: sd: support multipath disk John Garry
@ 2026-07-03 10:33 ` John Garry
2026-07-03 12:25 ` sashiko-bot
2026-07-03 10:34 ` [PATCH v3 15/17] scsi: sd: add mpath_dev file John Garry
` (2 subsequent siblings)
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:33 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Set multipath disk attr groups, which includes delayed disk removal and
everything from mpath_attr_group.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/sd.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3df70b24b688e..a9a29e50f5eec 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -4106,6 +4106,41 @@ static void sd_mpath_add_disk(struct scsi_disk *sdkp)
mpath_device_set_live(mpath_device);
}
+static ssize_t sd_mpath_device_delayed_removal_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct mpath_head *mpath_head = mpath_bd_device_to_head(dev);
+
+ return mpath_delayed_removal_secs_store(mpath_head, buf, count);
+}
+
+static ssize_t sd_mpath_device_delayed_removal_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct mpath_head *mpath_head = mpath_bd_device_to_head(dev);
+
+ return mpath_delayed_removal_secs_show(mpath_head, buf);
+}
+
+static DEVICE_ATTR(delayed_removal_secs, S_IRUGO | S_IWUSR,
+ sd_mpath_device_delayed_removal_show,
+ sd_mpath_device_delayed_removal_store);
+
+static struct attribute *sd_mpath_disk_attrs[] = {
+ &dev_attr_delayed_removal_secs.attr,
+ NULL
+};
+
+static const struct attribute_group sd_mpath_disk_attr_group = {
+ .attrs = sd_mpath_disk_attrs,
+};
+
+static const struct attribute_group *sd_mpath_disk_attr_groups[] = {
+ &sd_mpath_disk_attr_group,
+ &mpath_attr_group,
+ NULL
+};
+
static int sd_mpath_probe(struct scsi_disk *sdkp)
{
struct scsi_device *sdp = sdkp->device;
@@ -4161,6 +4196,7 @@ static int sd_mpath_probe(struct scsi_disk *sdkp)
mpath_head->parent = &sd_mpath_disk->dev;
mpath_head->drv_module = THIS_MODULE;
+ mpath_head->disk_groups = sd_mpath_disk_attr_groups;
error = mpath_alloc_head_disk(mpath_head, &lim,
dev_to_node(dma_dev));
if (error)
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 14/17] scsi: sd: add multipath disk attr groups
2026-07-03 10:33 ` [PATCH v3 14/17] scsi: sd: add multipath disk attr groups John Garry
@ 2026-07-03 12:25 ` sashiko-bot
0 siblings, 0 replies; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 12:25 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The patch references several undeclared identifiers and undefined functions, which will cause fatal compilation and linker errors.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but error paths in `sd_mpath_probe()` incorrectly use `kfree()` instead of `put_device()` after `device_initialize()`, leaking kobject resources. Furthermore, it cannot trivially use `put_device()` without causing a use-after-free and IDA corruption.
--
commit c032527f31f85b7a1e95cfcc2527fe7e368f3474
Author: John Garry <john.g.garry@oracle.com>
scsi: sd: add multipath disk attr groups
Set multipath disk attr groups, which includes delayed disk removal and
everything from mpath_attr_group.
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 3df70b24b688e..a9a29e50f5eec 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
[ ... ]
> @@ -4106,6 +4106,41 @@ static void sd_mpath_add_disk(struct scsi_disk *sdkp)
> mpath_device_set_live(mpath_device);
> }
>
> +static ssize_t sd_mpath_device_delayed_removal_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct mpath_head *mpath_head = mpath_bd_device_to_head(dev);
> +
> + return mpath_delayed_removal_secs_store(mpath_head, buf, count);
> +}
> +
> +static ssize_t sd_mpath_device_delayed_removal_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct mpath_head *mpath_head = mpath_bd_device_to_head(dev);
> +
> + return mpath_delayed_removal_secs_show(mpath_head, buf);
> +}
[Severity: Low]
Are the functions mpath_bd_device_to_head(), mpath_delayed_removal_secs_store(),
and mpath_delayed_removal_secs_show() declared anywhere? It looks like they
might be missing, which would cause a build error.
> +
> +static DEVICE_ATTR(delayed_removal_secs, S_IRUGO | S_IWUSR,
> + sd_mpath_device_delayed_removal_show,
> + sd_mpath_device_delayed_removal_store);
> +
> +static struct attribute *sd_mpath_disk_attrs[] = {
> + &dev_attr_delayed_removal_secs.attr,
> + NULL
> +};
> +
> +static const struct attribute_group sd_mpath_disk_attr_group = {
> + .attrs = sd_mpath_disk_attrs,
> +};
> +
> +static const struct attribute_group *sd_mpath_disk_attr_groups[] = {
> + &sd_mpath_disk_attr_group,
> + &mpath_attr_group,
[Severity: Low]
Is the external variable mpath_attr_group declared and defined? Building with
this patch applied results in an undeclared identifier error.
> + NULL
> +};
> +
> static int sd_mpath_probe(struct scsi_disk *sdkp)
> {
> struct scsi_device *sdp = sdkp->device;
> @@ -4161,6 +4196,7 @@ static int sd_mpath_probe(struct scsi_disk *sdkp)
>
> mpath_head->parent = &sd_mpath_disk->dev;
> mpath_head->drv_module = THIS_MODULE;
> + mpath_head->disk_groups = sd_mpath_disk_attr_groups;
> error = mpath_alloc_head_disk(mpath_head, &lim,
> dev_to_node(dma_dev));
> if (error)
[Severity: High]
This isn't a bug introduced by this patch, but since we are modifying
sd_mpath_probe(), is it safe to use kfree() on error paths after
device_initialize() has been called on sd_mpath_disk->dev?
If mpath_alloc_head_disk() fails, the code jumps to out_free_disk:
drivers/scsi/sd.c:sd_mpath_probe() {
...
device_initialize(&sd_mpath_disk->dev);
...
error = mpath_alloc_head_disk(mpath_head, &lim, dev_to_node(dma_dev));
if (error)
goto out_free_disk;
...
out_free_disk:
kfree(sd_mpath_disk);
...
}
Should this use put_device() instead to ensure the kobject internals are
properly cleaned up?
However, calling put_device() would invoke sd_mpath_disk_release(), which
unconditionally calls ida_free() and scsi_mpath_put_head(). Would
sd_mpath_disk_release() need to be updated to handle partial initialization if
put_device() were used here?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=14
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 15/17] scsi: sd: add mpath_dev file
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (13 preceding siblings ...)
2026-07-03 10:33 ` [PATCH v3 14/17] scsi: sd: add multipath disk attr groups John Garry
@ 2026-07-03 10:34 ` John Garry
2026-07-03 12:43 ` sashiko-bot
2026-07-03 10:34 ` [PATCH v3 16/17] scsi: sd: add mpath_numa_nodes dev attribute John Garry
2026-07-03 10:34 ` [PATCH v3 17/17] scsi: sd: add mpath_queue_depth " John Garry
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:34 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Since per-path gendisk is hidden, we have no
Add a mpath_dev file so that the multipath disk can be looked up from
per-path gendisk directory.
The following is an example of this usage:
$ ls -l /dev/sdc
brw-rw---- 1 root disk 8, 32 Feb 24 16:08 /dev/sdc
$ cat /sys/class/scsi_mpath_disk/scsi_mpath_disk0/sdc/multipath/sdc:0/mpath_dev
8:32
This can be used by a util like lsscsi, which would find that the gendisk
for the per-path scsi_device is missing.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/sd.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a9a29e50f5eec..0c77466f8291a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -4059,6 +4059,52 @@ static int sd_mpath_revalidate_head(struct scsi_disk *sdkp)
return ret;
}
+
+static ssize_t sd_mpath_dev_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ struct gendisk *gd = dev_to_disk(dev);
+ struct scsi_disk *sdkp = gd->private_data;
+ 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 device *disk_dev = disk_to_dev(disk);
+
+ return print_dev_t(page, disk_dev->devt);
+}
+static DEVICE_ATTR(mpath_dev, 0444, sd_mpath_dev_show, NULL);
+
+static struct attribute *sd_mpath_dev_attrs[] = {
+ &dev_attr_mpath_dev.attr,
+ NULL
+};
+
+static umode_t sd_mpath_dev_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int i)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct gendisk *gd = dev_to_disk(dev);
+ struct scsi_disk *sdkp = gd->private_data;
+ struct scsi_device *sdev = sdkp->device;
+ struct scsi_mpath_device *scsi_mpath_device = sdev->scsi_mpath_dev;
+
+ if (!scsi_mpath_device)
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group sd_mpath_dev_attr_group = {
+ .is_visible = sd_mpath_dev_attr_is_visible,
+ .attrs = sd_mpath_dev_attrs,
+};
+
+static const struct attribute_group *sd_mpath_dev_groups[] = {
+ &sd_mpath_dev_attr_group,
+ NULL
+};
+
static int sd_mpath_get_disk(struct sd_mpath_disk *sd_mpath_disk)
{
if (!get_device(&sd_mpath_disk->dev))
@@ -4375,6 +4421,8 @@ static int sd_mpath_revalidate_head(struct scsi_disk *sdkp)
static void sd_mpath_add_disk(struct scsi_disk *sdkp)
{
}
+
+#define sd_mpath_dev_groups NULL
#endif
/**
* sd_probe - called during driver initialization and whenever a
@@ -4523,7 +4571,7 @@ static int sd_probe(struct scsi_device *sdp)
sdp->host->rpm_autosuspend_delay);
}
- error = device_add_disk(dev, gd, NULL);
+ error = device_add_disk(dev, gd, sd_mpath_dev_groups);
if (error) {
sd_mpath_fail_probe(sdkp);
device_unregister(&sdkp->disk_dev);
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 15/17] scsi: sd: add mpath_dev file
2026-07-03 10:34 ` [PATCH v3 15/17] scsi: sd: add mpath_dev file John Garry
@ 2026-07-03 12:43 ` sashiko-bot
2026-07-06 15:56 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 12:43 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent sysfs read of `mpath_dev` can trigger a Use-After-Free of `sd_mpath_disk` during device removal.
--
commit 96501c42f9a149bd257256a80ef8d931781b2efe
Author: John Garry <john.g.garry@oracle.com>
scsi: sd: add mpath_dev file
This commit adds a mpath_dev sysfs file to allow looking up the multipath
disk from the per-path gendisk directory.
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index a9a29e50f5eec..0c77466f8291a 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
[ ... ]
> @@ -4059,6 +4059,52 @@ static int sd_mpath_revalidate_head(struct scsi_disk *sdkp)
>
> return ret;
> }
> +
> +static ssize_t sd_mpath_dev_show(struct device *dev,
> + struct device_attribute *attr, char *page)
> +{
> + struct gendisk *gd = dev_to_disk(dev);
> + struct scsi_disk *sdkp = gd->private_data;
> + struct sd_mpath_disk *sd_mpath_disk = sdkp->sd_mpath_disk;
> + struct scsi_mpath_head *scsi_mpath_head = sd_mpath_disk->scsi_mpath_head;
[Severity: High]
Can reading this sysfs attribute concurrently with device removal trigger a
use-after-free of sdkp->sd_mpath_disk?
During device teardown, sd_remove() calls sd_mpath_remove() before calling
del_gendisk(). This frees the multipath structures, but the mpath_dev sysfs
attribute (which is tied to the gendisk) remains active and accessible until
del_gendisk() finally tears it down.
If a user continuously reads this attribute during hot-unplug or module
removal, it appears we could blindly dereference the freed sd_mpath_disk
pointer here.
> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> + struct gendisk *disk = mpath_head->disk;
> + struct device *disk_dev = disk_to_dev(disk);
> +
> + return print_dev_t(page, disk_dev->devt);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=15
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 15/17] scsi: sd: add mpath_dev file
2026-07-03 12:43 ` sashiko-bot
@ 2026-07-06 15:56 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:56 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 13:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Concurrent sysfs read of `mpath_dev` can trigger a Use-After-Free of `sd_mpath_disk` during device removal.
> --
>
> commit 96501c42f9a149bd257256a80ef8d931781b2efe
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi: sd: add mpath_dev file
>
> This commit adds a mpath_dev sysfs file to allow looking up the multipath
> disk from the per-path gendisk directory.
>
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index a9a29e50f5eec..0c77466f8291a 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
> [ ... ]
>> @@ -4059,6 +4059,52 @@ static int sd_mpath_revalidate_head(struct scsi_disk *sdkp)
>>
>> return ret;
>> }
>> +
>> +static ssize_t sd_mpath_dev_show(struct device *dev,
>> + struct device_attribute *attr, char *page)
>> +{
>> + struct gendisk *gd = dev_to_disk(dev);
>> + struct scsi_disk *sdkp = gd->private_data;
>> + struct sd_mpath_disk *sd_mpath_disk = sdkp->sd_mpath_disk;
>> + struct scsi_mpath_head *scsi_mpath_head = sd_mpath_disk->scsi_mpath_head;
>
> [Severity: High]
> Can reading this sysfs attribute concurrently with device removal trigger a
> use-after-free of sdkp->sd_mpath_disk?
I think that we can avoid referencing sd_mpath_disk
>
> During device teardown, sd_remove() calls sd_mpath_remove() before calling
> del_gendisk(). This frees the multipath structures, but the mpath_dev sysfs
> attribute (which is tied to the gendisk) remains active and accessible until
> del_gendisk() finally tears it down.
>
> If a user continuously reads this attribute during hot-unplug or module
> removal, it appears we could blindly dereference the freed sd_mpath_disk
> pointer here.
>
>> + struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
>> + struct gendisk *disk = mpath_head->disk;
>> + struct device *disk_dev = disk_to_dev(disk);
>> +
>> + return print_dev_t(page, disk_dev->devt);
>> +}
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 16/17] scsi: sd: add mpath_numa_nodes dev attribute
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (14 preceding siblings ...)
2026-07-03 10:34 ` [PATCH v3 15/17] scsi: sd: add mpath_dev file John Garry
@ 2026-07-03 10:34 ` John Garry
2026-07-03 12:44 ` sashiko-bot
2026-07-03 10:34 ` [PATCH v3 17/17] scsi: sd: add mpath_queue_depth " John Garry
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:34 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add an attribute to show multipath NUMA node per-path (scsi_disk).
The following is an example of reading the file:
$ cat /sys/devices/platform/host8/session1/target8:0:0/8:0:0:0/block/sdc:0/numa_
mpath_numa_nodes
0-3
$ cat /sys/devices/platform/host9/session2/target9:0:0/9:0:0:0/block/sdc:1/numa_
mpath_numa_nodes
$
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/sd.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0c77466f8291a..5e0514304d81f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -4075,8 +4075,22 @@ static ssize_t sd_mpath_dev_show(struct device *dev,
}
static DEVICE_ATTR(mpath_dev, 0444, sd_mpath_dev_show, NULL);
+static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct gendisk *gd = dev_to_disk(dev);
+ struct scsi_disk *sdkp = gd->private_data;
+ struct scsi_device *sdev = sdkp->device;
+ struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
+ struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
+
+ return mpath_numa_nodes_show(mpath_device, buf);
+}
+static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
+
static struct attribute *sd_mpath_dev_attrs[] = {
&dev_attr_mpath_dev.attr,
+ &dev_attr_mpath_numa_nodes.attr,
NULL
};
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 16/17] scsi: sd: add mpath_numa_nodes dev attribute
2026-07-03 10:34 ` [PATCH v3 16/17] scsi: sd: add mpath_numa_nodes dev attribute John Garry
@ 2026-07-03 12:44 ` sashiko-bot
2026-07-06 15:57 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 12:44 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] Build failure due to undefined function `mpath_numa_nodes_show`.
Pre-existing issues:
- [Critical] Race condition during device removal leads to a Use-After-Free of `sd_mpath_disk`.
--
commit 90ab5069ccf099a5f4cd42027fac637f7869d718
Author: John Garry <john.g.garry@oracle.com>
scsi: sd: add mpath_numa_nodes dev attribute
Add an attribute to show multipath NUMA node per-path (scsi_disk).
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 0c77466f8291a..5e0514304d81f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -4075,8 +4075,22 @@ static ssize_t sd_mpath_dev_show(struct device *dev,
> }
> static DEVICE_ATTR(mpath_dev, 0444, sd_mpath_dev_show, NULL);
[Severity: Critical]
This is a pre-existing issue, but does device removal race with sysfs reads
and cause a Use-After-Free?
Looking at sd_remove(), sd_mpath_remove(sdkp) is called before del_gendisk(),
which frees sd_mpath_disk but leaves the sysfs attributes active:
if (sdp->scsi_mpath_dev)
sd_mpath_remove(sdkp);
scsi_autopm_get_device(sdkp->device);
device_del(&sdkp->disk_dev);
del_gendisk(sdkp->disk);
If a local user concurrently reads the mpath_dev sysfs attribute, won't
sd_mpath_dev_show() blindly dereference the freed sd_mpath_disk? Will the
newly added attribute suffer from a similar lifecycle problem?
>
> +static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct gendisk *gd = dev_to_disk(dev);
> + struct scsi_disk *sdkp = gd->private_data;
> + struct scsi_device *sdev = sdkp->device;
> + struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
> + struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
> +
> + return mpath_numa_nodes_show(mpath_device, buf);
[Severity: Low]
Will this compile correctly? I couldn't find a declaration or definition for
mpath_numa_nodes_show() anywhere. Could this lead to an implicit function
declaration and a subsequent linker error?
> +}
> +static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
> +
> static struct attribute *sd_mpath_dev_attrs[] = {
> &dev_attr_mpath_dev.attr,
> + &dev_attr_mpath_numa_nodes.attr,
> NULL
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=16
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 16/17] scsi: sd: add mpath_numa_nodes dev attribute
2026-07-03 12:44 ` sashiko-bot
@ 2026-07-06 15:57 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 13:44, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Low] Build failure due to undefined function `mpath_numa_nodes_show`.
>
> Pre-existing issues:
> - [Critical] Race condition during device removal leads to a Use-After-Free of `sd_mpath_disk`.
> --
>
> commit 90ab5069ccf099a5f4cd42027fac637f7869d718
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi: sd: add mpath_numa_nodes dev attribute
>
> Add an attribute to show multipath NUMA node per-path (scsi_disk).
>
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index 0c77466f8291a..5e0514304d81f 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -4075,8 +4075,22 @@ static ssize_t sd_mpath_dev_show(struct device *dev,
>> }
>> static DEVICE_ATTR(mpath_dev, 0444, sd_mpath_dev_show, NULL);
>
> [Severity: Critical]
> This is a pre-existing issue, but does device removal race with sysfs reads
> and cause a Use-After-Free?
>
> Looking at sd_remove(), sd_mpath_remove(sdkp) is called before del_gendisk(),
> which frees sd_mpath_disk but leaves the sysfs attributes active:
>
> if (sdp->scsi_mpath_dev)
> sd_mpath_remove(sdkp);
>
> scsi_autopm_get_device(sdkp->device);
>
> device_del(&sdkp->disk_dev);
> del_gendisk(sdkp->disk);
>
> If a local user concurrently reads the mpath_dev sysfs attribute, won't
> sd_mpath_dev_show() blindly dereference the freed sd_mpath_disk? Will the
> newly added attribute suffer from a similar lifecycle problem?
same as 15/17
>
>>
>> +static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct gendisk *gd = dev_to_disk(dev);
>> + struct scsi_disk *sdkp = gd->private_data;
>> + struct scsi_device *sdev = sdkp->device;
>> + struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
>> + struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
>> +
>> + return mpath_numa_nodes_show(mpath_device, buf);
>
> [Severity: Low]
> Will this compile correctly? I couldn't find a declaration or definition for
> mpath_numa_nodes_show() anywhere. Could this lead to an implicit function
> declaration and a subsequent linker error?
>
>> +}
>> +static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
>> +
>> static struct attribute *sd_mpath_dev_attrs[] = {
>> &dev_attr_mpath_dev.attr,
>> + &dev_attr_mpath_numa_nodes.attr,
>> NULL
>> };
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 17/17] scsi: sd: add mpath_queue_depth dev attribute
2026-07-03 10:33 [PATCH v3 00/17] Native SCSI multipath support John Garry
` (15 preceding siblings ...)
2026-07-03 10:34 ` [PATCH v3 16/17] scsi: sd: add mpath_numa_nodes dev attribute John Garry
@ 2026-07-03 10:34 ` John Garry
2026-07-03 12:57 ` sashiko-bot
16 siblings, 1 reply; 46+ messages in thread
From: John Garry @ 2026-07-03 10:34 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare
Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
bmarzins, dm-devel, linux-kernel, nilay, john.garry, John Garry
Add a queue_depth file so that the multipath dynamic queue depth can be
looked up from per-path gendisk (scsi_disk) directory.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/sd.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5e0514304d81f..f6feccd557e76 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -4088,9 +4088,27 @@ static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
}
static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
+static ssize_t sd_mpath_queue_depth_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct gendisk *gd = dev_to_disk(dev);
+ struct scsi_disk *sdkp = gd->private_data;
+ struct scsi_device *sdev = sdkp->device;
+ 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 Scsi_Host *shost = sdev->host;
+
+ if (!mpath_qd_iopolicy(&scsi_mpath_head->iopolicy))
+ return 0;
+
+ return sysfs_emit(buf, "%d\n", atomic_read(&shost->mpath_nr_active));
+}
+static DEVICE_ATTR(mpath_queue_depth, 0444, sd_mpath_queue_depth_show, NULL);
+
static struct attribute *sd_mpath_dev_attrs[] = {
&dev_attr_mpath_dev.attr,
&dev_attr_mpath_numa_nodes.attr,
+ &dev_attr_mpath_queue_depth.attr,
NULL
};
--
2.43.7
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 17/17] scsi: sd: add mpath_queue_depth dev attribute
2026-07-03 10:34 ` [PATCH v3 17/17] scsi: sd: add mpath_queue_depth " John Garry
@ 2026-07-03 12:57 ` sashiko-bot
2026-07-06 15:59 ` John Garry
0 siblings, 1 reply; 46+ messages in thread
From: sashiko-bot @ 2026-07-03 12:57 UTC (permalink / raw)
To: John Garry; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Critical] The patch calls an undeclared function `mpath_qd_iopolicy()`, which will cause a build failure.
- [Low] Returning 0 from a sysfs `show` callback yields an empty read (EOF) instead of a proper error code.
Pre-existing issues:
- [High] Concurrent sysfs read of `mpath_queue_depth` during device teardown accesses a freed `sd_mpath_disk`.
--
commit d24252fbed25782c1f5fb88edab9e008070c4b9c
Author: John Garry <john.g.garry@oracle.com>
scsi: sd: add mpath_queue_depth dev attribute
Add a queue_depth file so that the multipath dynamic queue depth can be
looked up from per-path gendisk (scsi_disk) directory.
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 5e0514304d81f..f6feccd557e76 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
[ ... ]
> @@ -4088,9 +4088,27 @@ static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
> }
> static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
>
> +static ssize_t sd_mpath_queue_depth_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct gendisk *gd = dev_to_disk(dev);
> + struct scsi_disk *sdkp = gd->private_data;
> + struct scsi_device *sdev = sdkp->device;
> + struct sd_mpath_disk *sd_mpath_disk = sdkp->sd_mpath_disk;
> + struct scsi_mpath_head *scsi_mpath_head = sd_mpath_disk->scsi_mpath_head;
[Severity: High]
This is a pre-existing issue, but can this sysfs read race with device
removal and cause a use-after-free?
During device removal in sd_remove(), sd_mpath_remove() is called to drop
the reference to sd_mpath_disk (and free it when the refcount hits zero):
sd_remove() {
...
if (sdp->scsi_mpath_dev)
sd_mpath_remove(sdkp);
scsi_autopm_get_device(sdkp->device);
device_del(&sdkp->disk_dev);
del_gendisk(sdkp->disk);
}
Since device_del() and del_gendisk() happen later in the removal process,
local userspace can trigger a use-after-free by reading this attribute in a
loop while the multipath device is being hot-unplugged.
> + struct Scsi_Host *shost = sdev->host;
> +
> + if (!mpath_qd_iopolicy(&scsi_mpath_head->iopolicy))
[Severity: Critical]
Is mpath_qd_iopolicy() declared anywhere in the tree?
Compiling the kernel with CONFIG_SCSI_MULTIPATH enabled will attempt to
compile this code, but a search of the source tree indicates this function
is neither defined nor declared, which will cause the build to fail.
> + return 0;
[Severity: Low]
Does returning 0 here result in an unexpected End-Of-File?
Returning 0 from a sysfs show callback causes the VFS to report 0 bytes
read, which userspace interpreters like cat treat as an immediate EOF,
resulting in empty output.
Should this return a standard error code like -EOPNOTSUPP instead?
> +
> + return sysfs_emit(buf, "%d\n", atomic_read(&shost->mpath_nr_active));
> +}
> +static DEVICE_ATTR(mpath_queue_depth, 0444, sd_mpath_queue_depth_show, NULL);
> +
> static struct attribute *sd_mpath_dev_attrs[] = {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=17
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 17/17] scsi: sd: add mpath_queue_depth dev attribute
2026-07-03 12:57 ` sashiko-bot
@ 2026-07-06 15:59 ` John Garry
0 siblings, 0 replies; 46+ messages in thread
From: John Garry @ 2026-07-06 15:59 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 03/07/2026 13:57, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Critical] The patch calls an undeclared function `mpath_qd_iopolicy()`, which will cause a build failure.
> - [Low] Returning 0 from a sysfs `show` callback yields an empty read (EOF) instead of a proper error code.
>
> Pre-existing issues:
> - [High] Concurrent sysfs read of `mpath_queue_depth` during device teardown accesses a freed `sd_mpath_disk`.
> --
>
> commit d24252fbed25782c1f5fb88edab9e008070c4b9c
> Author: John Garry <john.g.garry@oracle.com>
>
> scsi: sd: add mpath_queue_depth dev attribute
>
> Add a queue_depth file so that the multipath dynamic queue depth can be
> looked up from per-path gendisk (scsi_disk) directory.
>
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index 5e0514304d81f..f6feccd557e76 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
> [ ... ]
>> @@ -4088,9 +4088,27 @@ static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
>> }
>> static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
>>
>> +static ssize_t sd_mpath_queue_depth_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct gendisk *gd = dev_to_disk(dev);
>> + struct scsi_disk *sdkp = gd->private_data;
>> + struct scsi_device *sdev = sdkp->device;
>> + struct sd_mpath_disk *sd_mpath_disk = sdkp->sd_mpath_disk;
>> + struct scsi_mpath_head *scsi_mpath_head = sd_mpath_disk->scsi_mpath_head;
>
> [Severity: High]
> This is a pre-existing issue, but can this sysfs read race with device
> removal and cause a use-after-free?
same as 15/17
>
> During device removal in sd_remove(), sd_mpath_remove() is called to drop
> the reference to sd_mpath_disk (and free it when the refcount hits zero):
>
> sd_remove() {
> ...
> if (sdp->scsi_mpath_dev)
> sd_mpath_remove(sdkp);
>
> scsi_autopm_get_device(sdkp->device);
>
> device_del(&sdkp->disk_dev);
> del_gendisk(sdkp->disk);
> }
>
> Since device_del() and del_gendisk() happen later in the removal process,
> local userspace can trigger a use-after-free by reading this attribute in a
> loop while the multipath device is being hot-unplugged.
>
>> + struct Scsi_Host *shost = sdev->host;
>> +
>> + if (!mpath_qd_iopolicy(&scsi_mpath_head->iopolicy))
>
> [Severity: Critical]
> Is mpath_qd_iopolicy() declared anywhere in the tree?
>
> Compiling the kernel with CONFIG_SCSI_MULTIPATH enabled will attempt to
> compile this code, but a search of the source tree indicates this function
> is neither defined nor declared, which will cause the build to fail.
>
>> + return 0;
yeah, I should sysfs_emit(0) here
>
> [Severity: Low]
> Does returning 0 here result in an unexpected End-Of-File?
>
> Returning 0 from a sysfs show callback causes the VFS to report 0 bytes
> read, which userspace interpreters like cat treat as an immediate EOF,
> resulting in empty output.
>
> Should this return a standard error code like -EOPNOTSUPP instead?
>
>> +
>> + return sysfs_emit(buf, "%d\n", atomic_read(&shost->mpath_nr_active));
>> +}
>> +static DEVICE_ATTR(mpath_queue_depth, 0444, sd_mpath_queue_depth_show, NULL);
>> +
>> static struct attribute *sd_mpath_dev_attrs[] = {
>
^ permalink raw reply [flat|nested] 46+ messages in thread