* [PATCH 1/2] qla2xxx: Don't crash if we can't find cmd for failed CTIO
@ 2012-06-12 1:23 Roland Dreier
2012-06-12 1:23 ` [PATCH 2/2] qla2xxx: Don't leak commands we give up on in qlt_do_work() Roland Dreier
0 siblings, 1 reply; 2+ messages in thread
From: Roland Dreier @ 2012-06-12 1:23 UTC (permalink / raw)
To: Arun Easi, Chad Dupuis, Nicholas A. Bellinger
Cc: target-devel, linux-scsi, Roland Dreier
From: Roland Dreier <roland@purestorage.com>
In qlt_do_ctio_completion(), there's no point in calling
qlt_term_ctio_exchange() with a NULL cmd -- all that it does is crash
in a NULL pointer dereference, since it does
qlt_send_term_exchange(vha, cmd, &cmd->atio, 1);
and dereferencing &cmd->atio is a bad idea if cmd itself is NULL.
If we really need to do this, we could take the values from the
failed CTIO we're processing, but it's not clear if it's worth
the replumbing to do that.
Signed-off-by: Roland Dreier <roland@purestorage.com>
---
drivers/scsi/qla2xxx/qla_target.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 04f80eb..c263f90 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -2477,11 +2477,9 @@ static void qlt_do_ctio_completion(struct scsi_qla_host *vha, uint32_t handle,
}
cmd = qlt_ctio_to_cmd(vha, handle, ctio);
- if (cmd == NULL) {
- if (status != CTIO_SUCCESS)
- qlt_term_ctio_exchange(vha, ctio, NULL, status);
+ if (cmd == NULL)
return;
- }
+
se_cmd = &cmd->se_cmd;
tfo = se_cmd->se_tfo;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH 2/2] qla2xxx: Don't leak commands we give up on in qlt_do_work()
2012-06-12 1:23 [PATCH 1/2] qla2xxx: Don't crash if we can't find cmd for failed CTIO Roland Dreier
@ 2012-06-12 1:23 ` Roland Dreier
0 siblings, 0 replies; 2+ messages in thread
From: Roland Dreier @ 2012-06-12 1:23 UTC (permalink / raw)
To: Arun Easi, Chad Dupuis, Nicholas A. Bellinger
Cc: target-devel, linux-scsi, Roland Dreier
From: Roland Dreier <roland@purestorage.com>
If we go to the "out_term:" exit path in qlt_do_work(), we call
qlt_send_term_exchange() with a NULL cmd, which means that it can't
possibly free the cmd for us. Add an explicit call to free the
command memory, so we don't leak the allocation.
This will also fix warnings about "BUG qla_tgt_cmd_cachep: Objects
remaining on kmem_cache_close" from slub when unloading the qla2xxx
target module.
Signed-off-by: Roland Dreier <roland@purestorage.com>
---
drivers/scsi/qla2xxx/qla_target.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index c263f90..e912920 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -2725,10 +2725,12 @@ static void qlt_do_work(struct work_struct *work)
out_term:
ql_dbg(ql_dbg_tgt_mgt, vha, 0xf020, "Terminating work cmd %p", cmd);
/*
- * cmd has not sent to target yet, so pass NULL as the second argument
+ * cmd has not sent to target yet, so pass NULL as the second
+ * argument to qlt_send_term_exchange() and free the memory here.
*/
spin_lock_irqsave(&ha->hardware_lock, flags);
qlt_send_term_exchange(vha, NULL, &cmd->atio, 1);
+ kmem_cache_free(qla_tgt_cmd_cachep, cmd);
spin_unlock_irqrestore(&ha->hardware_lock, flags);
if (sess)
ha->tgt.tgt_ops->put_sess(sess);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-06-12 1:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-12 1:23 [PATCH 1/2] qla2xxx: Don't crash if we can't find cmd for failed CTIO Roland Dreier
2012-06-12 1:23 ` [PATCH 2/2] qla2xxx: Don't leak commands we give up on in qlt_do_work() Roland Dreier
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).