[PATCH v2] scsi: scsi_dh: Return error pointer in scsi_dh_attached_handler_name

[PATCH v2] scsi: scsi_dh: Return error pointer in scsi_dh_attached_handler_name

[Benjamin Marzinski]

If scsi_dh_attached_handler_name() fails to allocate the handler name,
dm-multipath (its only caller) assumes there is no attached device
handler, and sets the device up incorrectly. Return an error pointer
instead, so multipath can distinguish between failure, success where
there is no attached device handler, or when the path device is not
a scsi device at all.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---

Changes from v1:
If scsi_dh_attached_handler_name() returns that the path device is not
a scsi device (-ENODEV) but m->hw_hander_name is set, print an error
message and clear it, like the code does when a hardware handler is set
on a bio-based multipath device.

 drivers/md/dm-mpath.c  | 13 +++++++++++++
 drivers/scsi/scsi_dh.c |  8 +++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index c18358271618..65a3e93385c0 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -950,6 +950,19 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 
 	q = bdev_get_queue(p->path.dev->bdev);
 	attached_handler_name = scsi_dh_attached_handler_name(q, GFP_KERNEL);
+	if (IS_ERR(attached_handler_name)) {
+		if (PTR_ERR(attached_handler_name) == -ENODEV) {
+			if (m->hw_handler_name) {
+				DMERR("hardware handlers are only allowed for scsi devices");
+				kfree(m->hw_handler_name);
+				m->hw_handler_name = NULL;
+			}
+			attached_handler_name = NULL;
+		} else {
+			r = PTR_ERR(attached_handler_name);
+			goto bad;
+		}
+	}
 	if (attached_handler_name || m->hw_handler_name) {
 		INIT_DELAYED_WORK(&p->activate_path, activate_path_work);
 		r = setup_scsi_dh(p->path.dev->bdev, m, &attached_handler_name, &ti->error);
diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c
index 7b56e00c7df6..b9d805317814 100644
--- a/drivers/scsi/scsi_dh.c
+++ b/drivers/scsi/scsi_dh.c
@@ -353,7 +353,8 @@ EXPORT_SYMBOL_GPL(scsi_dh_attach);
  *      that may have a device handler attached
  * @gfp - the GFP mask used in the kmalloc() call when allocating memory
  *
- * Returns name of attached handler, NULL if no handler is attached.
+ * Returns name of attached handler, NULL if no handler is attached, or
+ * and error pointer if an error occurred.
  * Caller must take care to free the returned string.
  */
 const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp)
@@ -363,10 +364,11 @@ const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp)
 
 	sdev = scsi_device_from_queue(q);
 	if (!sdev)
-		return NULL;
+		return ERR_PTR(-ENODEV);
 
 	if (sdev->handler)
-		handler_name = kstrdup(sdev->handler->name, gfp);
+		handler_name = kstrdup(sdev->handler->name, gfp) ? :
+			       ERR_PTR(-ENOMEM);
 	put_device(&sdev->sdev_gendev);
 	return handler_name;
 }
-- 
2.50.1

Re: [PATCH v2] scsi: scsi_dh: Return error pointer in scsi_dh_attached_handler_name

[Martin Wilck]

On Fri, 2025-12-05 at 20:00 -0500, Benjamin Marzinski wrote:
> If scsi_dh_attached_handler_name() fails to allocate the handler
> name,
> dm-multipath (its only caller) assumes there is no attached device
> handler, and sets the device up incorrectly. Return an error pointer
> instead, so multipath can distinguish between failure, success where
> there is no attached device handler, or when the path device is not
> a scsi device at all.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

Re: [PATCH v2] scsi: scsi_dh: Return error pointer in scsi_dh_attached_handler_name

[Martin K. Petersen]

Benjamin,

> If scsi_dh_attached_handler_name() fails to allocate the handler name,
> dm-multipath (its only caller) assumes there is no attached device
> handler, and sets the device up incorrectly. Return an error pointer
> instead, so multipath can distinguish between failure, success where
> there is no attached device handler, or when the path device is not a
> scsi device at all.

Applied to 6.19/scsi-staging, thanks!

-- 
Martin K. Petersen