From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-driver@qlogic.com, linux-scsi@vger.kernel.org,
target-devel <target-devel@vger.kernel.org>,
Andrew Vasquez <andrew.vasquez@qlogic.com>,
Giridhar Malavali <giridhar.malavali@qlogic.com>
Subject: re: qla2xxx: Add LLD target-mode infrastructure for >= 24xx series
Date: Thu, 15 Mar 2012 16:10:15 -0700 [thread overview]
Message-ID: <1331853015.10203.128.camel@haakon2.linux-iscsi.org> (raw)
In-Reply-To: <20120313172044.GD31808@elgon.mountain>
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()
next prev parent reply other threads:[~2012-03-15 23:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-13 17:20 qla2xxx: Add LLD target-mode infrastructure for >= 24xx series Dan Carpenter
2012-03-15 23:10 ` Nicholas A. Bellinger [this message]
2012-03-16 13:59 ` Jörn Engel
2012-03-17 1:22 ` Nicholas A. Bellinger
-- strict thread matches above, loose matches on Subject: below --
2012-05-07 13:59 Dan Carpenter
2012-05-07 20:51 ` Nicholas A. Bellinger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1331853015.10203.128.camel@haakon2.linux-iscsi.org \
--to=nab@linux-iscsi.org \
--cc=andrew.vasquez@qlogic.com \
--cc=dan.carpenter@oracle.com \
--cc=giridhar.malavali@qlogic.com \
--cc=linux-driver@qlogic.com \
--cc=linux-scsi@vger.kernel.org \
--cc=target-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).