* [PATCH v8 1/6] ata: libata-scsi: add atapi_max_lun module parameter
2026-07-31 21:34 [PATCH v8 0/6] libata-scsi: multi-LUN ATAPI device support Phil Pemberton
@ 2026-07-31 21:34 ` Phil Pemberton
2026-07-31 21:34 ` [PATCH v8 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array Phil Pemberton
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Phil Pemberton @ 2026-07-31 21:34 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: linux-kernel, Damien Le Moal, Niklas Cassel,
James E . J . Bottomley, Martin K . Petersen, Hannes Reinecke,
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>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
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 d893c916df0b..15ee44cf5bf2 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 1d225ee9eb86..04c96f3fd865 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -5154,7 +5154,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 39494ab206a2..7ee3bf1a4789 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 18edb36c29fc..25e4b671adfb 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -180,6 +180,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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v8 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array
2026-07-31 21:34 [PATCH v8 0/6] libata-scsi: multi-LUN ATAPI device support Phil Pemberton
2026-07-31 21:34 ` [PATCH v8 1/6] ata: libata-scsi: add atapi_max_lun module parameter Phil Pemberton
@ 2026-07-31 21:34 ` Phil Pemberton
2026-07-31 22:07 ` sashiko-bot
2026-07-31 21:35 ` [PATCH v8 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI Phil Pemberton
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Phil Pemberton @ 2026-07-31 21:34 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: linux-kernel, Damien Le Moal, Niklas Cassel,
James E . J . Bottomley, Martin K . Petersen, Hannes Reinecke,
Phil Pemberton, Hannes Reinecke
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
Reviewed-by: Hannes Reinecke <hare@kernel.org>
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 15ee44cf5bf2..43d5221dc347 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6381,11 +6381,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 04c96f3fd865..808368b952b5 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1129,7 +1129,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;
}
@@ -1200,10 +1202,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.
@@ -1218,11 +1220,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);
@@ -2963,12 +2977,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;
@@ -5185,7 +5196,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;
@@ -5223,7 +5234,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))
@@ -5234,11 +5245,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;
}
}
}
@@ -5249,7 +5260,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;
}
}
@@ -5290,7 +5301,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)
@@ -5300,11 +5311,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;
}
/**
@@ -5320,49 +5336,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]);
}
}
@@ -5399,9 +5404,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);
}
/**
@@ -5534,7 +5542,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,
@@ -5543,28 +5552,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 25e4b671adfb..3b9207a9f334 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -733,7 +733,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;
@@ -1726,6 +1726,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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array
2026-07-31 21:34 ` [PATCH v8 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array Phil Pemberton
@ 2026-07-31 22:07 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-31 22:07 UTC (permalink / raw)
To: Phil Pemberton; +Cc: linux-scsi, cassel, dlemoal, linux-ide
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Sleeping in atomic context due to `scsi_device_put()` under spinlock in `ata_scsi_dev_rescan` error path
Pre-existing issues:
- [High] Double-free or reference count underflow of OF device node in `ata_scsi_assign_ofnode`
- [High] Use-After-Free in `ata_port_detach` due to missing reference count before `scsi_remove_device`
--
--- Patch [2]: [PATCH v8 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array ---
commit 576f81319ae7ef4943e02f2cfa1da008a5f4256c
Author: Phil Pemberton <philpem@philpem.me.uk>
Subject: ata: libata-scsi: convert dev->sdev to per-LUN array
This commit replaces the scalar dev->sdev pointer with an array of
scsi_device pointers to support multi-LUN ATAPI devices. Code paths
are updated to iterate over all ATAPI_MAX_LUN slots instead of assuming
a single LUN.
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 15ee44cf5bf23..43d5221dc347c 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6381,11 +6381,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);
[Severity: High]
This is a pre-existing issue, but does dropping the lock here without
holding a reference to sdev expose us to a use-after-free?
The commit message mentions that clearing the array slot closes the UAF
window, but if a concurrent sysfs removal drops the final reference
while the lock is released, could scsi_remove_device(sdev) end up
dereferencing freed memory?
Would it be safer to call scsi_device_get(sdev) before dropping the lock?
> spin_lock_irqsave(ap->lock, flags);
> - dev->sdev = NULL;
> }
> }
> }
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 04c96f3fd8652..808368b952b51 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[ ... ]
> @@ -5185,7 +5196,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;
[Severity: High]
This isn't a bug introduced by this patch, but does
ata_scsi_assign_ofnode() cause a reference count underflow or
double-free on the device node?
If for_each_available_child_of_node() assigns the child pointer to both
dev->tdev.of_node and sdev->sdev_gendev.of_node, we have two pointers
sharing a single acquired reference. When both devices are destroyed,
they will each call of_node_put(), potentially leading to a double-free.
Should we add an of_node_get() for the second assignment?
[ ... ]
> @@ -5534,7 +5542,8 @@ void ata_scsi_dev_rescan(struct work_struct *work)
[ ... ]
> ret = scsi_rescan_device(sdevs[lun]);
> scsi_device_put(sdevs[lun]);
> spin_lock_irqsave(ap->lock, flags);
>
> - if (ret)
> - goto unlock_ap;
> + if (ret) {
> + while (++lun < ATAPI_MAX_LUN)
> + if (sdevs[lun])
> + scsi_device_put(sdevs[lun]);
> + goto unlock_ap;
> }
[Severity: High]
Can this call to scsi_device_put() sleep while holding the spinlock?
Since this block runs after spin_lock_irqsave(ap->lock, flags) is
called, interrupts are disabled. If a concurrent removal has dropped
other references, this scsi_device_put() call might drop the final
reference and invoke the release handler scsi_device_dev_release().
Since the release handler contains blocking calls like
cancel_work_sync(), could this cause a schedule-while-atomic BUG?
> - 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;
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260731213503.2379771-1-philpem@philpem.me.uk?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI
2026-07-31 21:34 [PATCH v8 0/6] libata-scsi: multi-LUN ATAPI device support Phil Pemberton
2026-07-31 21:34 ` [PATCH v8 1/6] ata: libata-scsi: add atapi_max_lun module parameter Phil Pemberton
2026-07-31 21:34 ` [PATCH v8 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array Phil Pemberton
@ 2026-07-31 21:35 ` Phil Pemberton
2026-07-31 22:07 ` sashiko-bot
2026-07-31 21:35 ` [PATCH v8 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag Phil Pemberton
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Phil Pemberton @ 2026-07-31 21:35 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: linux-kernel, Damien Le Moal, Niklas Cassel,
James E . J . Bottomley, Martin K . Petersen, Hannes Reinecke,
Phil Pemberton, Hannes Reinecke
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 808368b952b5..0b1e4842860c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3012,6 +3012,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;
@@ -3122,6 +3136,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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI
2026-07-31 21:35 ` [PATCH v8 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI Phil Pemberton
@ 2026-07-31 22:07 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-31 22:07 UTC (permalink / raw)
To: Phil Pemberton; +Cc: cassel, linux-scsi, dlemoal, linux-ide
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Flawed LUN validation logic in atapi_xlat() incorrectly checks the host's max_lun instead of just the device limit, triggering a WARN_ON_ONCE and failing commands for ATAPI devices attached to SAS HBAs.
- [High] Unconditionally clearing bits 7:5 of CDB byte 1 in `atapi_xlat()` corrupts commands that repurpose these bits, such as SECURITY PROTOCOL IN/OUT.
--
--- Patch [3]: [PATCH v8 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI ---
commit 3d48864344b5c5d234865e96735b1a7de55c13b5
Author: Phil Pemberton <philpem@philpem.me.uk>
ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI
This commit modifies libata-scsi to support routing commands to non-zero
ATAPI LUNs. It updates __ata_scsi_find_dev to allow non-zero LUNs for
ATAPI devices and modifies atapi_xlat to encode the LUN into the SCSI-2
CDB for non-zero LUNs.
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3012,6 +3012,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]
Does unconditionally clearing bits 7:5 of CDB byte 1 corrupt commands that
repurpose these bits?
For example, in SECURITY PROTOCOL IN (0xA2) and OUT (0xB5) commands, byte 1
is used as the 8-bit Security Protocol field. Stripping the top 3 bits would
mutate protocol 0xEF into 0x0F, which could break features like Trusted
Computing (e.g., AACS on Blu-Ray, Opal) if a userspace application issues
raw SG_IO commands to the ATAPI device.
> + if (unlikely(scmd->device->lun)) {
> + if (WARN_ON_ONCE(scmd->device->host->max_lun > ATAPI_MAX_LUN ||
[Severity: High]
Will this WARN_ON_ONCE incorrectly trigger and fail commands for ATAPI
devices attached to SAS HBAs?
If a multi-LUN ATAPI device is attached to a host controller that advertises
a large max_lun (such as libsas drivers like hisi_sas which set
shost->max_lun = ~0), the condition checking if the host's max_lun is
greater than ATAPI_MAX_LUN evaluates to true.
This would unconditionally fail commands directed to non-zero LUNs with
DID_ERROR, breaking multi-LUN ATAPI support on modern HBAs.
> + 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;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260731213503.2379771-1-philpem@philpem.me.uk?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag
2026-07-31 21:34 [PATCH v8 0/6] libata-scsi: multi-LUN ATAPI device support Phil Pemberton
` (2 preceding siblings ...)
2026-07-31 21:35 ` [PATCH v8 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI Phil Pemberton
@ 2026-07-31 21:35 ` Phil Pemberton
2026-07-31 22:04 ` sashiko-bot
2026-07-31 21:35 ` [PATCH v8 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices Phil Pemberton
2026-07-31 21:35 ` [PATCH v8 6/6] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk Phil Pemberton
5 siblings, 1 reply; 11+ messages in thread
From: Phil Pemberton @ 2026-07-31 21:35 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: linux-kernel, Damien Le Moal, Niklas Cassel,
James E . J . Bottomley, Martin K . Petersen, Hannes Reinecke,
Phil Pemberton, Hannes Reinecke
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>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
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 e27da038603a..98bd4d49fd62 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1296,6 +1296,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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag
2026-07-31 21:35 ` [PATCH v8 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag Phil Pemberton
@ 2026-07-31 22:04 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-31 22:04 UTC (permalink / raw)
To: Phil Pemberton; +Cc: dlemoal, linux-scsi, cassel, linux-ide
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent modification of `struct scsi_target` bitfields introduces a data race, potentially causing updates to `pdt_1f_for_no_lun` or `expecting_lun_change` to be lost.
--
--- Patch [4]: [PATCH v8 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag ---
commit 90f0f3c59ea5fbec1b6c95c81d334666e21cd3a9
Author: Phil Pemberton <philpem@philpem.me.uk>
scsi: add BLIST_NO_LUN_1F blacklist flag
This commit adds the BLIST_NO_LUN_1F blacklist flag to allow devices to
suppress spurious "No Device" entries for unpopulated LUNs returning
PQ=0/PDT=0x1f. The flag is wired up in scsi_probe_and_add_lun() to set
starget->pdt_1f_for_no_lun.
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
[ ... ]
> @@ -1296,6 +1296,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]
This isn't a bug introduced by this patch, but could this bitfield assignment
introduce a data race with other bitfields in struct scsi_target?
Looking at include/scsi/scsi_device.h, pdt_1f_for_no_lun shares the same
memory word with other bitfields:
unsigned int single_lun:1;
unsigned int pdt_1f_for_no_lun:1;
unsigned int no_report_luns:1;
unsigned int expecting_lun_change:1;
If a multi-LUN device reports a LUN change (REPORT LUNS DATA HAS CHANGED) on
an active LUN, scsi_report_lun_change() will set expecting_lun_change = 1 in
softirq context.
Simultaneously, if the system handles the LUN change by rescanning the
target, scsi_probe_and_add_lun() executes in process context and sets
pdt_1f_for_no_lun = 1.
Because these contexts perform read-modify-write operations on the same
memory word without mutual exclusion (the softirq does not hold
shost->scan_mutex), one of the updates might be lost.
If expecting_lun_change is lost, the SCSI layer fails to handle a LUN change
UNIT ATTENTION correctly, which on removable devices triggers a spurious
media-change event (cmd->device->changed = 1 and ACTION_FAIL), failing
in-flight I/O. If pdt_1f_for_no_lun is lost, spurious LUNs may be added.
> 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/20260731213503.2379771-1-philpem@philpem.me.uk?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
2026-07-31 21:34 [PATCH v8 0/6] libata-scsi: multi-LUN ATAPI device support Phil Pemberton
` (3 preceding siblings ...)
2026-07-31 21:35 ` [PATCH v8 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag Phil Pemberton
@ 2026-07-31 21:35 ` Phil Pemberton
2026-07-31 22:10 ` sashiko-bot
2026-07-31 21:35 ` [PATCH v8 6/6] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk Phil Pemberton
5 siblings, 1 reply; 11+ messages in thread
From: Phil Pemberton @ 2026-07-31 21:35 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: linux-kernel, Damien Le Moal, Niklas Cassel,
James E . J . Bottomley, Martin K . Petersen, Hannes Reinecke,
Phil Pemberton, Hannes Reinecke
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 0b1e4842860c..5bbb3169bea7 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>
@@ -5281,13 +5282,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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
2026-07-31 21:35 ` [PATCH v8 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices Phil Pemberton
@ 2026-07-31 22:10 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-31 22:10 UTC (permalink / raw)
To: Phil Pemberton; +Cc: dlemoal, cassel, linux-ide, linux-scsi
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]` and dereferencing it inside `ata_scsi_assign_ofnode()`, potentially leading to a NULL pointer dereference.
--
--- Patch [5]: [PATCH v8 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices ---
commit 26698e8a692fea36d598b32f39fd7513b22c4cf9
Author: Phil Pemberton <philpem@philpem.me.uk>
ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
This commit checks the BLIST_FORCELUN flag after LUN 0 is added for an ATAPI
device. If set, it triggers a sequential LUN scan for that target using
scsi_scan_target() to probe additional LUNs. It also moves the call to
ata_scsi_assign_ofnode() before scsi_device_put() to hold the device reference.
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 0b1e4842860c1..5bbb3169bea70 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[ ... ]
> @@ -5281,13 +5282,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);
[Severity: High]
This is a pre-existing issue, but does this check leave a race condition where
concurrent device removals can cause a NULL pointer dereference?
In ata_scsi_scan_host(), the lockless if (dev->sdev[0]) check is evaluated
after releasing shost->scan_mutex (which was temporarily held by the preceding
scsi_scan_target and __scsi_add_device calls).
Concurrently, a hot-unplug event can trigger ata_scsi_remove_dev() on a
workqueue, which takes the scan_mutex and sets dev->sdev[0] = NULL.
If this concurrent removal occurs exactly after the check but before
ata_scsi_assign_ofnode() re-reads dev->sdev[0]:
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;
}
Can sdev be NULL when it is unconditionally dereferenced? Would it be safer
to pass the safely refcounted local sdev directly to ata_scsi_assign_ofnode()?
> + scsi_device_put(sdev);
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260731213503.2379771-1-philpem@philpem.me.uk?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 6/6] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk
2026-07-31 21:34 [PATCH v8 0/6] libata-scsi: multi-LUN ATAPI device support Phil Pemberton
` (4 preceding siblings ...)
2026-07-31 21:35 ` [PATCH v8 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices Phil Pemberton
@ 2026-07-31 21:35 ` Phil Pemberton
5 siblings, 0 replies; 11+ messages in thread
From: Phil Pemberton @ 2026-07-31 21:35 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: linux-kernel, Damien Le Moal, Niklas Cassel,
James E . J . Bottomley, Martin K . Petersen, Hannes Reinecke,
Phil Pemberton, Hannes Reinecke
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).
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
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 15ffbe93ac72..b3fbfcd5dd93 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
^ permalink raw reply related [flat|nested] 11+ messages in thread