Thread (83 messages) flat view 83 messages, 4 authors, 4d ago

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

From: sashiko-bot@kernel.org
Date: 2026-09-08 09:18:11
Also in: linux-s390, linux-scsi

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help