[PATCH v7 0/6] libata-scsi: multi-LUN ATAPI device support

[PATCH v7 0/6] libata-scsi: multi-LUN ATAPI device support

[Phil Pemberton]

Some ATAPI devices expose more than one logical unit behind a single ATA
target: Panasonic/COMPAQ PD/CD combo drives (LUN 0 = CD-ROM, LUN 1 =
PD), and Nakamichi CD changers (one LUN per disc slot, up to 7).

libata has historically hard-coded shost->max_lun = 1, so the SCSI
layer never scans past LUN 0 on any ATA-attached device.  This series
lifts that restriction for ATAPI devices gated by BLIST_FORCELUN.

Changes since v6
================

Addressing review from Hannes Reinecke (v6 review) and automated
analysis (Sashiko):

Patch 1: MODULE_PARM_DESC clarified to describe the parameter as a
count of LUNs (not a maximum LUN number), and the range corrected from
"1..7" to "1..8" (eight LUN slots, values 0..7).

Patch 2:
  - Drop the nr_luns companion field.  The ATAPI_MAX_LUN constant (8)
    is the correct fixed bound for all iterations; nr_luns adds
    complexity without benefit since sdev[] slots not populated are
    always NULL.  Use ATAPI_MAX_LUN throughout (Hannes Reinecke).
  - Fix ata_port_detach UAF window: clear dev->sdev[lun] to NULL before
    the spin_unlock, then pass the saved pointer to scsi_remove_device()
    (Hannes Reinecke).
  - ata_scsi_sdev_destroy: trigger ATA-level detach only when all LUN
    slots are NULL, not only on LUN 0 destruction.
  - ZPODD: iterate all LUN slots in zpodd_enable_run_wake(),
    zpodd_post_poweron(), and zpodd_wake_dev() (Hannes Reinecke).
  - ata_scsi_dev_rescan: snapshot all LUN sdevs under the spinlock
    before releasing it; release remaining refs on early exit paths.

Patch 3:
  - atapi_xlat(): always clear CDB byte 1 bits 7:5 first, then set them
    only for non-zero LUNs.  This avoids silently zeroing those bits on
    every LUN-0 command regardless of LUN, and separates the "zero for
    LUN 0" from the "encode LUN" paths.
  - On WARN_ON_ONCE overflow, set scmd->result = DID_ERROR before
    returning so the SCSI layer does not treat the aborted command as
    a success.

Patch 4:
  - Move the BLIST_NO_LUN_1F -> pdt_1f_for_no_lun assignment from
    scsi_add_lun() to scsi_probe_and_add_lun(), immediately before the
    PDT=0x1f check.  scsi_add_lun() is called after that check, so the
    v6 placement was too late to suppress LUN 0 if it returned
    PDT=0x1f.

Patch 5:
  - Call ata_scsi_assign_ofnode() before scsi_device_put(), so the
    reference to dev->sdev[0] is still held during the OF node
    assignment.

Patches 3/6, 4/6, and 5/6 carry Reviewed-by from Hannes Reinecke
(unchanged from v6).

Series structure
================

  1/6  ata: libata-scsi: add atapi_max_lun module parameter
  2/6  ata: libata-scsi: convert dev->sdev to per-LUN array
  3/6  ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI
  4/6  scsi: add BLIST_NO_LUN_1F blacklist flag
  5/6  ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
  6/6  scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

Testing
=======

Tested on real hardware (Panasonic/COMPAQ LF-1195C on Intel ICH5 PATA):
  [x] Boot with CD inserted: sr0 attaches, mount and read files
  [x] Boot with PD inserted: sda attaches at correct capacity
      (1298496 x 512 B = 634 MiB)
  [x] All seven LUNs scanned (atapi_max_lun=7); LUNs 2..6 correctly
      report PDT 0x1f and are silently skipped
  [x] Single-LUN ATAPI CD-ROM (LITE-ON iHAS124): no regression,
      only LUN 0 scanned

Known limitations
=================

Media-change events are not propagated across LUNs of a SINGLELUN
multi-LUN device.  The SCSI layer's UA handling is per-sdev.  On the
PD/CD combo, swapping media and then accessing the other LUN may return
stale capacity until a manual rescan:

  echo 1 > /sys/class/scsi_device/H:0:0:1/device/rescan

A follow-up patch to propagate media-change events to sibling LUNs is
deferred to keep this series focused on the LUN-scanning core.

Phil Pemberton (6):
  ata: libata-scsi: add atapi_max_lun module parameter
  ata: libata-scsi: convert dev->sdev to per-LUN array
  ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI
  scsi: add BLIST_NO_LUN_1F blacklist flag
  ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
  scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

 drivers/ata/libata-acpi.c   |   9 +-
 drivers/ata/libata-core.c   |  16 ++-
 drivers/ata/libata-scsi.c   | 226 ++++++++++++++++++++++++------------
 drivers/ata/libata-zpodd.c  |  27 ++++-
 drivers/ata/libata.h        |   1 +
 drivers/scsi/scsi_devinfo.c |   2 +
 drivers/scsi/scsi_scan.c    |   3 +
 include/linux/libata.h      |  11 +-
 include/scsi/scsi_devinfo.h |   6 +-
 9 files changed, 210 insertions(+), 91 deletions(-)

base-commit: a3f75e5e6b023958c92ad03fa2e68e047b6169c4
--
2.43.0

[PATCH v7 1/6] ata: libata-scsi: add atapi_max_lun module parameter

[Phil Pemberton]

Until now libata has hard-coded shost->max_lun = 1 for every ATA host,
so the SCSI layer never scans past LUN 0.  This blocks support for
the small handful of multi-LUN ATAPI devices (Panasonic LF-1195C and
COMPAQ PD-1 PD/CD combos export CD on LUN 0 and PD on LUN 1; old
Nakamichi MJ-x.y CD changers expose one LUN per disc slot, up to 7).

Introduce a libata module parameter, atapi_max_lun, that controls the
upper bound of the per-host SCSI LUN scan.  Default is 1, preserving
current behaviour exactly: out-of-the-box only LUN 0 is scanned.
Range is clamped to 1..ATAPI_MAX_LUN (8, the SCSI-2 ceiling, covering
LUN values 0..7).

Subsequent patches gate actual LUN>0 probing on BLIST_FORCELUN, so a
device must both be on the SCSI device list (or carry the appropriate
quirk) and run on a host whose atapi_max_lun has been raised before
any extra LUNs are scanned.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-core.c | 5 +++++
 drivers/ata/libata-scsi.c | 2 +-
 drivers/ata/libata.h      | 1 +
 include/linux/libata.h    | 1 +
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e76d15411e2a..d39ac4292f81 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -122,6 +122,11 @@ int atapi_passthru16 = 1;
 module_param(atapi_passthru16, int, 0444);
 MODULE_PARM_DESC(atapi_passthru16, "Enable ATA_16 passthru for ATAPI devices (0=off, 1=on [default])");
 
+int atapi_max_lun = 1;
+module_param(atapi_max_lun, int, 0444);
+MODULE_PARM_DESC(atapi_max_lun,
+	"Number of LUNs to scan on ATAPI devices flagged BLIST_FORCELUN (1 [default] = LUN 0 only, 8 = all SCSI-2 LUNs 0..7)");
+
 int libata_fua = 0;
 module_param_named(fua, libata_fua, int, 0444);
 MODULE_PARM_DESC(fua, "FUA support (0=off [default], 1=on)");
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f44612e269a4..32c6a0e497cf 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4627,7 +4627,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
 		shost->transportt = &ata_scsi_transportt;
 		shost->unique_id = ap->print_id;
 		shost->max_id = 16;
