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

[sashiko-bot@kernel.org]

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