[PATCH v2 0/2] scsi: 3ware: bound firmware error strings

[PATCH v2 0/2] scsi: 3ware: bound firmware error strings

[Pengpeng Hou]

The 3w-9xxx and 3w-sas drivers parse a fixed firmware-owned field
containing a description and an optional second string.  Both drivers must
terminate the field before finding that optional string and must not form a
pointer one byte past the field when the first string consumes its last
byte.

Changes in v2:
- Group the two same-root SCSI fixes into one ordered series.
- Factor the repeated bounded optional-string handling into one helper per
  driver.
- Preserve the prior standalone submission lineage below.
Link: https://lore.kernel.org/r/20260704010512.71912-1-pengpeng@iscas.ac.cn
Link: https://lore.kernel.org/r/20260704010619.81878-1-pengpeng@iscas.ac.cn

[PATCH v2 1/2] scsi: 3w-9xxx: bound firmware error strings

[Pengpeng Hou]

The controller response header stores the description and optional firmware
error string in its fixed 98-byte err_specific_desc field.
twa_aen_queue_event() used strlen() to find the second string before
forcing the final byte to NUL, while twa_fill_sense() used the same
unbounded layout directly.

If a response does not contain a terminator, strlen() can scan past
the field.
If the first string reaches the last field byte, adding one also forms a
pointer past the array.

Terminate the field before parsing it and use a shared helper that treats a
full first string as having no optional second string. This keeps both AEN
and sense reporting paths within the response field.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/scsi/3w-9xxx.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index 9b93a2440af8..b2462ee43008 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -149,6 +149,24 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
 static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int request_id);
 static char *twa_string_lookup(twa_message_type *table, unsigned int aen_code);
 
+/*
+ * The firmware field contains two NUL-terminated strings in one fixed-size
+ * array.  Make the last byte a terminator before finding the optional second
+ * string, and never form a pointer one byte past the array.
+ */
+static const char *twa_error_string(TW_Command_Apache_Header *header)
+{
+	size_t description_len;
+
+	header->err_specific_desc[sizeof(header->err_specific_desc) - 1] = '\0';
+	description_len = strnlen(header->err_specific_desc,
+				  sizeof(header->err_specific_desc));
+	if (description_len == sizeof(header->err_specific_desc) - 1)
+		return "";
+
+	return &header->err_specific_desc[description_len + 1];
+}
+
 /* Functions */
 
 /* Show some statistics about the card */