-		shost->max_lun = 1;
+		shost->max_lun = clamp(atapi_max_lun, 1, ATAPI_MAX_LUN);
 		shost->max_channel = 1;
 		shost->max_cmd_len = 32;
 
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index b5423b6e97de..96d804d02b99 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -33,6 +33,7 @@ enum {
 #define ATA_PORT_TYPE_NAME	"ata_port"
 
 extern int atapi_passthru16;
+extern int atapi_max_lun;
 extern int libata_fua;
 extern int libata_noacpi;
 extern int libata_allow_tpm;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5c085ef4eda7..3e33ee30628d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -131,6 +131,7 @@ enum {
 	ATA_SHORT_PAUSE		= 16,
 
 	ATAPI_MAX_DRAIN		= 16 << 10,
+	ATAPI_MAX_LUN		= 8,	/* SCSI-2 cap (LUN values 0..7) */
 
 	ATA_ALL_DEVICES		= (1 << ATA_MAX_DEVICES) - 1,
 
-- 
2.43.0

[PATCH v7 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array

[Phil Pemberton]

Multi-LUN ATAPI devices (PD/CD combos, CD changers) share a single
ata_device but expose multiple scsi_devices.  The previous single
dev->sdev pointer could only track one LUN, making all other LUNs
invisible to code that operates on sdevs: port detach, suspend/resume,
ACPI uevent, ZPODD, media change notification, and EH teardown.

Replace the scalar struct scsi_device *sdev with a fixed-size array
dev->sdev[ATAPI_MAX_LUN] indexed by LUN number, where ATAPI_MAX_LUN
is 8 (the SCSI-2 ceiling, LUN values 0..7).  All callers are updated
to iterate the full array and skip NULL slots; only populated LUN slots
are ever non-NULL so single-LUN devices (the vast majority) see no
change in behaviour.

Add an inline helper ata_dev_scsi_device(dev, lun) that returns
dev->sdev[lun] guarded by a WARN_ON_ONCE(lun >= ATAPI_MAX_LUN) bounds
check.  Use it for the hardcoded LUN-0 references in libata-acpi
(uevent kobj), libata-zpodd (disk events, wake notify for all LUNs),
and the door-lock and OF-node paths in libata-scsi.

Key changes per call site:
  - ata_scsi_dev_config:    bounds-check lun, assign sdev to dev->sdev[sdev->lun]
  - ata_scsi_sdev_destroy:  clear per-LUN slot; trigger ATA detach only
                            when the last populated LUN is destroyed
  - ata_port_detach:        iterate all ATAPI_MAX_LUN slots descending;
                            clear dev->sdev[lun] before unlock to close
                            the UAF window (Hannes Reinecke)
  - ata_scsi_offline_dev:   iterate all slots
  - ata_scsi_remove_dev:    snapshot all LUN slots then remove outside lock
  - ata_scsi_media_change_notify: send event to all populated LUNs
  - ata_scsi_dev_rescan:    snapshot all LUNs under lock, then resume and
                            rescan each; release remaining refs on early exit
  - ACPI, ZPODD, door-lock: use ata_dev_scsi_device(dev, 0)
  - ZPODD disk-events:      iterate all LUNs for enable/disable and wake

Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-acpi.c  |   9 +-
 drivers/ata/libata-core.c  |  11 ++-
 drivers/ata/libata-scsi.c  | 164 +++++++++++++++++++++----------------
 drivers/ata/libata-zpodd.c |  27 ++++--
 include/linux/libata.h     |  10 ++-
 5 files changed, 138 insertions(+), 83 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 4433f626246b..2d1662f6f064 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -153,10 +153,13 @@ static void ata_acpi_uevent(struct ata_port *ap, struct ata_device *dev,
 	char *envp[] = { event_string, NULL };
 
 	if (dev) {
-		if (dev->sdev)
-			kobj = &dev->sdev->sdev_gendev.kobj;
-	} else
+		struct scsi_device *sdev = ata_dev_scsi_device(dev, 0);
+
+		if (sdev)
+			kobj = &sdev->sdev_gendev.kobj;
+	} else {
 		kobj = &ap->dev->kobj;
+	}
 
 	if (kobj) {
 		snprintf(event_string, 20, "BAY_EVENT=%d", event);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d39ac4292f81..11f8a341252d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6275,11 +6275,16 @@ static void ata_port_detach(struct ata_port *ap)
 	/* Remove scsi devices */
 	ata_for_each_link(link, ap, HOST_FIRST) {
 		ata_for_each_dev(dev, link, ALL) {
-			if (dev->sdev) {
+			int lun;
+
+			for (lun = ATAPI_MAX_LUN - 1; lun >= 0; lun--) {
+				struct scsi_device *sdev = dev->sdev[lun];
+				if (!sdev)
+					continue;
+				dev->sdev[lun] = NULL;
 				spin_unlock_irqrestore(ap->lock, flags);
-				scsi_remove_device(dev->sdev);
+				scsi_remove_device(sdev);
 				spin_lock_irqsave(ap->lock, flags);
-				dev->sdev = NULL;
 			}
 		}
 	}
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 32c6a0e497cf..b65358955cf1 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1131,7 +1131,9 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
 	if (dev->flags & ATA_DFLAG_TRUSTED)
 		sdev->security_supported = 1;
 
-	dev->sdev = sdev;
+	if (WARN_ON_ONCE(sdev->lun >= ATAPI_MAX_LUN))
+		return -EINVAL;
+	dev->sdev[sdev->lun] = sdev;
 	return 0;
 }
 
@@ -1202,10 +1204,10 @@ EXPORT_SYMBOL_GPL(ata_scsi_sdev_configure);
  *
  *	@sdev is about to be destroyed for hot/warm unplugging.  If
  *	this unplugging was initiated by libata as indicated by NULL
- *	dev->sdev, this function doesn't have to do anything.
+ *	dev->sdev[], this function doesn't have to do anything.
  *	Otherwise, SCSI layer initiated warm-unplug is in progress.
- *	Clear dev->sdev, schedule the device for ATA detach and invoke
- *	EH.
+ *	Clear the per-LUN slot; when the last LUN (LUN 0) is destroyed,
+ *	schedule ATA-level detach via EH.
  *
  *	LOCKING:
  *	Defined by SCSI layer.  We don't really care.
@@ -1220,11 +1222,23 @@ void ata_scsi_sdev_destroy(struct scsi_device *sdev)
 
 	spin_lock_irqsave(ap->lock, flags);
 	dev = __ata_scsi_find_dev(ap, sdev);
-	if (dev && dev->sdev) {
-		/* SCSI device already in CANCEL state, no need to offline it */
-		dev->sdev = NULL;
-		dev->flags |= ATA_DFLAG_DETACH;
-		ata_port_schedule_eh(ap);
+	if (dev && !WARN_ON_ONCE(sdev->lun >= ATAPI_MAX_LUN) &&
+	    dev->sdev[sdev->lun] == sdev) {
+		int lun;
+		bool last;
+
+		dev->sdev[sdev->lun] = NULL;
+		last = true;
+		for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
+			if (dev->sdev[lun]) {
+				last = false;
+				break;
+			}
+		}
+		if (last) {
+			dev->flags |= ATA_DFLAG_DETACH;
+			ata_port_schedule_eh(ap);
+		}
 	}
 	spin_unlock_irqrestore(ap->lock, flags);
 
@@ -2909,12 +2923,9 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
 		 *
 		 * If door lock fails, always clear sdev->locked to
 		 * avoid this infinite loop.
-		 *
-		 * This may happen before SCSI scan is complete.  Make
-		 * sure qc->dev->sdev isn't NULL before dereferencing.
 		 */
-		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL && qc->dev->sdev)
-			qc->dev->sdev->locked = 0;
+		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL)
+			qc->scsicmd->device->locked = 0;
 
 		ata_scsi_qc_done(qc, true, SAM_STAT_CHECK_CONDITION);
 		return;
@@ -4658,7 +4669,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
 #ifdef CONFIG_OF
 static void ata_scsi_assign_ofnode(struct ata_device *dev, struct ata_port *ap)
 {
-	struct scsi_device *sdev = dev->sdev;
+	struct scsi_device *sdev = ata_dev_scsi_device(dev, 0);
 	struct device *d = ap->host->dev;
 	struct device_node *np = d->of_node;
 	struct device_node *child;
@@ -4696,7 +4707,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			struct scsi_device *sdev;
 			int channel = 0, id = 0;
 
-			if (dev->sdev)
+			if (dev->sdev[0])
 				continue;
 
 			if (ata_is_host_link(link))
@@ -4707,11 +4718,11 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
 						 NULL);
 			if (!IS_ERR(sdev)) {
-				dev->sdev = sdev;
+				dev->sdev[0] = sdev;
 				ata_scsi_assign_ofnode(dev, ap);
 				scsi_device_put(sdev);
 			} else {
-				dev->sdev = NULL;
+				dev->sdev[0] = NULL;
 			}
 		}
 	}
