Re: [PATCH v6 04/40] scsi: rename sense field of struct scsi_failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing sentinel element in failure_defs array leads to stack out-of-bounds read.
--

commit 36d3480d935664ac35e1691894e4eefff76dfc0f
Author: Damien Le Moal <dlemoal@kernel.org>

scsi: rename sense field of struct scsi_failure
    
Rename the sense field of struct scsi_failure to sense_key. This makes
it clear that this field stores the sense key, and also unifies this
structure field names with the names used in struct scsi_sense_hdr.

> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 8694eeadd753e..e321471e8ca22 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
[ ... ]
> @@ -514,18 +514,18 @@ extern void scsi_sanitize_inquiry_string(unsigned char *s, int len);
>   */
>  #define SCMD_FAILURE_STAT_ANY	0xff
>  /*
> - * The following can be set to the scsi_failure sense, asc and ascq fields to
> - * match on any sense, ASC, or ASCQ value.
> + * The following can be set to the scsi_failure sense key, asc and ascq fields
> + * to match any sense key, ASC, and ASCQ value.
>   */
> -#define SCMD_FAILURE_SENSE_ANY	0xff
> -#define SCMD_FAILURE_ASC_ANY	0xff
> -#define SCMD_FAILURE_ASCQ_ANY	0xff
> +#define SCMD_FAILURE_SENSE_KEY_ANY	0xff
> +#define SCMD_FAILURE_ASC_ANY		0xff
> +#define SCMD_FAILURE_ASCQ_ANY		0xff
>  /* Always retry a matching failure. */
>  #define SCMD_FAILURE_NO_LIMIT	-1
>  
>  struct scsi_failure {
>  	int result;
> -	u8 sense;
> +	u8 sense_key;
>  	u8 asc;
>  	u8 ascq;
>  	/*

While reviewing the usage of struct scsi_failure, I noticed a related
regression in another file:

drivers/ufs/core/ufshcd.c:ufshcd_execute_start_stop() {
	struct scsi_failure failure_defs[] = {
		{
			.allowed = 2,
			.result = SCMD_FAILURE_RESULT_ANY,
		},
	};
...

[Severity: High]
This is a pre-existing issue, but does this array lack the {} terminator
required by the iteration logic in scsi_check_passthrough()?

A stack out-of-bounds read can occur because the loop expects the array of
scsi_failure elements to be terminated by an empty {} element (where
result == 0).

If the stack memory adjacent to the array contains a non-zero value where
result would be, the loop evaluates garbage data.

Could this cause spurious command retries (returning -EAGAIN) or
unpredictable error handling behavior?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908090308.1085097-1-dlemoal@kernel.org?part=4