@@ -376,7 +394,7 @@ static void twa_aen_queue_event(TW_Device_Extension *tw_dev, TW_Command_Apache_H
 	TW_Event *event;
 	unsigned short aen;
 	char host[16];
-	char *error_str;
+	const char *error_str;
 
 	tw_dev->aen_count++;
 
@@ -404,10 +422,9 @@ static void twa_aen_queue_event(TW_Device_Extension *tw_dev, TW_Command_Apache_H
 	tw_dev->error_sequence_id++;
 
 	/* Check for embedded error string */
-	error_str = &(header->err_specific_desc[strlen(header->err_specific_desc)+1]);
-
-	header->err_specific_desc[sizeof(header->err_specific_desc) - 1] = '\0';
-	event->parameter_len = strlen(header->err_specific_desc);
+	error_str = twa_error_string(header);
+	event->parameter_len = strnlen(header->err_specific_desc,
+				       sizeof(header->err_specific_desc));
 	memcpy(event->parameter_data, header->err_specific_desc, event->parameter_len + (error_str[0] == '\0' ? 0 : (1 + strlen(error_str))));
 	if (event->severity != TW_AEN_SEVERITY_DEBUG)
 		printk(KERN_WARNING "3w-9xxx:%s AEN: %s (0x%02X:0x%04X): %s:%s.\n",
@@ -992,12 +1009,12 @@ static int twa_fill_sense(TW_Device_Extension *tw_dev, int request_id, int copy_
 	TW_Command_Full *full_command_packet;
 	unsigned short error;
 	int retval = 1;
-	char *error_str;
+	const char *error_str;
 
 	full_command_packet = tw_dev->command_packet_virt[request_id];
 
 	/* Check for embedded error string */
-	error_str = &(full_command_packet->header.err_specific_desc[strlen(full_command_packet->header.err_specific_desc) + 1]);
+	error_str = twa_error_string(&full_command_packet->header);
 
 	/* Don't print error for Logical unit not supported during rollcall */
 	error = le16_to_cpu(full_command_packet->header.status_block.error);
-- 
2.43.0

[PATCH v2 2/2] scsi: 3w-sas: bound firmware error strings

[Pengpeng Hou]

The controller response header stores the description and optional firmware
error string in its fixed 98-byte err_specific_desc field.
twl_aen_queue_event() and twl_fill_sense() use strlen().  They derive an
optional second string without first proving a terminator lies in the
firmware-owned field.

Without a terminator, strlen() can scan past the field.
A first string reaching the last field byte also makes adding one form a
pointer past the array.

Terminate the field before parsing it and use a shared helper that treats a
full first string as having no optional second string.  This keeps both AEN
and sense reporting paths within the response field.

Fixes: f619106bdd9d ("[SCSI] 3w-sas: Add new driver for LSI 3ware 9750")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/scsi/3w-sas.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
index 52dc1aa639f7..5fc46a90e026 100644
--- a/drivers/scsi/3w-sas.c
+++ b/drivers/scsi/3w-sas.c
@@ -92,6 +92,24 @@ MODULE_PARM_DESC(use_msi, "Use Message Signaled Interrupts. Default: 0");
 /* Function prototypes */
 static int twl_reset_device_extension(TW_Device_Extension *tw_dev, int ioctl_reset);
 
+/*
+ * The firmware field contains two NUL-terminated strings in one fixed-size
+ * array.  Make the last byte a terminator before finding the optional second
+ * string, and never form a pointer one byte past the array.
+ */
+static const char *twl_error_string(TW_Command_Apache_Header *header)
+{
+	size_t description_len;
+
+	header->err_specific_desc[sizeof(header->err_specific_desc) - 1] = '\0';
+	description_len = strnlen(header->err_specific_desc,
+				  sizeof(header->err_specific_desc));
+	if (description_len == sizeof(header->err_specific_desc) - 1)
+		return "";
+
+	return &header->err_specific_desc[description_len + 1];
+}
+
 /* Functions */
 
 /* This function returns AENs through sysfs */
@@ -226,7 +244,7 @@ static void twl_aen_queue_event(TW_Device_Extension *tw_dev, TW_Command_Apache_H
 	TW_Event *event;
 	unsigned short aen;
 	char host[16];
-	char *error_str;
+	const char *error_str;
 
 	tw_dev->aen_count++;
 
@@ -250,10 +268,9 @@ static void twl_aen_queue_event(TW_Device_Extension *tw_dev, TW_Command_Apache_H
 	tw_dev->error_sequence_id++;
 
 	/* Check for embedded error string */
-	error_str = &(header->err_specific_desc[strlen(header->err_specific_desc)+1]);
-
-	header->err_specific_desc[sizeof(header->err_specific_desc) - 1] = '\0';
-	event->parameter_len = strlen(header->err_specific_desc);
+	error_str = twl_error_string(header);
+	event->parameter_len = strnlen(header->err_specific_desc,
+				       sizeof(header->err_specific_desc));
 	memcpy(event->parameter_data, header->err_specific_desc, event->parameter_len + 1 + strlen(error_str));
 	if (event->severity != TW_AEN_SEVERITY_DEBUG)
 		printk(KERN_WARNING "3w-sas:%s AEN: %s (0x%02X:0x%04X): %s:%s.\n",
@@ -861,13 +878,13 @@ static int twl_fill_sense(TW_Device_Extension *tw_dev, int i, int request_id, in
 	TW_Command_Apache_Header *header;
 	TW_Command_Full *full_command_packet;
 	unsigned short error;
-	char *error_str;
+	const char *error_str;
 
 	header = tw_dev->sense_buffer_virt[i];
 	full_command_packet = tw_dev->command_packet_virt[request_id];
 
 	/* Get embedded firmware error string */
-	error_str = &(header->err_specific_desc[strlen(header->err_specific_desc) + 1]);
+	error_str = twl_error_string(header);
 
 	/* Don't print error for Logical unit not supported during rollcall */
 	error = le16_to_cpu(header->status_block.error);
-- 
2.43.0