@@ -4722,7 +4733,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 	 */
 	ata_for_each_link(link, ap, EDGE) {
 		ata_for_each_dev(dev, link, ENABLED) {
-			if (!dev->sdev)
+			if (!dev->sdev[0])
 				goto exit_loop;
 		}
 	}
@@ -4763,7 +4774,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
  *
  *	This function is called from ata_eh_detach_dev() and is responsible for
  *	taking the SCSI device attached to @dev offline.  This function is
- *	called with host lock which protects dev->sdev against clearing.
+ *	called with host lock which protects dev->sdev[] against clearing.
  *
  *	LOCKING:
  *	spin_lock_irqsave(host lock)
@@ -4773,11 +4784,16 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
  */
 bool ata_scsi_offline_dev(struct ata_device *dev)
 {
-	if (dev->sdev) {
-		scsi_device_set_state(dev->sdev, SDEV_OFFLINE);
-		return true;
+	bool found = false;
+	int lun;
+
+	for (lun = ATAPI_MAX_LUN - 1; lun >= 0; lun--) {
+		if (dev->sdev[lun]) {
+			scsi_device_set_state(dev->sdev[lun], SDEV_OFFLINE);
+			found = true;
+		}
 	}
-	return false;
+	return found;
 }
 
 /**
@@ -4793,49 +4809,38 @@ bool ata_scsi_offline_dev(struct ata_device *dev)
 static void ata_scsi_remove_dev(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
-	struct scsi_device *sdev;
+	struct scsi_device *sdevs[ATAPI_MAX_LUN] = {};
 	unsigned long flags;
+	int lun;
 
-	/* Alas, we need to grab scan_mutex to ensure SCSI device
-	 * state doesn't change underneath us and thus
-	 * scsi_device_get() always succeeds.  The mutex locking can
-	 * be removed if there is __scsi_device_get() interface which
-	 * increments reference counts regardless of device state.
-	 */
 	mutex_lock(&ap->scsi_host->scan_mutex);
 	spin_lock_irqsave(ap->lock, flags);
 
-	/* clearing dev->sdev is protected by host lock */
-	sdev = dev->sdev;
-	dev->sdev = NULL;
+	for (lun = ATAPI_MAX_LUN - 1; lun >= 0; lun--) {
+		struct scsi_device *sdev = dev->sdev[lun];
+
+		dev->sdev[lun] = NULL;
+		if (!sdev)
+			continue;
 
-	if (sdev) {
-		/* If user initiated unplug races with us, sdev can go
-		 * away underneath us after the host lock and
-		 * scan_mutex are released.  Hold onto it.
-		 */
 		if (scsi_device_get(sdev) == 0) {
-			/* The following ensures the attached sdev is
-			 * offline on return from ata_scsi_offline_dev()
-			 * regardless it wins or loses the race
-			 * against this function.
-			 */
 			scsi_device_set_state(sdev, SDEV_OFFLINE);
+			sdevs[lun] = sdev;
 		} else {
 			WARN_ON(1);
-			sdev = NULL;
 		}
 	}
 
 	spin_unlock_irqrestore(ap->lock, flags);
 	mutex_unlock(&ap->scsi_host->scan_mutex);
 
-	if (sdev) {
+	for (lun = ATAPI_MAX_LUN - 1; lun >= 0; lun--) {
+		if (!sdevs[lun])
+			continue;
 		ata_dev_info(dev, "detaching (SCSI %s)\n",
-			     dev_name(&sdev->sdev_gendev));
-
-		scsi_remove_device(sdev);
-		scsi_device_put(sdev);
+			     dev_name(&sdevs[lun]->sdev_gendev));
+		scsi_remove_device(sdevs[lun]);
+		scsi_device_put(sdevs[lun]);
 	}
 }
 
@@ -4872,9 +4877,12 @@ static void ata_scsi_handle_link_detach(struct ata_link *link)
  */
 void ata_scsi_media_change_notify(struct ata_device *dev)
 {
-	if (dev->sdev)
-		sdev_evt_send_simple(dev->sdev, SDEV_EVT_MEDIA_CHANGE,
-				     GFP_ATOMIC);
+	int lun;
+
+	for (lun = 0; lun < ATAPI_MAX_LUN; lun++)
+		if (dev->sdev[lun])
+			sdev_evt_send_simple(dev->sdev[lun],
+					     SDEV_EVT_MEDIA_CHANGE, GFP_ATOMIC);
 }
 
 /**
@@ -5007,7 +5015,8 @@ void ata_scsi_dev_rescan(struct work_struct *work)
 
 	ata_for_each_link(link, ap, EDGE) {
 		ata_for_each_dev(dev, link, ENABLED) {
-			struct scsi_device *sdev = dev->sdev;
+			struct scsi_device *sdevs[ATAPI_MAX_LUN] = {};
+			int lun;
 
 			/*
 			 * If the port was suspended before this was scheduled,
@@ -5016,28 +5025,43 @@ void ata_scsi_dev_rescan(struct work_struct *work)
 			if (ap->pflags & ATA_PFLAG_SUSPENDED)
 				goto unlock_ap;
 
-			if (!sdev)
-				continue;
-			if (scsi_device_get(sdev))
-				continue;
+			for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
+				if (dev->sdev[lun] &&
+				    !scsi_device_get(dev->sdev[lun]))
+					sdevs[lun] = dev->sdev[lun];
+			}
 
 			do_resume = dev->flags & ATA_DFLAG_RESUMING;
 
-			spin_unlock_irqrestore(ap->lock, flags);
-			if (do_resume) {
-				ret = scsi_resume_device(sdev);
-				if (ret == -EWOULDBLOCK) {
-					scsi_device_put(sdev);
-					goto unlock_scan;
+			for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
+				if (!sdevs[lun])
+					continue;
+
+				spin_unlock_irqrestore(ap->lock, flags);
+				if (do_resume) {
+					ret = scsi_resume_device(sdevs[lun]);
+					if (ret == -EWOULDBLOCK) {
+						scsi_device_put(sdevs[lun]);
+						while (++lun < ATAPI_MAX_LUN)
+							if (sdevs[lun])
+								scsi_device_put(sdevs[lun]);
+						goto unlock_scan;
+					}
+				}
+				ret = scsi_rescan_device(sdevs[lun]);
+				scsi_device_put(sdevs[lun]);
+				spin_lock_irqsave(ap->lock, flags);
+
+				if (ret) {
+					while (++lun < ATAPI_MAX_LUN)
+						if (sdevs[lun])
+							scsi_device_put(sdevs[lun]);
+					goto unlock_ap;
 				}
-				dev->flags &= ~ATA_DFLAG_RESUMING;
 			}
-			ret = scsi_rescan_device(sdev);
-			scsi_device_put(sdev);
-			spin_lock_irqsave(ap->lock, flags);
 
-			if (ret)
-				goto unlock_ap;
+			if (do_resume)
+				dev->flags &= ~ATA_DFLAG_RESUMING;
 		}
 	}
 
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index 414e7c63bd85..151ae5726aca 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -184,8 +184,13 @@ bool zpodd_zpready(struct ata_device *dev)
 void zpodd_enable_run_wake(struct ata_device *dev)
 {
 	struct zpodd *zpodd = dev->zpodd;
+	int lun;
 
-	sdev_disable_disk_events(dev->sdev);
+	for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
+		struct scsi_device *sdev = dev->sdev[lun];
+		if (sdev)
+			sdev_disable_disk_events(sdev);
+	}
 
 	zpodd->powered_off = true;
 	acpi_pm_set_device_wakeup(&dev->tdev, true);
@@ -218,6 +223,7 @@ void zpodd_disable_run_wake(struct ata_device *dev)
 void zpodd_post_poweron(struct ata_device *dev)
 {
 	struct zpodd *zpodd = dev->zpodd;
+	int lun;
 
 	if (!zpodd->powered_off)
 		return;
@@ -233,18 +239,27 @@ void zpodd_post_poweron(struct ata_device *dev)
 	zpodd->zp_sampled = false;
 	zpodd->zp_ready = false;
 
-	sdev_enable_disk_events(dev->sdev);
+	for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
+		struct scsi_device *sdev = dev->sdev[lun];
+		if (sdev)
+			sdev_enable_disk_events(sdev);
+	}
 }
 
 static void zpodd_wake_dev(acpi_handle handle, u32 event, void *context)
 {
 	struct ata_device *ata_dev = context;
 	struct zpodd *zpodd = ata_dev->zpodd;
-	struct device *dev = &ata_dev->sdev->sdev_gendev;
+	int lun;
 
-	if (event == ACPI_NOTIFY_DEVICE_WAKE && pm_runtime_suspended(dev)) {
-		zpodd->from_notify = true;
-		pm_runtime_resume(dev);
+	if (event != ACPI_NOTIFY_DEVICE_WAKE)
+		return;
+	for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
+		struct scsi_device *sdev = ata_dev->sdev[lun];
+		if (sdev && pm_runtime_suspended(&sdev->sdev_gendev)) {
+			zpodd->from_notify = true;
+			pm_runtime_resume(&sdev->sdev_gendev);
+		}
 	}
 }
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3e33ee30628d..237a8d259ba7 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -722,7 +722,7 @@ struct ata_device {
 	unsigned int		devno;		/* 0 or 1 */
 	u64			quirks;		/* List of broken features */
 	unsigned long		flags;		/* ATA_DFLAG_xxx */
-	struct scsi_device	*sdev;		/* attached SCSI device */
+	struct scsi_device	*sdev[ATAPI_MAX_LUN];	/* per-LUN SCSI devices */
 	void			*private_data;
 #ifdef CONFIG_ATA_ACPI
 	union acpi_object	*gtf_cache;
@@ -1715,6 +1715,14 @@ static inline unsigned int ata_dev_absent(const struct ata_device *dev)
 	return ata_class_absent(dev->class);
 }
 
