* [PATCH v3 1/7] Revert "qla2xxx: Make sure that aborted commands are freed"
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
@ 2021-03-20 23:23 ` Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 2/7] qla2xxx: Constify struct qla_tgt_func_tmpl Bart Van Assche
` (8 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2021-03-20 23:23 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani
Calling vha->hw->tgt.tgt_ops->free_cmd() from qlt_xmit_response() is wrong
since the command for which a response is sent must remain valid until the
SCSI target core calls .release_cmd(). It has been observed that the
following scenario triggers a kernel crash:
- qlt_xmit_response() calls qlt_check_reserve_free_req().
- qlt_check_reserve_free_req() returns -EAGAIN.
- qlt_xmit_response() calls vha->hw->tgt.tgt_ops->free_cmd(cmd).
- transport_handle_queue_full() tries to retransmit the response.
Fix this crash by reverting the patch that introduced it.
Fixes: 0dcec41acb85 ("scsi: qla2xxx: Make sure that aborted commands are freed")
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_target.c | 13 +++++--------
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 ----
2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 03a045f4e1b1..af5e6f4cb1a0 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3222,8 +3222,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) ||
(cmd->sess && cmd->sess->deleted)) {
cmd->state = QLA_TGT_STATE_PROCESSED;
- res = 0;
- goto free;
+ return 0;
}
ql_dbg_qp(ql_dbg_tgt, qpair, 0xe018,
@@ -3234,8 +3233,9 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
res = qlt_pre_xmit_response(cmd, &prm, xmit_type, scsi_status,
&full_req_cnt);
- if (unlikely(res != 0))
- goto free;
+ if (unlikely(res != 0)) {
+ return res;
+ }
spin_lock_irqsave(qpair->qp_lock_ptr, flags);
@@ -3255,8 +3255,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
vha->flags.online, qla2x00_reset_active(vha),
cmd->reset_count, qpair->chip_reset);
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
- res = 0;
- goto free;
+ return 0;
}
/* Does F/W have an IOCBs for this request */
@@ -3359,8 +3358,6 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
qlt_unmap_sg(vha, cmd);
spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
-free:
- vha->hw->tgt.tgt_ops->free_cmd(cmd);
return res;
}
EXPORT_SYMBOL(qlt_xmit_response);
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 30959f8da065..15650a0bde09 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -653,7 +653,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
{
struct qla_tgt_cmd *cmd = container_of(se_cmd,
struct qla_tgt_cmd, se_cmd);
- struct scsi_qla_host *vha = cmd->vha;
if (cmd->aborted) {
/* Cmd can loop during Q-full. tcm_qla2xxx_aborted_task
@@ -666,7 +665,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
cmd->se_cmd.transport_state,
cmd->se_cmd.t_state,
cmd->se_cmd.se_cmd_flags);
- vha->hw->tgt.tgt_ops->free_cmd(cmd);
return 0;
}
@@ -694,7 +692,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
{
struct qla_tgt_cmd *cmd = container_of(se_cmd,
struct qla_tgt_cmd, se_cmd);
- struct scsi_qla_host *vha = cmd->vha;
int xmit_type = QLA_TGT_XMIT_STATUS;
if (cmd->aborted) {
@@ -708,7 +705,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
cmd, kref_read(&cmd->se_cmd.cmd_kref),
cmd->se_cmd.transport_state, cmd->se_cmd.t_state,
cmd->se_cmd.se_cmd_flags);
- vha->hw->tgt.tgt_ops->free_cmd(cmd);
return 0;
}
cmd->bufflen = se_cmd->data_length;
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 2/7] qla2xxx: Constify struct qla_tgt_func_tmpl
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 1/7] Revert "qla2xxx: Make sure that aborted commands are freed" Bart Van Assche
@ 2021-03-20 23:23 ` Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 3/7] qla2xxx: Fix endianness annotations Bart Van Assche
` (7 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2021-03-20 23:23 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani, Lee Duncan
Since the target function pointers are not modified at runtime, declare
the data structure with the target function pointers const.
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_def.h | 2 +-
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 49b42b430df4..3bdf55bb0833 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -3815,7 +3815,7 @@ struct qlt_hw_data {
__le32 __iomem *atio_q_in;
__le32 __iomem *atio_q_out;
- struct qla_tgt_func_tmpl *tgt_ops;
+ const struct qla_tgt_func_tmpl *tgt_ops;
struct qla_tgt_vp_map *tgt_vp_map;
int saved_set;
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 15650a0bde09..46111f031be9 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1578,7 +1578,7 @@ static void tcm_qla2xxx_update_sess(struct fc_port *sess, port_id_t s_id,
/*
* Calls into tcm_qla2xxx used by qla2xxx LLD I/O path.
*/
-static struct qla_tgt_func_tmpl tcm_qla2xxx_template = {
+static const struct qla_tgt_func_tmpl tcm_qla2xxx_template = {
.find_cmd_by_tag = tcm_qla2xxx_find_cmd_by_tag,
.handle_cmd = tcm_qla2xxx_handle_cmd,
.handle_data = tcm_qla2xxx_handle_data,
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 3/7] qla2xxx: Fix endianness annotations
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 1/7] Revert "qla2xxx: Make sure that aborted commands are freed" Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 2/7] qla2xxx: Constify struct qla_tgt_func_tmpl Bart Van Assche
@ 2021-03-20 23:23 ` Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 4/7] qla2xxx: Suppress Coverity complaints about dseg_r* Bart Van Assche
` (6 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2021-03-20 23:23 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani, Lee Duncan
Fix all recently introduced endianness annotation issues.
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_def.h | 2 +-
drivers/scsi/qla2xxx/qla_iocb.c | 3 ++-
drivers/scsi/qla2xxx/qla_isr.c | 2 +-
drivers/scsi/qla2xxx/qla_sup.c | 9 +++++----
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 3bdf55bb0833..52ba75591f9a 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -1527,7 +1527,7 @@ struct init_sf_cb {
* BIT_12 = Remote Write Optimization (1 - Enabled, 0 - Disabled)
* BIT 11-0 = Reserved
*/
- uint16_t flags;
+ __le16 flags;
uint8_t reserved1[32];
uint16_t discard_OHRB_timeout_value;
uint16_t remote_write_opt_queue_num;
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index e765ee4ce162..c89e6d0f3616 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2379,7 +2379,8 @@ qla24xx_prli_iocb(srb_t *sp, struct logio_entry_24xx *logio)
cpu_to_le32(NVME_PRLI_SP_FIRST_BURST);
if (sp->vha->flags.nvme2_enabled) {
/* Set service parameter BIT_7 for NVME CONF support */
- logio->io_parameter[0] |= NVME_PRLI_SP_CONF;
+ logio->io_parameter[0] |=
+ cpu_to_le32(NVME_PRLI_SP_CONF);
/* Set service parameter BIT_8 for SLER support */
logio->io_parameter[0] |=
cpu_to_le32(NVME_PRLI_SP_SLER);
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 27165abda96d..0fa7082f3cc8 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -3440,7 +3440,7 @@ qla24xx_abort_iocb_entry(scsi_qla_host_t *vha, struct req_que *req,
return;
abt = &sp->u.iocb_cmd;
- abt->u.abt.comp_status = le16_to_cpu(pkt->comp_status);
+ abt->u.abt.comp_status = pkt->comp_status;
orig_sp = sp->cmd_sp;
/* Need to pass original sp */
if (orig_sp)
diff --git a/drivers/scsi/qla2xxx/qla_sup.c b/drivers/scsi/qla2xxx/qla_sup.c
index f771fabcba59..060c89237777 100644
--- a/drivers/scsi/qla2xxx/qla_sup.c
+++ b/drivers/scsi/qla2xxx/qla_sup.c
@@ -2621,10 +2621,11 @@ qla24xx_read_optrom_data(struct scsi_qla_host *vha, void *buf,
}
static int
-qla28xx_extract_sfub_and_verify(struct scsi_qla_host *vha, uint32_t *buf,
+qla28xx_extract_sfub_and_verify(struct scsi_qla_host *vha, __le32 *buf,
uint32_t len, uint32_t buf_size_without_sfub, uint8_t *sfub_buf)
{
- uint32_t *p, check_sum = 0;
+ uint32_t check_sum = 0;
+ __le32 *p;
int i;
p = buf + buf_size_without_sfub;
@@ -2790,8 +2791,8 @@ qla28xx_write_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr,
goto done;
}
- rval = qla28xx_extract_sfub_and_verify(vha, dwptr, dwords,
- buf_size_without_sfub, (uint8_t *)sfub);
+ rval = qla28xx_extract_sfub_and_verify(vha, (__le32 *)dwptr,
+ dwords, buf_size_without_sfub, (uint8_t *)sfub);
if (rval != QLA_SUCCESS)
goto done;
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 4/7] qla2xxx: Suppress Coverity complaints about dseg_r*
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (2 preceding siblings ...)
2021-03-20 23:23 ` [PATCH v3 3/7] qla2xxx: Fix endianness annotations Bart Van Assche
@ 2021-03-20 23:23 ` Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 5/7] qla2xxx: Simplify qla8044_minidump_process_control() Bart Van Assche
` (5 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2021-03-20 23:23 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani, Lee Duncan
Change dseq_rq and dseg_rsp from scalar structure members into
single-element arrays such that Coverity does not complain about the
(*cur_dsd)++ statement in append_dsd64().
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_mr.c | 12 ++++++------
drivers/scsi/qla2xxx/qla_mr.h | 8 ++++++--
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index d488ae95e149..6e920da64863 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -3266,8 +3266,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
fx_iocb.req_xfrcnt =
cpu_to_le16(fxio->u.fxiocb.req_len);
put_unaligned_le64(fxio->u.fxiocb.req_dma_handle,
- &fx_iocb.dseg_rq.address);
- fx_iocb.dseg_rq.length =
+ &fx_iocb.dseg_rq[0].address);
+ fx_iocb.dseg_rq[0].length =
cpu_to_le32(fxio->u.fxiocb.req_len);
}
@@ -3276,8 +3276,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
fx_iocb.rsp_xfrcnt =
cpu_to_le16(fxio->u.fxiocb.rsp_len);
put_unaligned_le64(fxio->u.fxiocb.rsp_dma_handle,
- &fx_iocb.dseg_rsp.address);
- fx_iocb.dseg_rsp.length =
+ &fx_iocb.dseg_rsp[0].address);
+ fx_iocb.dseg_rsp[0].length =
cpu_to_le32(fxio->u.fxiocb.rsp_len);
}
@@ -3314,7 +3314,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
cpu_to_le16(bsg_job->request_payload.sg_cnt);
tot_dsds =
bsg_job->request_payload.sg_cnt;
- cur_dsd = &fx_iocb.dseg_rq;
+ cur_dsd = &fx_iocb.dseg_rq[0];
avail_dsds = 1;
for_each_sg(bsg_job->request_payload.sg_list, sg,
tot_dsds, index) {
@@ -3369,7 +3369,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)
fx_iocb.rsp_dsdcnt =
cpu_to_le16(bsg_job->reply_payload.sg_cnt);
tot_dsds = bsg_job->reply_payload.sg_cnt;
- cur_dsd = &fx_iocb.dseg_rsp;
+ cur_dsd = &fx_iocb.dseg_rsp[0];
avail_dsds = 1;
for_each_sg(bsg_job->reply_payload.sg_list, sg,
diff --git a/drivers/scsi/qla2xxx/qla_mr.h b/drivers/scsi/qla2xxx/qla_mr.h
index 73be8348402a..4f63aff333db 100644
--- a/drivers/scsi/qla2xxx/qla_mr.h
+++ b/drivers/scsi/qla2xxx/qla_mr.h
@@ -176,8 +176,12 @@ struct fxdisc_entry_fx00 {
uint8_t flags;
uint8_t reserved_1;
- struct dsd64 dseg_rq;
- struct dsd64 dseg_rsp;
+ /*
+ * Use array size 1 below to prevent that Coverity complains about
+ * the append_dsd64() calls for the two arrays below.
+ */
+ struct dsd64 dseg_rq[1];
+ struct dsd64 dseg_rsp[1];
__le32 dataword;
__le32 adapid;
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 5/7] qla2xxx: Simplify qla8044_minidump_process_control()
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (3 preceding siblings ...)
2021-03-20 23:23 ` [PATCH v3 4/7] qla2xxx: Suppress Coverity complaints about dseg_r* Bart Van Assche
@ 2021-03-20 23:23 ` Bart Van Assche
2021-03-20 23:23 ` [PATCH v3 6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2021-03-20 23:23 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Daniel Wagner, Himanshu Madhani, Lee Duncan
This patch fixes the following Coverity complaint:
CID 177490 (#1 of 1): Unused value (UNUSED_VALUE)
assigned_value: Assigning value from opcode & 0xffffff7fU to opcode
here, but that stored value is overwritten before it can be used.
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_nx2.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_nx2.c b/drivers/scsi/qla2xxx/qla_nx2.c
index 7c413f93d53e..5ceecc9642fc 100644
--- a/drivers/scsi/qla2xxx/qla_nx2.c
+++ b/drivers/scsi/qla2xxx/qla_nx2.c
@@ -2226,19 +2226,16 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
if (opcode & QLA82XX_DBG_OPCODE_WR) {
qla8044_wr_reg_indirect(vha, crb_addr,
crb_entry->value_1);
- opcode &= ~QLA82XX_DBG_OPCODE_WR;
}
if (opcode & QLA82XX_DBG_OPCODE_RW) {
qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
qla8044_wr_reg_indirect(vha, crb_addr, read_value);
- opcode &= ~QLA82XX_DBG_OPCODE_RW;
}
if (opcode & QLA82XX_DBG_OPCODE_AND) {
qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
read_value &= crb_entry->value_2;
- opcode &= ~QLA82XX_DBG_OPCODE_AND;
if (opcode & QLA82XX_DBG_OPCODE_OR) {
read_value |= crb_entry->value_3;
opcode &= ~QLA82XX_DBG_OPCODE_OR;
@@ -2249,7 +2246,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
qla8044_rd_reg_indirect(vha, crb_addr, &read_value);
read_value |= crb_entry->value_3;
qla8044_wr_reg_indirect(vha, crb_addr, read_value);
- opcode &= ~QLA82XX_DBG_OPCODE_OR;
}
if (opcode & QLA82XX_DBG_OPCODE_POLL) {
poll_time = crb_entry->crb_strd.poll_timeout;
@@ -2269,7 +2265,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
crb_addr, &read_value);
}
} while (1);
- opcode &= ~QLA82XX_DBG_OPCODE_POLL;
}
if (opcode & QLA82XX_DBG_OPCODE_RDSTATE) {
@@ -2283,7 +2278,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
qla8044_rd_reg_indirect(vha, addr, &read_value);
index = crb_entry->crb_ctrl.state_index_v;
tmplt_hdr->saved_state_array[index] = read_value;
- opcode &= ~QLA82XX_DBG_OPCODE_RDSTATE;
}
if (opcode & QLA82XX_DBG_OPCODE_WRSTATE) {
@@ -2303,7 +2297,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
}
qla8044_wr_reg_indirect(vha, addr, read_value);
- opcode &= ~QLA82XX_DBG_OPCODE_WRSTATE;
}
if (opcode & QLA82XX_DBG_OPCODE_MDSTATE) {
@@ -2316,7 +2309,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,
read_value |= crb_entry->value_3;
read_value += crb_entry->value_1;
tmplt_hdr->saved_state_array[index] = read_value;
- opcode &= ~QLA82XX_DBG_OPCODE_MDSTATE;
}
crb_addr += crb_entry->crb_strd.addr_stride;
}
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (4 preceding siblings ...)
2021-03-20 23:23 ` [PATCH v3 5/7] qla2xxx: Simplify qla8044_minidump_process_control() Bart Van Assche
@ 2021-03-20 23:23 ` Bart Van Assche
2021-03-21 12:03 ` Daniel Wagner
2021-03-22 15:46 ` Himanshu Madhani
2021-03-20 23:23 ` [PATCH v3 7/7] qla2xxx: Check kzalloc() return value Bart Van Assche
` (3 subsequent siblings)
9 siblings, 2 replies; 16+ messages in thread
From: Bart Van Assche @ 2021-03-20 23:23 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Quinn Tran, Mike Christie,
Himanshu Madhani, Daniel Wagner, Lee Duncan
This patch fixes the following Coverity warning:
CID 361199 (#1 of 1): Unchecked return value (CHECKED_RETURN)
3. check_return: Calling qla24xx_get_isp_stats without checking return
value (as is done elsewhere 4 out of 5 times).
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Daniel Wagner <dwagner@suse.de>
Cc: Lee Duncan <lduncan@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_attr.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 63391c9be05d..3aa9869f6fae 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -2864,6 +2864,8 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
vha->qla_stats.jiffies_at_last_reset = get_jiffies_64();
if (IS_FWI2_CAPABLE(ha)) {
+ int rval;
+
stats = dma_alloc_coherent(&ha->pdev->dev,
sizeof(*stats), &stats_dma, GFP_KERNEL);
if (!stats) {
@@ -2873,7 +2875,11 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
}
/* reset firmware statistics */
- qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
+ rval = qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
+ if (rval != QLA_SUCCESS)
+ ql_log(ql_log_warn, vha, 0x70de,
+ "Resetting ISP statistics failed: rval = %d\n",
+ rval);
dma_free_coherent(&ha->pdev->dev, sizeof(*stats),
stats, stats_dma);
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
2021-03-20 23:23 ` [PATCH v3 6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
@ 2021-03-21 12:03 ` Daniel Wagner
2021-03-22 15:46 ` Himanshu Madhani
1 sibling, 0 replies; 16+ messages in thread
From: Daniel Wagner @ 2021-03-21 12:03 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
Quinn Tran, Mike Christie, Himanshu Madhani, Lee Duncan
On Sat, Mar 20, 2021 at 04:23:58PM -0700, Bart Van Assche wrote:
> This patch fixes the following Coverity warning:
>
> CID 361199 (#1 of 1): Unchecked return value (CHECKED_RETURN)
> 3. check_return: Calling qla24xx_get_isp_stats without checking return
> value (as is done elsewhere 4 out of 5 times).
>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Lee Duncan <lduncan@suse.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
2021-03-20 23:23 ` [PATCH v3 6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
2021-03-21 12:03 ` Daniel Wagner
@ 2021-03-22 15:46 ` Himanshu Madhani
1 sibling, 0 replies; 16+ messages in thread
From: Himanshu Madhani @ 2021-03-22 15:46 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin Petersen, James E . J . Bottomley, linux-scsi, Quinn Tran,
Michael Christie, Daniel Wagner, Lee Duncan
> On Mar 20, 2021, at 6:23 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> This patch fixes the following Coverity warning:
>
> CID 361199 (#1 of 1): Unchecked return value (CHECKED_RETURN)
> 3. check_return: Calling qla24xx_get_isp_stats without checking return
> value (as is done elsewhere 4 out of 5 times).
>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Lee Duncan <lduncan@suse.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_attr.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
> index 63391c9be05d..3aa9869f6fae 100644
> --- a/drivers/scsi/qla2xxx/qla_attr.c
> +++ b/drivers/scsi/qla2xxx/qla_attr.c
> @@ -2864,6 +2864,8 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
> vha->qla_stats.jiffies_at_last_reset = get_jiffies_64();
>
> if (IS_FWI2_CAPABLE(ha)) {
> + int rval;
> +
> stats = dma_alloc_coherent(&ha->pdev->dev,
> sizeof(*stats), &stats_dma, GFP_KERNEL);
> if (!stats) {
> @@ -2873,7 +2875,11 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost)
> }
>
> /* reset firmware statistics */
> - qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
> + rval = qla24xx_get_isp_stats(base_vha, stats, stats_dma, BIT_0);
> + if (rval != QLA_SUCCESS)
> + ql_log(ql_log_warn, vha, 0x70de,
> + "Resetting ISP statistics failed: rval = %d\n",
> + rval);
>
> dma_free_coherent(&ha->pdev->dev, sizeof(*stats),
> stats, stats_dma);
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
--
Himanshu Madhani Oracle Linux Engineering
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 7/7] qla2xxx: Check kzalloc() return value
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (5 preceding siblings ...)
2021-03-20 23:23 ` [PATCH v3 6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats() Bart Van Assche
@ 2021-03-20 23:23 ` Bart Van Assche
2021-03-21 12:10 ` Daniel Wagner
` (2 more replies)
2021-03-25 1:48 ` [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Martin K. Petersen
` (2 subsequent siblings)
9 siblings, 3 replies; 16+ messages in thread
From: Bart Van Assche @ 2021-03-20 23:23 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Himanshu Madhani, Saurav Kashyap,
Nilesh Javali, Quinn Tran, Mike Christie, Daniel Wagner,
Lee Duncan
Instead of crashing if kzalloc() fails, make qla2x00_get_host_stats()
return -ENOMEM.
Fixes: dbf1f53cfd23 ("scsi: qla2xxx: Implementation to get and manage host, target stats and initiator port")
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Saurav Kashyap <skashyap@marvell.com>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Mike Christie <michael.christie@oracle.com>
Cc: Daniel Wagner <dwagner@suse.de>
Cc: Lee Duncan <lduncan@suse.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_bsg.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index bee8cf9f8123..bc84b2f389f8 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -2583,6 +2583,10 @@ qla2x00_get_host_stats(struct bsg_job *bsg_job)
}
data = kzalloc(response_len, GFP_KERNEL);
+ if (!data) {
+ kfree(req_data);
+ return -ENOMEM;
+ }
ret = qla2xxx_get_ini_stats(fc_bsg_to_shost(bsg_job), req_data->stat_type,
data, response_len);
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 7/7] qla2xxx: Check kzalloc() return value
2021-03-20 23:23 ` [PATCH v3 7/7] qla2xxx: Check kzalloc() return value Bart Van Assche
@ 2021-03-21 12:10 ` Daniel Wagner
2021-03-22 5:04 ` [EXT] " Saurav Kashyap
2021-03-22 15:47 ` Himanshu Madhani
2 siblings, 0 replies; 16+ messages in thread
From: Daniel Wagner @ 2021-03-21 12:10 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
Himanshu Madhani, Saurav Kashyap, Nilesh Javali, Quinn Tran,
Mike Christie, Lee Duncan
On Sat, Mar 20, 2021 at 04:23:59PM -0700, Bart Van Assche wrote:
> data = kzalloc(response_len, GFP_KERNEL);
> + if (!data) {
> + kfree(req_data);
> + return -ENOMEM;
> + }
There is the host_stat_out label which could be reused for the exit
path. I am not sure if this the preferred solution by the
maintainers.
Reviewed-by: Daniel Wagner <dwagner@suse.de>
^ permalink raw reply [flat|nested] 16+ messages in thread* RE: [EXT] [PATCH v3 7/7] qla2xxx: Check kzalloc() return value
2021-03-20 23:23 ` [PATCH v3 7/7] qla2xxx: Check kzalloc() return value Bart Van Assche
2021-03-21 12:10 ` Daniel Wagner
@ 2021-03-22 5:04 ` Saurav Kashyap
2021-03-22 15:47 ` Himanshu Madhani
2 siblings, 0 replies; 16+ messages in thread
From: Saurav Kashyap @ 2021-03-22 5:04 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Himanshu Madhani, Nilesh Javali, Quinn Tran,
Mike Christie, Daniel Wagner, Lee Duncan
Hi Bart,
> -----Original Message-----
> From: Bart Van Assche <bvanassche@acm.org>
> Sent: Sunday, March 21, 2021 4:54 AM
> To: Martin K . Petersen <martin.petersen@oracle.com>; James E . J .
> Bottomley <jejb@linux.vnet.ibm.com>
> Cc: linux-scsi@vger.kernel.org; Bart Van Assche <bvanassche@acm.org>;
> Himanshu Madhani <himanshu.madhani@oracle.com>; Saurav Kashyap
> <skashyap@marvell.com>; Nilesh Javali <njavali@marvell.com>; Quinn Tran
> <qutran@marvell.com>; Mike Christie <michael.christie@oracle.com>;
> Daniel Wagner <dwagner@suse.de>; Lee Duncan <lduncan@suse.com>
> Subject: [EXT] [PATCH v3 7/7] qla2xxx: Check kzalloc() return value
>
> External Email
>
> ----------------------------------------------------------------------
> Instead of crashing if kzalloc() fails, make qla2x00_get_host_stats()
> return -ENOMEM.
>
> Fixes: dbf1f53cfd23 ("scsi: qla2xxx: Implementation to get and manage host,
> target stats and initiator port")
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Saurav Kashyap <skashyap@marvell.com>
> Cc: Nilesh Javali <njavali@marvell.com>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Lee Duncan <lduncan@suse.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_bsg.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index bee8cf9f8123..bc84b2f389f8 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -2583,6 +2583,10 @@ qla2x00_get_host_stats(struct bsg_job *bsg_job)
> }
>
> data = kzalloc(response_len, GFP_KERNEL);
> + if (!data) {
> + kfree(req_data);
> + return -ENOMEM;
> + }
>
> ret = qla2xxx_get_ini_stats(fc_bsg_to_shost(bsg_job), req_data-
> >stat_type,
> data, response_len);
Thanks for the patch.
Acked-by: Saurav Kashyap <skashyap@marvell.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 7/7] qla2xxx: Check kzalloc() return value
2021-03-20 23:23 ` [PATCH v3 7/7] qla2xxx: Check kzalloc() return value Bart Van Assche
2021-03-21 12:10 ` Daniel Wagner
2021-03-22 5:04 ` [EXT] " Saurav Kashyap
@ 2021-03-22 15:47 ` Himanshu Madhani
2 siblings, 0 replies; 16+ messages in thread
From: Himanshu Madhani @ 2021-03-22 15:47 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin Petersen, James E . J . Bottomley, linux-scsi,
Saurav Kashyap, Nilesh Javali, Quinn Tran, Michael Christie,
Daniel Wagner, Lee Duncan
> On Mar 20, 2021, at 6:23 PM, Bart Van Assche <bvanassche@acm.org> wrote:
>
> Instead of crashing if kzalloc() fails, make qla2x00_get_host_stats()
> return -ENOMEM.
>
> Fixes: dbf1f53cfd23 ("scsi: qla2xxx: Implementation to get and manage host, target stats and initiator port")
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Saurav Kashyap <skashyap@marvell.com>
> Cc: Nilesh Javali <njavali@marvell.com>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Lee Duncan <lduncan@suse.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/qla2xxx/qla_bsg.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index bee8cf9f8123..bc84b2f389f8 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -2583,6 +2583,10 @@ qla2x00_get_host_stats(struct bsg_job *bsg_job)
> }
>
> data = kzalloc(response_len, GFP_KERNEL);
> + if (!data) {
> + kfree(req_data);
> + return -ENOMEM;
> + }
>
> ret = qla2xxx_get_ini_stats(fc_bsg_to_shost(bsg_job), req_data->stat_type,
> data, response_len);
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
--
Himanshu Madhani Oracle Linux Engineering
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (6 preceding siblings ...)
2021-03-20 23:23 ` [PATCH v3 7/7] qla2xxx: Check kzalloc() return value Bart Van Assche
@ 2021-03-25 1:48 ` Martin K. Petersen
2021-03-25 3:53 ` Martin K. Petersen
2021-03-30 3:54 ` Martin K. Petersen
9 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2021-03-25 1:48 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi
Bart,
> Please consider the first patch in this series for kernel v5.12 and
> the remaining patches for kernel v5.13.
Applied patches 2 through 7 to 5.13/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (7 preceding siblings ...)
2021-03-25 1:48 ` [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Martin K. Petersen
@ 2021-03-25 3:53 ` Martin K. Petersen
2021-03-30 3:54 ` Martin K. Petersen
9 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2021-03-25 3:53 UTC (permalink / raw)
To: James E . J . Bottomley, Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi
On Sat, 20 Mar 2021 16:23:52 -0700, Bart Van Assche wrote:
> Please consider the first patch in this series for kernel v5.12 and the
> remaining patches for kernel v5.13.
>
> Thanks,
>
> Bart.
>
> [...]
Applied to 5.12/scsi-fixes, thanks!
[1/7] Revert "qla2xxx: Make sure that aborted commands are freed"
https://git.kernel.org/mkp/scsi/c/39c0c8553bfb
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13
2021-03-20 23:23 [PATCH v3 0/7] qla2xxx patches for kernel v5.12 and v5.13 Bart Van Assche
` (8 preceding siblings ...)
2021-03-25 3:53 ` Martin K. Petersen
@ 2021-03-30 3:54 ` Martin K. Petersen
9 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2021-03-30 3:54 UTC (permalink / raw)
To: James E . J . Bottomley, Bart Van Assche; +Cc: Martin K . Petersen, linux-scsi
On Sat, 20 Mar 2021 16:23:52 -0700, Bart Van Assche wrote:
> Please consider the first patch in this series for kernel v5.12 and the
> remaining patches for kernel v5.13.
>
> Thanks,
>
> Bart.
>
> [...]
Applied to 5.13/scsi-queue, thanks!
[2/7] qla2xxx: Constify struct qla_tgt_func_tmpl
https://git.kernel.org/mkp/scsi/c/634b9774d335
[3/7] qla2xxx: Fix endianness annotations
https://git.kernel.org/mkp/scsi/c/37ce4f3531d4
[4/7] qla2xxx: Suppress Coverity complaints about dseg_r*
https://git.kernel.org/mkp/scsi/c/17603237f789
[5/7] qla2xxx: Simplify qla8044_minidump_process_control()
https://git.kernel.org/mkp/scsi/c/a20821e3f471
[6/7] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
https://git.kernel.org/mkp/scsi/c/a2b2cc660822
[7/7] qla2xxx: Check kzalloc() return value
https://git.kernel.org/mkp/scsi/c/e5406d8ad4a1
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 16+ messages in thread