On Tue, 2012-03-13 at 20:20 +0300, Dan Carpenter wrote:
> Hello Nicholas,
>
> The patch 962f988d031f: "qla2xxx: Add LLD target-mode infrastructure
> for >= 24xx series" from Mar 8, 2012, leads to the following Smatch
> warning:
>
> drivers/scsi/qla2xxx/qla_target.c:3207 qla_tgt_handle_srr_work()
> error: potential null derefence 'vha'.
>
>
> drivers/scsi/qla2xxx/qla_target.c
> 3198 imm = NULL;
> 3199 list_for_each_entry_safe(i, ti, &tgt->srr_imm_list,
> 3200 srr_list_entry) {
> 3201 if (i->srr_id == sctio->srr_id) {
> 3202 list_del(&i->srr_list_entry);
> 3203 if (imm) {
> 3204 printk(KERN_ERR "qla_target(%d): There must "
> 3205 "be only one IMM SRR per CTIO SRR "
> 3206 "(IMM SRR %p, id %d, CTIO %p\n",
> 3207 vha->vp_idx, i, i->srr_id, sctio);
> ^^^
> It looks like "vha" might not be initialized yet.
>
> 3208 qla_tgt_reject_free_srr_imm(vha, i, 0);
> 3209 } else
> 3210 imm = i;
> 3211 }
> 3212 }
> 3213
> 3214 ql_dbg(ql_dbg_tgt_mgt, tgt->vha, 0xe12f, "IMM SRR %p, CTIO SRR %p (id %d)\n",
> 3215 imm, sctio, sctio->srr_id);
> 3216
> 3217 if (imm == NULL) {
> 3218 ql_dbg(ql_dbg_tgt_mgt, tgt->vha, 0xe130, "Not found matching IMM"
> 3219 " for SRR CTIO (id %d)\n", sctio->srr_id);
> 3220 continue;
> 3221 } else
> 3222 list_del(&sctio->srr_list_entry);
> 3223
> 3224 spin_unlock_irqrestore(&tgt->srr_lock, flags);
> 3225
> 3226 cmd = sctio->cmd;
> 3227 vha = cmd->vha;
> ^^^
> It gets set here.
>
It looks like this should be using qla_tgt->vha everywhere. Applying
the following patch.
--nab
iff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index e415d1e..2af075b 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3169,11 +3169,11 @@ static void qla_tgt_reject_free_srr_imm(struct scsi_qla_host *vha, struct qla_tg
static void qla_tgt_handle_srr_work(struct work_struct *work)
{
struct qla_tgt *tgt = container_of(work, struct qla_tgt, srr_work);
- struct scsi_qla_host *vha = NULL;
+ struct scsi_qla_host *vha = tgt->vha;
struct qla_tgt_srr_ctio *sctio;
unsigned long flags;
- ql_dbg(ql_dbg_tgt_mgt, tgt->vha, 0xe12e, "Entering SRR work (tgt %p)\n", tgt);
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe12e, "Entering SRR work (tgt %p)\n", tgt);
restart:
spin_lock_irqsave(&tgt->srr_lock, flags);
@@ -3192,17 +3192,17 @@ restart:
"be only one IMM SRR per CTIO SRR "
"(IMM SRR %p, id %d, CTIO %p\n",
vha->vp_idx, i, i->srr_id, sctio);
- qla_tgt_reject_free_srr_imm(vha, i, 0);
+ qla_tgt_reject_free_srr_imm(tgt->vha, i, 0);
} else
imm = i;
}
}
- ql_dbg(ql_dbg_tgt_mgt, tgt->vha, 0xe12f, "IMM SRR %p, CTIO SRR %p (id %d)\n",
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe12f, "IMM SRR %p, CTIO SRR %p (id %d)\n",
imm, sctio, sctio->srr_id);
if (imm == NULL) {
- ql_dbg(ql_dbg_tgt_mgt, tgt->vha, 0xe130, "Not found matching IMM"
+ ql_dbg(ql_dbg_tgt_mgt, vha, 0xe130, "Not found matching IMM"
" for SRR CTIO (id %d)\n", sctio->srr_id);
continue;
} else
@@ -3211,7 +3211,6 @@ restart:
spin_unlock_irqrestore(&tgt->srr_lock, flags);
cmd = sctio->cmd;
- vha = cmd->vha;
/*
* Reset qla_tgt_cmd SRR values and SGL pointer+count to follow
* tcm_qla2xxx_write_pending() and tcm_qla2xxx_queue_data_in()
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
