Re: [PATCH 2/2] scsi: core: Convert inquiry information

[Damien Le Moal <dlemoal@kernel.org>]

On 5/13/26 04:46, Bart Van Assche wrote:
> Currently the vendor, model, and revision members of struct scsi_device
> are pointers to fixed-length strings that are not NUL-terminated.

s/NUL/NULL

> Fixed-precision format specifiers (e.g., "%.8s") are required whenever
> they are printed and strncmp() must be used to compare these fields.
> This is error-prone.
> 
> Convert these fields to fixed-size character arrays within struct
> scsi_device. Remove an !sdev->model check because sdev->model is now
> guaranteed not to be NULL.
> 
> This patch fixes a bug in the qla2xxx driver. It makes the following
> code safe:
> 
> 		if (state_flags & BIT_4)
> 			scmd_printk(KERN_WARNING, cp,
> 			    "Unsupported device '%s' found.\n",
> 			    cp->device->vendor);
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/hwmon/drivetemp.c  |  5 +----
>  drivers/scsi/scsi_scan.c   | 12 ++++++------
>  include/scsi/scsi_device.h |  7 ++++---
>  3 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
> index 002e0660a0b8..efe8b229bdbe 100644
> --- a/drivers/hwmon/drivetemp.c
> +++ b/drivers/hwmon/drivetemp.c
> @@ -306,13 +306,10 @@ static bool drivetemp_sct_avoid(struct drivetemp_data *st)
>  	struct scsi_device *sdev = st->sdev;
>  	unsigned int ctr;
>  
> -	if (!sdev->model)
> -		return false;
> -
>  	/*
>  	 * The "model" field contains just the raw SCSI INQUIRY response
>  	 * "product identification" field, which has a width of 16 bytes.
> -	 * This field is space-filled, but is NOT NULL-terminated.
> +	 * This field is space-filled and NUL-terminated.

s/NUL/NULL

>  	 */
>  	for (ctr = 0; ctr < ARRAY_SIZE(sct_avoid_models); ctr++)
>  		if (!strncmp(sdev->model, sct_avoid_models[ctr],
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index ef22a4228b85..6c3a5d451c1d 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -292,9 +292,9 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
>  	if (!sdev)
>  		goto out;
>  
> -	sdev->vendor = scsi_null_device_strs;
> -	sdev->model = scsi_null_device_strs;
> -	sdev->rev = scsi_null_device_strs;
> +	strscpy(sdev->vendor, scsi_null_device_strs);
> +	strscpy(sdev->model, scsi_null_device_strs);
> +	strscpy(sdev->rev, scsi_null_device_strs);
>  	sdev->host = shost;
>  	sdev->queue_ramp_up_period = SCSI_DEFAULT_RAMP_UP_PERIOD;
>  	sdev->id = starget->id;
> @@ -905,9 +905,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>  	if (sdev->inquiry == NULL)
>  		return SCSI_SCAN_NO_RESPONSE;
>  
> -	sdev->vendor = (char *) (sdev->inquiry + 8);
> -	sdev->model = (char *) (sdev->inquiry + 16);
> -	sdev->rev = (char *) (sdev->inquiry + 32);
> +	strscpy(sdev->vendor, sdev->inquiry + 8);
> +	strscpy(sdev->model, sdev->inquiry + 16);
> +	strscpy(sdev->rev, sdev->inquiry + 32);
>  
>  	sdev->is_ata = strncmp(sdev->vendor, "ATA     ", 8) == 0;
>  	if (sdev->is_ata) {
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 9c2a7bbe5891..029f5115b2ea 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -7,6 +7,7 @@
>  #include <linux/workqueue.h>
>  #include <linux/blk-mq.h>
>  #include <scsi/scsi.h>
> +#include <scsi/scsi_common.h>
>  #include <linux/atomic.h>
>  #include <linux/sbitmap.h>
>  
> @@ -137,9 +138,9 @@ struct scsi_device {
>  	struct mutex inquiry_mutex;
>  	unsigned char inquiry_len;	/* valid bytes in 'inquiry' */
>  	unsigned char * inquiry;	/* INQUIRY response data */
> -	const char * vendor;		/* [back_compat] point into 'inquiry' ... */
> -	const char * model;		/* ... after scan; point to static string */
> -	const char * rev;		/* ... "nullnullnullnull" before scan */
> +	char vendor[INQUIRY_VENDOR_LEN + 1];
> +	char model[INQUIRY_MODEL_LEN + 1];
> +	char rev[INQUIRY_REVISION_LEN + 1];
>  
>  #define SCSI_DEFAULT_VPD_LEN	255	/* default SCSI VPD page size (max) */
>  	struct scsi_vpd __rcu *vpd_pg0;


-- 
Damien Le Moal
Western Digital Research