Menu
▾
▴
[Scst-devel] [PATCH 15/15] scsi: qla2xxx: improve safety of cmd lookup by handle
|
From: Tony B. <to...@cy...> - 2025-09-08 19:13:36
|
(target mode)
The driver associates two different structs with numeric handles and
passes the handles to the hardware. When the hardware passes the
handle back to the driver, the driver consults a table of void * to
convert the handle back to the struct without checking the type of
struct. This can lead to treating one type of struct as a different
type if the HBA firmware misbehaves (and some firmware versions do). So
verify the type of struct is what is expected before using it.
But we can also do better than that. Also verify that the exchange
address of the message sent from the hardware matches the exchange
address of the command being returned. This adds an extra guard against
buggy HBA firmware that returns duplicate messages multiple times
(which has also been seen) in case the driver has reused the handle for
a different command of the same type.
These problems were seen on a QLE2694L with firmware 9.08.02 when
testing SLER / SRR support. The SRR caused the HBA to flood the
response queue with hundreds of bogus entries.
Signed-off-by: Tony Battersby <to...@cy...>
---
drivers/scsi/qla2xxx/qla_dbg.c | 2 +-
drivers/scsi/qla2xxx/qla_target.c | 115 ++++++++++++++++++++++++------
2 files changed, 96 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
index 9f56bec26231..a7e3ec9bba47 100644
--- a/drivers/scsi/qla2xxx/qla_dbg.c
+++ b/drivers/scsi/qla2xxx/qla_dbg.c
@@ -54,7 +54,7 @@
* | Misc | 0xd303 | 0xd031-0xd0ff |
* | | | 0xd101-0xd1fe |
* | | | 0xd214-0xd2fe |
- * | Target Mode | 0xe086 | |
+ * | Target Mode | 0xe089 | |
* | Target Mode Management | 0xf09b | 0xf002 |
* | | | 0xf046-0xf049 |
* | Target Mode Task Management | 0x1000d | |
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 70e1810d4996..a2f880ceb8d6 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3993,7 +3993,8 @@ static int qlt_prepare_srr_ctio(struct qla_qpair *qpair,
/* ha->hardware_lock supposed to be held on entry */
static void *qlt_ctio_to_cmd(struct scsi_qla_host *vha,
- struct rsp_que *rsp, uint32_t handle, void *ctio)
+ struct rsp_que *rsp, uint32_t handle, uint8_t cmd_type,
+ const void *ctio)
{
void *cmd = NULL;
struct req_que *req;
@@ -4016,29 +4017,103 @@ static void *qlt_ctio_to_cmd(struct scsi_qla_host *vha,
h &= QLA_CMD_HANDLE_MASK;
- if (h != QLA_TGT_NULL_HANDLE) {
- if (unlikely(h >= req->num_outstanding_cmds)) {
- ql_dbg(ql_dbg_tgt, vha, 0xe052,
- "qla_target(%d): Wrong handle %x received\n",
- vha->vp_idx, handle);
- return NULL;
- }
-
- cmd = req->outstanding_cmds[h];
- if (unlikely(cmd == NULL)) {
- ql_dbg(ql_dbg_async, vha, 0xe053,
- "qla_target(%d): Suspicious: unable to find the command with handle %x req->id %d rsp->id %d\n",
- vha->vp_idx, handle, req->id, rsp->id);
- return NULL;
- }
- req->outstanding_cmds[h] = NULL;
- } else if (ctio != NULL) {
+ if (h == QLA_TGT_NULL_HANDLE) {
/* We can't get loop ID from CTIO7 */
ql_dbg(ql_dbg_tgt, vha, 0xe054,
"qla_target(%d): Wrong CTIO received: QLA24xx doesn't "
"support NULL handles\n", vha->vp_idx);
return NULL;
}
+ if (unlikely(h >= req->num_outstanding_cmds)) {
+ ql_dbg(ql_dbg_tgt, vha, 0xe052,
+ "qla_target(%d): Wrong handle %x received\n",
+ vha->vp_idx, handle);
+ return NULL;
+ }
+
+ cmd = req->outstanding_cmds[h];
+
+ /*
+ * Some places in the code set outstanding_cmds[h] to NULL and force
+ * the command to complete early without waiting for the CTIO. In that
+ * case we would get cmd == NULL. But in the rare case that the
+ * detached handle is reused for a different command before we get the
+ * CTIO for the detached cmd, we might find the wrong command here
+ * instead. So compare the cmd_type and exchange_address to make sure
+ * that the cmd matches the CTIO. If they don't match, return NULL and
+ * ignore the CTIO.
+ *
+ * This also guards against buggy HBA firmware that returns the same
+ * CTIO multiple times.
+ */
+
+ if (unlikely(cmd == NULL)) {
+ if (cmd_type == TYPE_TGT_CMD) {
+ __le32 ctio_exchange_addr =
+ ((const struct ctio7_from_24xx *)ctio)->
+ exchange_address;
+
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe053,
+ "qla_target(%d): tag %u: handle %x: cmd detached; ignoring CTIO (handle %x req->id %d rsp->id %d)\n",
+ vha->vp_idx, le32_to_cpu(ctio_exchange_addr), h,
+ handle, req->id, rsp->id);
+ } else {
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe053,
+ "qla_target(%d): cmd detached; ignoring CTIO (handle %x req->id %d rsp->id %d)\n",
+ vha->vp_idx, handle, req->id, rsp->id);
+ }
+ return NULL;
+ }
+
+ if (unlikely(((srb_t *)cmd)->cmd_type != cmd_type)) {
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe087,
+ "qla_target(%d): handle %x: cmd detached; ignoring CTIO (cmd_type mismatch)\n",
+ vha->vp_idx, h);
+ return NULL;
+ }
+
+ switch (cmd_type) {
+ case TYPE_TGT_CMD: {
+ __le32 ctio_exchange_addr =
+ ((const struct ctio7_from_24xx *)ctio)->
+ exchange_address;
+ __le32 cmd_exchange_addr =
+ ((struct qla_tgt_cmd *)cmd)->
+ atio.u.isp24.exchange_addr;
+
+ BUILD_BUG_ON(offsetof(struct ctio7_from_24xx,
+ exchange_address) !=
+ offsetof(struct ctio_crc_from_fw,
+ exchange_address));
+
+ if (unlikely(ctio_exchange_addr != cmd_exchange_addr)) {
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe088,
+ "qla_target(%d): tag %u: handle %x: cmd detached; ignoring CTIO (exchange address mismatch)\n",
+ vha->vp_idx, le32_to_cpu(ctio_exchange_addr), h);
+ return NULL;
+ }
+ break;
+ }
+
+ case TYPE_TGT_TMCMD: {
+ __le32 ctio_exchange_addr =
+ ((const struct abts_resp_from_24xx_fw *)ctio)->
+ exchange_address;
+ __le32 cmd_exchange_addr =
+ ((struct qla_tgt_mgmt_cmd *)cmd)->
+ orig_iocb.abts.exchange_address;
+
+ if (unlikely(ctio_exchange_addr != cmd_exchange_addr)) {
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe089,
+ "qla_target(%d): ABTS: handle %x: cmd detached; ignoring CTIO (exchange address mismatch)\n",
+ vha->vp_idx, h);
+ return NULL;
+ }
+ break;
+ }
+ }
+
+ req->outstanding_cmds[h] = NULL;
return cmd;
}
@@ -4067,7 +4142,7 @@ static void qlt_do_ctio_completion(struct scsi_qla_host *vha,
ctio_flags = le16_to_cpu(ctio->flags);
- cmd = qlt_ctio_to_cmd(vha, rsp, handle, ctio);
+ cmd = qlt_ctio_to_cmd(vha, rsp, handle, TYPE_TGT_CMD, ctio);
if (unlikely(cmd == NULL)) {
if ((handle & ~QLA_TGT_HANDLE_MASK) == QLA_TGT_SKIP_HANDLE &&
(ctio_flags & 0xe1ff) == (CTIO7_FLAGS_STATUS_MODE_1 |
@@ -6817,7 +6892,7 @@ static void qlt_handle_abts_completion(struct scsi_qla_host *vha,
struct qla_tgt_mgmt_cmd *mcmd;
struct qla_hw_data *ha = vha->hw;
- mcmd = qlt_ctio_to_cmd(vha, rsp, pkt->handle, pkt);
+ mcmd = qlt_ctio_to_cmd(vha, rsp, pkt->handle, TYPE_TGT_TMCMD, pkt);
if (mcmd == NULL && h != QLA_TGT_SKIP_HANDLE) {
ql_dbg(ql_dbg_async, vha, 0xe064,
"qla_target(%d): ABTS Comp without mcmd\n",
--
2.43.0
|