+static inline struct scsi_device *
+ata_dev_scsi_device(struct ata_device *dev, unsigned int lun)
+{
+	if (WARN_ON_ONCE(lun >= ATAPI_MAX_LUN))
+		return NULL;
+	return dev->sdev[lun];
+}
+
 /*
  * link helpers
  */
-- 
2.43.0

[PATCH v7 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI

[Phil Pemberton]

Two changes are required to route commands to ATAPI LUNs other than 0:

1. __ata_scsi_find_dev():  The existing code rejects any scsi_device
   with a non-zero LUN, returning NULL and dropping the command on
   the floor.  Hoist a non-zero LUN early-exit ahead of the original
   channel/id checks: when scsidev->lun is non-zero, allow it through
   only if the underlying ata_device is ATAPI class.  The original
   LUN-0 path is left structurally unchanged.

2. atapi_xlat():  Older ATAPI devices (SCSI-2 era) expect the LUN in
   CDB byte 1 bits 7:5 rather than relying on transport-level LUN
   addressing.  Always clear those bits first, then encode
   scmd->device->lun into them for non-zero LUNs.  This is required by
   both the Panasonic PD/CD combos and Nakamichi CD changers.

   Guard with WARN_ON_ONCE() and fail the command (setting scmd->result
   to DID_ERROR) if the LUN is out of range, since the 3-bit CDB field
   cannot represent it.

Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-scsi.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b65358955cf1..2e3b5fd41d05 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2958,6 +2958,20 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
 	memset(qc->cdb, 0, dev->cdb_len);
 	memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);
 
+	/*
+	 * SCSI-2 CDB LUN encoding: bits 7:5 of byte 1 (3-bit field).
+	 * Always clear those bits; only set them for non-zero LUNs.
+	 */
+	qc->cdb[1] = qc->cdb[1] & 0x1f;
+	if (unlikely(scmd->device->lun)) {
+		if (WARN_ON_ONCE(scmd->device->host->max_lun > ATAPI_MAX_LUN ||
+				 scmd->device->lun >= scmd->device->host->max_lun)) {
+			scmd->result = DID_ERROR << 16;
+			return 1;
+		}
+		qc->cdb[1] |= (u8)scmd->device->lun << 5;
+	}
+
 	qc->complete_fn = atapi_qc_complete;
 
 	qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
