|
From: Tony B. <to...@cy...> - 2025-09-02 17:18:02
|
SCST_GET_CDB_LEN() returns 0 for vendor-specific opcodes such as:
0xD1 READ DYN RUNTIME ATT
0xD2 WRITE DYN RUNTIME ATTR
0xE7 INIT ELEMENT STATUS WRANGE
This causes scst_set_cmd_from_cdb_info() to check cdb[-1] for the
control byte, causing an out-of-bounds array read.
- Move the parsing of the control byte after get_cdb_info() since that
may set the CDB length to a known value.
- If the CDB length is still unknown, then assume the control byte is
0 without accessing the CDB.
- Check for variable-length CDBs in scst_set_cmd_from_cdb_info() rather
than using the wrong control byte and then overriding it in
get_cdb_info_var_len(). This is necessary because the override would
no longer work after the change above.
Also, the following code doesn't work:
#define CONTROL_BYTE_NACA_BIT 0x04
unsigned int cmd_naca:1;
cmd_naca = (control & CONTROL_BYTE_NACA_BIT);
The result will always be 0. Use this instead:
cmd_naca = !!(control & CONTROL_BYTE_NACA_BIT);
(cmd_linked happened to work because CONTROL_BYTE_LINK_BIT is 0x01, but
apply the same fix there also for consistency).
Signed-off-by: Tony Battersby <to...@cy...>
---
scst/src/scst_lib.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c
index 7eda3aa1e..6b480301a 100644
--- a/scst/src/scst_lib.c
+++ b/scst/src/scst_lib.c
@@ -11941,8 +11941,6 @@ static int scst_set_cmd_from_cdb_info(struct scst_cmd *cmd,
int res;
cmd->cdb_len = SCST_GET_CDB_LEN(cmd->cdb[0]);
- cmd->cmd_naca = (cmd->cdb[cmd->cdb_len - 1] & CONTROL_BYTE_NACA_BIT);
- cmd->cmd_linked = (cmd->cdb[cmd->cdb_len - 1] & CONTROL_BYTE_LINK_BIT);
cmd->op_name = ptr->info_op_name;
cmd->data_direction = ptr->info_data_direction;
cmd->op_flags = ptr->info_op_flags | SCST_INFO_VALID;
@@ -11952,6 +11950,15 @@ static int scst_set_cmd_from_cdb_info(struct scst_cmd *cmd,
cmd->len_len = ptr->info_len_len;
cmd->log2_max_buf_len = ptr->log2_max_buf_len;
res = (*ptr->get_cdb_info)(cmd, ptr);
+
+ /* get_cdb_info() may override cdb_len, so do this after */
+ if (likely(cmd->cdb_len != 0)) {
+ unsigned int control = (cmd->cdb[0] == 0x7F) ? cmd->cdb[1] :
+ cmd->cdb[cmd->cdb_len - 1];
+ cmd->cmd_naca = !!(control & CONTROL_BYTE_NACA_BIT);
+ cmd->cmd_linked = !!(control & CONTROL_BYTE_LINK_BIT);
+ }
+
if (!cmd->log2_max_buf_len ||
cmd->bufflen <= (1U << cmd->log2_max_buf_len))
return res;
@@ -12047,9 +12054,6 @@ static int get_cdb_info_var_len(struct scst_cmd *cmd,
res = scst_set_cmd_from_cdb_info(cmd, ptr);
- cmd->cmd_naca = (cmd->cdb[1] & CONTROL_BYTE_NACA_BIT);
- cmd->cmd_linked = (cmd->cdb[1] & CONTROL_BYTE_LINK_BIT);
-
out:
TRACE_EXIT_RES(res);
return res;
--
2.43.0
|