@@ -3068,6 +3082,29 @@ static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
 {
 	int devno;
 
+	/*
+	 * Non-zero LUN is only legal for ATAPI devices, since they can
+	 * legitimately expose more than one LUN (PD/CD combos, CD changers).
+	 * Handle that case up front so the LUN-0 path below stays unchanged.
+	 */
+	if (unlikely(scsidev->lun)) {
+		struct ata_device *dev;
+
+		if (!sata_pmp_attached(ap)) {
+			if (unlikely(scsidev->channel))
+				return NULL;
+			devno = scsidev->id;
+		} else {
+			if (unlikely(scsidev->id))
+				return NULL;
+			devno = scsidev->channel;
+		}
+		dev = ata_find_dev(ap, devno);
+		if (!dev || dev->class != ATA_DEV_ATAPI)
+			return NULL;
+		return dev;
+	}
+
 	/* skip commands not addressed to targets we simulate */
 	if (!sata_pmp_attached(ap)) {
 		if (unlikely(scsidev->channel || scsidev->lun))
-- 
2.43.0

[PATCH v7 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag

[Phil Pemberton]

Some multi-LUN devices respond to INQUIRY on unpopulated LUNs with
PQ=0 / PDT=0x1f instead of the standard PQ=3.  The SCSI scan layer
normally adds such devices (PQ=0 means "connected"), producing
spurious "No Device" entries.

The scsi_target field pdt_1f_for_no_lun already exists to suppress
this, but was previously only set by the USB UFI driver.

Add BLIST_NO_LUN_1F so the flag can be set per-device from
scsi_devinfo, and wire it up in scsi_probe_and_add_lun() to set
starget->pdt_1f_for_no_lun from the blacklist flags.  This is placed
immediately before the PDT=0x1f check so it takes effect for all LUNs,
including LUN 0, without waiting for scsi_add_lun() to run.

Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/scsi/scsi_scan.c    | 3 +++
 include/scsi/scsi_devinfo.h | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ef22a4228b85..ef0f5ee6be87 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1285,6 +1285,9 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 	 * PDT=00h Direct-access device (floppy)
 	 * PDT=1Fh none (no FDD connected to the requested logical unit)
 	 */
+	if (bflags & BLIST_NO_LUN_1F)
+		starget->pdt_1f_for_no_lun = 1;
+
 	if (((result[0] >> 5) == 1 || starget->pdt_1f_for_no_lun) &&
 	    (result[0] & 0x1f) == 0x1f &&
 	    !scsi_is_wlun(lun)) {
diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 1d79a3b536ce..6957b0705510 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -34,7 +34,8 @@
 #define BLIST_NOSTARTONADD	((__force blist_flags_t)(1ULL << 12))
 /* do not ask for VPD page size first on some broken targets */
 #define BLIST_NO_VPD_SIZE	((__force blist_flags_t)(1ULL << 13))
-#define __BLIST_UNUSED_14	((__force blist_flags_t)(1ULL << 14))
+/* PDT 0x1f with PQ 0 means no LUN present (e.g. some ATAPI multi-LUN) */
+#define BLIST_NO_LUN_1F		((__force blist_flags_t)(1ULL << 14))
 #define __BLIST_UNUSED_15	((__force blist_flags_t)(1ULL << 15))
 #define __BLIST_UNUSED_16	((__force blist_flags_t)(1ULL << 16))
 /* try REPORT_LUNS even for SCSI-2 devs (if HBA supports more than 8 LUNs) */
@@ -77,8 +78,7 @@
 #define __BLIST_HIGH_UNUSED (~(__BLIST_LAST_USED | \
 			       (__force blist_flags_t) \
 			       ((__force __u64)__BLIST_LAST_USED - 1ULL)))
-#define __BLIST_UNUSED_MASK (__BLIST_UNUSED_14 | \
-			     __BLIST_UNUSED_15 | \
+#define __BLIST_UNUSED_MASK (__BLIST_UNUSED_15 | \
 			     __BLIST_UNUSED_16 | \
 			     __BLIST_UNUSED_24 | \
 			     __BLIST_UNUSED_27 | \
-- 
2.43.0

[PATCH v7 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

[Phil Pemberton]

After LUN 0 is added for an ATAPI device, check its BLIST_FORCELUN
flag.  If set, call scsi_scan_target() with SCAN_WILD_CARD to trigger
the SCSI layer's built-in sequential LUN scan for that target only.
This probes LUNs 1..shost->max_lun, driven by the libata atapi_max_lun
module parameter.

Devices without BLIST_FORCELUN (the vast majority of ATAPI devices)
are left with only LUN 0 -- no sequential scan is triggered, so
single-LUN devices like the iHAS124 DVD writer are completely
unaffected.

Non-responding LUNs (PQ=0/PDT=0x1f) are silently skipped by
scsi_probe_and_add_lun() when BLIST_NO_LUN_1F is set on the device
via scsi_devinfo.

Also fix a TOCTOU window: call ata_scsi_assign_ofnode() before
scsi_device_put() so the reference to dev->sdev[0] is held while
the OF node is assigned.

Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-scsi.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2e3b5fd41d05..64cb2860a67b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -26,6 +26,7 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_tcq.h>
 #include <scsi/scsi_transport.h>
+#include <scsi/scsi_devinfo.h>
 #include <linux/libata.h>
 #include <linux/hdreg.h>
 #include <linux/uaccess.h>
@@ -4754,13 +4755,27 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 
 			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
 						 NULL);
-			if (!IS_ERR(sdev)) {
-				dev->sdev[0] = sdev;
-				ata_scsi_assign_ofnode(dev, ap);
-				scsi_device_put(sdev);
-			} else {
+			if (IS_ERR(sdev)) {
 				dev->sdev[0] = NULL;
+				continue;
 			}
+
+			/*
+			 * For multi-LUN ATAPI (BLIST_FORCELUN), trigger a
+			 * sequential scan for this target.  pdt_1f_for_no_lun,
+			 * set during LUN 0 configure, ensures non-responding
+			 * LUNs are silently skipped; dev->sdev[] is populated
+			 * by ata_scsi_dev_config() during the scan.
+			 */
+			if (dev->class == ATA_DEV_ATAPI &&
+			    sdev->sdev_bflags & BLIST_FORCELUN &&
+			    !WARN_ON_ONCE(ap->scsi_host->max_lun > ATAPI_MAX_LUN))
+				scsi_scan_target(&ap->scsi_host->shost_gendev,
+						 channel, id, SCAN_WILD_CARD,
+						 SCSI_SCAN_RESCAN);
+			if (dev->sdev[0])
+				ata_scsi_assign_ofnode(dev, ap);
+			scsi_device_put(sdev);
 		}
 	}
 
-- 
2.43.0

[PATCH v7 6/6] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

[Phil Pemberton]

The Compaq PD-1 (and equivalent Panasonic LF-1195C) is a combination
PD/CD-ROM drive that exposes two LUNs: LUN 0 is the CD-ROM and LUN 1
is the PD (Phase-change rewritable) drive.

Add a scsi_devinfo entry with BLIST_FORCELUN to enable multi-LUN
scanning, BLIST_SINGLELUN to prevent issuing LUN-aware commands
simultaneously, and BLIST_NO_LUN_1F to suppress spurious "No Device"
entries for unpopulated LUNs (which respond with PQ=0/PDT=0x1f).

Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/scsi/scsi_devinfo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 68a992494b12..bfc2cbd43897 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -150,6 +150,8 @@ static struct {
 	{"COMPAQ", "MSA1000", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
 	{"COMPAQ", "MSA1000 VOLUME", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
 	{"COMPAQ", "HSV110", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
+	{"COMPAQ", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN |
+				 BLIST_NO_LUN_1F},
 	{"DDN", "SAN DataDirector", "*", BLIST_SPARSELUN},
 	{"DEC", "HSG80", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
 	{"DELL", "PV660F", NULL, BLIST_SPARSELUN},
-- 
2.43.0

Re: [PATCH v7 6/6] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Max-length vendor (8 characters) and model (16 characters) static quirks are silently ignored due to forced null-termination by `strscpy_pad`.
--

commit 5a65a1f1e5d686dcd28440ac5645f5a5b22ab1ae
Author: Phil Pemberton <philpem@philpem.me.uk>

scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

The Compaq PD-1 (and equivalent Panasonic LF-1195C) is a combination
PD/CD-ROM drive that exposes two LUNs: LUN 0 is the CD-ROM and LUN 1
is the PD (Phase-change rewritable) drive.

Add a scsi_devinfo entry with BLIST_FORCELUN to enable multi-LUN
scanning, BLIST_SINGLELUN to prevent issuing LUN-aware commands
simultaneously, and BLIST_NO_LUN_1F to suppress spurious "No Device"
entries for unpopulated LUNs (which respond with PQ=0/PDT=0x1f).

> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> index 68a992494b121..bfc2cbd438975 100644
> --- a/drivers/scsi/scsi_devinfo.c
> +++ b/drivers/scsi/scsi_devinfo.c
> @@ -150,6 +150,8 @@ static struct {
>  	{"COMPAQ", "MSA1000", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
>  	{"COMPAQ", "MSA1000 VOLUME", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
>  	{"COMPAQ", "HSV110", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
> +	{"COMPAQ", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN |
> +				 BLIST_NO_LUN_1F},
[Severity: High]
This isn't a bug introduced by this patch, but max-length vendor (8
characters) and model (16 characters) static quirks in this list are
silently ignored due to forced null-termination by strscpy_pad().

While this specific COMPAQ quirk is short enough to avoid the issue, the use
of strscpy_pad() in scsi_strcpy_devinfo() forces a null-terminator for
string lengths exactly equal to the maximum array size:

drivers/scsi/scsi_devinfo.c:scsi_strcpy_devinfo() {
	...
	strscpy_pad(to, from, to_length);
	...
}

If a vendor string is 8 characters long (e.g. MATSHITA, transtec) or a model
is 16 characters long (e.g. ARRAY CONTROLLER, ImageMate CF-SD1),
strscpy_pad() truncates the string to leave room for the null byte.

In scsi_dev_info_list_find(), the matching logic compares the length of the
device's INQUIRY data (vmax) against strnlen(devinfo->vendor, 8):

drivers/scsi/scsi_devinfo.c:scsi_dev_info_list_find() {
	...
	if (vmax != strnlen(devinfo->vendor,
			    sizeof(devinfo->vendor)) ||
	...
}

Since vmax is 8 but strnlen() returns 7 for truncated strings, the length
comparison fails. This silently breaks critical SCSI quirks. Without these
quirks, devices like the Compaq ARRAY CONTROLLER fail to report LUNs
correctly (leading to missing disks or unbootable systems), or devices could
lock up the SCSI bus.

Could the truncation logic be revised to safely handle exact length strings
without null-termination?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611024356.2769320-1-philpem@philpem.me.uk?part=6

Re: [PATCH v7 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] LUN validation incorrectly checks the host's max_lun instead of the device's LUN limit, breaking SAS HBAs and introducing a kernel panic vector.
- [High] Unconditionally clearing bits 7:5 of CDB byte 1 destroys command-specific parameters for standard MMC commands, breaking Blu-ray decryption.
- [High] Error handling routines for ATAPI do not propagate the non-zero LUN, causing atapi_eh_request_sense() to incorrectly fetch sense data for LUN 0.
--

commit 0fa4117328a093e17803a117416e6ddc64dee46b
Author: Phil Pemberton <philpem@philpem.me.uk>

ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI

This patch modifies __ata_scsi_find_dev() to allow non-zero LUNs for ATAPI
devices and updates atapi_xlat() to encode the LUN into CDB byte 1 as
expected by older SCSI-2 era devices.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index b65358955cf17..2e3b5fd41d05f 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c

[ ... ]

> @@ -2958,6 +2958,20 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
>  	memset(qc->cdb, 0, dev->cdb_len);
>  	memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);
>  
> +	/*
> +	 * SCSI-2 CDB LUN encoding: bits 7:5 of byte 1 (3-bit field).
> +	 * Always clear those bits; only set them for non-zero LUNs.
> +	 */
> +	qc->cdb[1] = qc->cdb[1] & 0x1f;

[Severity: High]
Could this unconditional masking cause a regression by destroying
command-specific parameters for standard MMC commands?

Modern ATAPI/MMC devices reuse these bits for essential parameters since they
no longer rely on LUN addressing inside the CDB. For example, the REPORT KEY
command (0xA4) relies on bits 7:6 of byte 1 for the Key Class. Clearing
these bits forces the Key Class to 0 (DVD CSS), which breaks Blu-ray AACS
decryption tools for SATA/ATAPI optical drives.

> +	if (unlikely(scmd->device->lun)) {
> +		if (WARN_ON_ONCE(scmd->device->host->max_lun > ATAPI_MAX_LUN ||
> +				 scmd->device->lun >= scmd->device->host->max_lun)) {

[Severity: High]
Does this check incorrectly validate against the host max_lun instead of the
device LUN limit?

On SAS HBAs (like mpt3sas), host->max_lun is legitimately set to very large
values (e.g., 16895). Because 16895 > 8 (ATAPI_MAX_LUN), any command sent to
a non-zero LUN on an ATAPI device attached to such an HBA will trigger this
WARN_ON_ONCE and fail with DID_ERROR. 

On systems with panic_on_warn enabled, this will cause a kernel panic when a
multi-LUN ATAPI device is scanned.

[Severity: High]
Is there a missing update for error handling routines to propagate the
non-zero LUN, leading to a regression in sense data retrieval?

While this patch routes non-zero LUN commands by mutating CDB byte 1 in
atapi_xlat(), it appears this routing is missing from error handling. When a
command fails, ata_eh_analyze_tf() calls atapi_eh_request_sense().

In drivers/ata/libata-eh.c:atapi_eh_request_sense():
    u8 cdb[ATAPI_CDB_LEN] =
        { REQUEST_SENSE, 0, 0, 0, SCSI_SENSE_BUFFERSIZE, 0 };

This constructs a hardcoded REQUEST_SENSE CDB with byte 1 explicitly set to
0. If an I/O error occurs on LUN 1, the kernel will fetch the sense data
for LUN 0 instead. 

This could cause the SCSI midlayer to receive incorrect error status,
potentially masking legitimate I/O errors and leading to infinite retry
loops.

> +			scmd->result = DID_ERROR << 16;
> +			return 1;
> +		}
> +		qc->cdb[1] |= (u8)scmd->device->lun << 5;
> +	}
> +
>  	qc->complete_fn = atapi_qc_complete;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611024356.2769320-1-philpem@philpem.me.uk?part=3

Re: [PATCH v7 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsynchronized read-modify-write of bitfields in `struct scsi_target` (`pdt_1f_for_no_lun`) causes a data race that can corrupt concurrent updates to `expecting_lun_change`.
--

commit 22aca9342fbe7619010508e9209d9f91ff554f8a
Author: Phil Pemberton <philpem@philpem.me.uk>

scsi: add BLIST_NO_LUN_1F blacklist flag

This commit adds a new blacklist flag, BLIST_NO_LUN_1F, to handle multi-LUN
devices that respond to INQUIRY on unpopulated LUNs with PQ=0 / PDT=0x1f. It
wires this flag up in scsi_probe_and_add_lun() to set the pdt_1f_for_no_lun
field on the scsi_target.

> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index ef22a4228b855..ef0f5ee6be87d 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -1285,6 +1285,9 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
>  	 * PDT=00h Direct-access device (floppy)
>  	 * PDT=1Fh none (no FDD connected to the requested logical unit)
>  	 */
> +	if (bflags & BLIST_NO_LUN_1F)
> +		starget->pdt_1f_for_no_lun = 1;

[Severity: High]
Does this unprotected bitfield assignment risk a data race?

In include/scsi/scsi_device.h, pdt_1f_for_no_lun shares a memory word with
expecting_lun_change in struct scsi_target.

If this function processes LUN 1 while LUN 0 has already been added and is
processing I/O, a concurrent UNIT_ATTENTION could trigger an update to
expecting_lun_change from softirq context in
drivers/scsi/scsi_error.c:scsi_report_lun_change():

	sdev->sdev_target->expecting_lun_change = 1;

Since bitfield assignments are implemented as non-atomic read-modify-write
sequences by the compiler, could this process-context write to
pdt_1f_for_no_lun silently overwrite and drop the softirq's update to
expecting_lun_change?

> +
>  	if (((result[0] >> 5) == 1 || starget->pdt_1f_for_no_lun) &&
>  	    (result[0] & 0x1f) == 0x1f &&
>  	    !scsi_is_wlun(lun)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611024356.2769320-1-philpem@philpem.me.uk?part=4

Re: [PATCH v7 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition exists between checking `dev->sdev[0]` in `ata_scsi_scan_host()` and re-fetching it inside `ata_scsi_assign_ofnode()`, potentially leading to a NULL pointer dereference.
--

commit 67ae5925dcb0647224ec016d88000305f0e5c2f2
Author: Phil Pemberton <philpem@philpem.me.uk>

ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

This commit probes additional LUNs for ATAPI devices with the BLIST_FORCELUN
flag by triggering a sequential LUN scan. Non-responding LUNs are silently
skipped. It also attempts to fix a TOCTOU window by calling
ata_scsi_assign_ofnode() before releasing the scsi_device reference.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 2e3b5fd41d05f..64cb2860a67b1 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[ ... ]
> @@ -4754,13 +4755,27 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
[ ... ]
> +			if (dev->class == ATA_DEV_ATAPI &&
> +			    sdev->sdev_bflags & BLIST_FORCELUN &&
> +			    !WARN_ON_ONCE(ap->scsi_host->max_lun > ATAPI_MAX_LUN))
> +				scsi_scan_target(&ap->scsi_host->shost_gendev,
> +						 channel, id, SCAN_WILD_CARD,
> +						 SCSI_SCAN_RESCAN);
> +			if (dev->sdev[0])
> +				ata_scsi_assign_ofnode(dev, ap);

[Severity: High]
This is a pre-existing issue, but does holding the scsi_device reference
actually prevent the TOCTOU race here?

While this patch holds the scsi_device_put() until after the call to
ata_scsi_assign_ofnode(), a concurrent sysfs hot-unplug can trigger
scsi_remove_device(). This unconditionally calls ata_scsi_sdev_destroy(),
which safely acquires ap->lock and sets dev->sdev[0] = NULL, regardless of
the scsi_device refcount.

If dev->sdev[0] is cleared right after the unprotected if (dev->sdev[0])
check, the re-fetch inside ata_scsi_assign_ofnode() will return NULL:

drivers/ata/libata-scsi.c:ata_scsi_assign_ofnode() {
    ...
    struct scsi_device *sdev = ata_dev_scsi_device(dev, 0);
    ...
    sdev->sdev_gendev.of_node = child;
    ...
}

This would lead to a kernel panic when dereferencing sdev.

Could the local sdev pointer be passed directly to ata_scsi_assign_ofnode()
instead of re-fetching it from dev->sdev[0]?

> +			scsi_device_put(sdev);
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611024356.2769320-1-philpem@philpem.me.uk?part=5

Re: [PATCH v7 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Deadlock caused by calling scsi_device_put() while holding an IRQ-disabled spinlock.
- [High] Infinite retry loop and LUN starvation due to early loop exit in ata_scsi_dev_rescan().
- [Medium] Denial of Service via WARN_ON_ONCE triggered by malicious device LUN.

Pre-existing issues:
- [High] Use-After-Free in ata_port_detach due to unreferenced scsi_device.
- [High] Use-After-Free in ZPODD wake callbacks racing with concurrent sysfs device deletion.
--

commit 49dbde19170ff410a516820c897120bfd39b44b3
Author: Phil Pemberton <philpem@philpem.me.uk>
Subject: ata: libata-scsi: convert dev->sdev to per-LUN array

Multi-LUN ATAPI devices share a single ata_device but expose multiple
scsi_devices. This patch replaces the scalar struct scsi_device *sdev
with a fixed-size array dev->sdev[ATAPI_MAX_LUN] indexed by LUN number,
allowing callers to operate on all populated LUNs.

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index d39ac4292f818..11f8a341252db 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6275,11 +6275,16 @@ static void ata_port_detach(struct ata_port *ap)
>  	/* Remove scsi devices */
>  	ata_for_each_link(link, ap, HOST_FIRST) {
>  		ata_for_each_dev(dev, link, ALL) {
> -			if (dev->sdev) {
> +			int lun;
> +
> +			for (lun = ATAPI_MAX_LUN - 1; lun >= 0; lun--) {
> +				struct scsi_device *sdev = dev->sdev[lun];
> +				if (!sdev)
> +					continue;
> +				dev->sdev[lun] = NULL;
>  				spin_unlock_irqrestore(ap->lock, flags);
> -				scsi_remove_device(dev->sdev);
> +				scsi_remove_device(sdev);
>  				spin_lock_irqsave(ap->lock, flags);
> -				dev->sdev = NULL;
>  			}
>  		}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does dropping ap->lock without
taking a reference to sdev open a race condition in ata_port_detach() in
drivers/ata/libata-core.c?

If a concurrent sysfs device deletion drops the last reference right after
the lock is released, could scsi_remove_device(sdev) trigger a use-after-free
when it accesses sdev->host to lock the scan mutex?

Would it be safer to use scsi_device_get() before dropping the lock, and
scsi_device_put() after scsi_remove_device()?

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 32c6a0e497cf9..b65358955cf17 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1131,7 +1131,9 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
>  	if (dev->flags & ATA_DFLAG_TRUSTED)
>  		sdev->security_supported = 1;
>  
> -	dev->sdev = sdev;
> +	if (WARN_ON_ONCE(sdev->lun >= ATAPI_MAX_LUN))
> +		return -EINVAL;
> +	dev->sdev[sdev->lun] = sdev;
>  	return 0;
>  }

[Severity: Medium]
In ata_scsi_dev_config() in drivers/ata/libata-scsi.c, can an external device
provide an untrusted LUN (for example, via a crafted REPORT LUNS response over
USB)?

If so, would this WARN_ON_ONCE crash the machine when panic_on_warn is
enabled?

Should this use ata_dev_warn() or ata_dev_err() instead of WARN_ON_ONCE?

[ ... ]

> @@ -5016,28 +5025,43 @@ void ata_scsi_dev_rescan(struct work_struct *work)
>  			if (ap->pflags & ATA_PFLAG_SUSPENDED)
>  				goto unlock_ap;
>  
> -			if (!sdev)
> -				continue;
> -			if (scsi_device_get(sdev))
> -				continue;
> +			for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
> +				if (dev->sdev[lun] &&
> +				    !scsi_device_get(dev->sdev[lun]))
> +					sdevs[lun] = dev->sdev[lun];
> +			}
>  
>  			do_resume = dev->flags & ATA_DFLAG_RESUMING;
>  
> -			spin_unlock_irqrestore(ap->lock, flags);
> -			if (do_resume) {
> -				ret = scsi_resume_device(sdev);
> -				if (ret == -EWOULDBLOCK) {
> -					scsi_device_put(sdev);
> -					goto unlock_scan;
> +			for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
> +				if (!sdevs[lun])
> +					continue;
> +
> +				spin_unlock_irqrestore(ap->lock, flags);
> +				if (do_resume) {
> +					ret = scsi_resume_device(sdevs[lun]);
> +					if (ret == -EWOULDBLOCK) {
> +						scsi_device_put(sdevs[lun]);
> +						while (++lun < ATAPI_MAX_LUN)
> +							if (sdevs[lun])
> +								scsi_device_put(sdevs[lun]);
> +						goto unlock_scan;
> +					}
> +				}
> +				ret = scsi_rescan_device(sdevs[lun]);
> +				scsi_device_put(sdevs[lun]);
> +				spin_lock_irqsave(ap->lock, flags);
> +
> +				if (ret) {
> +					while (++lun < ATAPI_MAX_LUN)
> +						if (sdevs[lun])
> +							scsi_device_put(sdevs[lun]);
> +					goto unlock_ap;
>  				}

[Severity: High]
In ata_scsi_dev_rescan() in drivers/ata/libata-scsi.c, could calling
scsi_device_put() here while holding the IRQ-disabled ap->lock cause a
deadlock?

If this drops the last reference to the device, the synchronous release
function ata_scsi_sdev_destroy() will execute. That function attempts to
acquire ap->lock again, which would cause a hard deadlock on the same CPU
since we just acquired it via spin_lock_irqsave(ap->lock, flags).

[Severity: High]
If scsi_rescan_device() returns an error for an early LUN (such as LUN 0), we
exit the loop early via goto unlock_ap.

Does this skip the dev->flags &= ~ATA_DFLAG_RESUMING clearance that happens
at the end of the loop?

Since the work item reschedules itself on error, would this cause an infinite
loop that repeatedly attempts to scan LUN 0 while permanently starving the
higher LUNs?

> -				dev->flags &= ~ATA_DFLAG_RESUMING;
>  			}
> -			ret = scsi_rescan_device(sdev);
> -			scsi_device_put(sdev);
> -			spin_lock_irqsave(ap->lock, flags);
>  
> -			if (ret)
> -				goto unlock_ap;
> +			if (do_resume)
> +				dev->flags &= ~ATA_DFLAG_RESUMING;
>  		}
>  	}

> diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
> index 414e7c63bd850..151ae5726aca1 100644
> --- a/drivers/ata/libata-zpodd.c
> +++ b/drivers/ata/libata-zpodd.c
[ ... ]
> @@ -233,18 +239,27 @@ void zpodd_post_poweron(struct ata_device *dev)
>  	zpodd->zp_sampled = false;
>  	zpodd->zp_ready = false;
>  
> -	sdev_enable_disk_events(dev->sdev);
> +	for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
> +		struct scsi_device *sdev = dev->sdev[lun];
> +		if (sdev)
> +			sdev_enable_disk_events(sdev);
> +	}
>  }
>  
>  static void zpodd_wake_dev(acpi_handle handle, u32 event, void *context)
>  {
>  	struct ata_device *ata_dev = context;
>  	struct zpodd *zpodd = ata_dev->zpodd;
> -	struct device *dev = &ata_dev->sdev->sdev_gendev;
> +	int lun;
>  
> -	if (event == ACPI_NOTIFY_DEVICE_WAKE && pm_runtime_suspended(dev)) {
> -		zpodd->from_notify = true;
> -		pm_runtime_resume(dev);
> +	if (event != ACPI_NOTIFY_DEVICE_WAKE)
> +		return;
> +	for (lun = 0; lun < ATAPI_MAX_LUN; lun++) {
> +		struct scsi_device *sdev = ata_dev->sdev[lun];
> +		if (sdev && pm_runtime_suspended(&sdev->sdev_gendev)) {

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to iterate and
dereference ata_dev->sdev[lun] here in zpodd_wake_dev() in
drivers/ata/libata-zpodd.c without holding ap->lock or acquiring a reference
via scsi_device_get()?

If a SCSI device is concurrently removed (for example, via a sysfs delete
operation), could the scsi_device structure be freed while the ACPI notify
callback is executing?

Would this lead to a use-after-free when pm_runtime_suspended() is called?

> +			zpodd->from_notify = true;
> +			pm_runtime_resume(&sdev->sdev_gendev);
> +		}
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611024356.2769320-1-philpem@philpem.me.uk?part=2

Re: [PATCH v7 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array

[Hannes Reinecke]

On 6/11/26 04:43, Phil Pemberton wrote:
> Multi-LUN ATAPI devices (PD/CD combos, CD changers) share a single
> ata_device but expose multiple scsi_devices.  The previous single
> dev->sdev pointer could only track one LUN, making all other LUNs
> invisible to code that operates on sdevs: port detach, suspend/resume,
> ACPI uevent, ZPODD, media change notification, and EH teardown.
> 
> Replace the scalar struct scsi_device *sdev with a fixed-size array
> dev->sdev[ATAPI_MAX_LUN] indexed by LUN number, where ATAPI_MAX_LUN
> is 8 (the SCSI-2 ceiling, LUN values 0..7).  All callers are updated
> to iterate the full array and skip NULL slots; only populated LUN slots
> are ever non-NULL so single-LUN devices (the vast majority) see no
> change in behaviour.
> 
> Add an inline helper ata_dev_scsi_device(dev, lun) that returns
> dev->sdev[lun] guarded by a WARN_ON_ONCE(lun >= ATAPI_MAX_LUN) bounds
> check.  Use it for the hardcoded LUN-0 references in libata-acpi
> (uevent kobj), libata-zpodd (disk events, wake notify for all LUNs),
> and the door-lock and OF-node paths in libata-scsi.
> 
> Key changes per call site:
>    - ata_scsi_dev_config:    bounds-check lun, assign sdev to dev->sdev[sdev->lun]
>    - ata_scsi_sdev_destroy:  clear per-LUN slot; trigger ATA detach only
>                              when the last populated LUN is destroyed
>    - ata_port_detach:        iterate all ATAPI_MAX_LUN slots descending;
>                              clear dev->sdev[lun] before unlock to close
>                              the UAF window (Hannes Reinecke)
>    - ata_scsi_offline_dev:   iterate all slots
>    - ata_scsi_remove_dev:    snapshot all LUN slots then remove outside lock
>    - ata_scsi_media_change_notify: send event to all populated LUNs
>    - ata_scsi_dev_rescan:    snapshot all LUNs under lock, then resume and
>                              rescan each; release remaining refs on early exit
>    - ACPI, ZPODD, door-lock: use ata_dev_scsi_device(dev, 0)
>    - ZPODD disk-events:      iterate all LUNs for enable/disable and wake
> 
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>   drivers/ata/libata-acpi.c  |   9 +-
>   drivers/ata/libata-core.c  |  11 ++-
>   drivers/ata/libata-scsi.c  | 164 +++++++++++++++++++++----------------
>   drivers/ata/libata-zpodd.c |  27 ++++--
>   include/linux/libata.h     |  10 ++-
>   5 files changed, 138 insertions(+), 83 deletions(-)
> 

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v7 6/6] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

[Hannes Reinecke]

On 6/11/26 04:43, Phil Pemberton wrote:
> The Compaq PD-1 (and equivalent Panasonic LF-1195C) is a combination
> PD/CD-ROM drive that exposes two LUNs: LUN 0 is the CD-ROM and LUN 1
> is the PD (Phase-change rewritable) drive.
> 
> Add a scsi_devinfo entry with BLIST_FORCELUN to enable multi-LUN
> scanning, BLIST_SINGLELUN to prevent issuing LUN-aware commands
> simultaneously, and BLIST_NO_LUN_1F to suppress spurious "No Device"
> entries for unpopulated LUNs (which respond with PQ=0/PDT=0x1f).
> 
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>   drivers/scsi/scsi_devinfo.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> index 68a992494b12..bfc2cbd43897 100644
> --- a/drivers/scsi/scsi_devinfo.c
> +++ b/drivers/scsi/scsi_devinfo.c
> @@ -150,6 +150,8 @@ static struct {
>   	{"COMPAQ", "MSA1000", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
>   	{"COMPAQ", "MSA1000 VOLUME", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
>   	{"COMPAQ", "HSV110", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
> +	{"COMPAQ", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN |
> +				 BLIST_NO_LUN_1F},
>   	{"DDN", "SAN DataDirector", "*", BLIST_SPARSELUN},
>   	{"DEC", "HSG80", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
>   	{"DELL", "PV660F", NULL, BLIST_SPARSELUN},

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich