From: Quinn Tran <quinn.t...@qlogic.com>
During LUN/Target reset, the TMR code attempt to intercept
cmds and try to aborted them. Current code assume cmds are
always intercepted at the back end device. The cleanup code
would issue a "queue_status() & check_stop_free()" to terminate
the command. However, when a cmd is intercepted at the front
end/Fabric layer, current code introduce premature free or
cause Fabric to double free.
When command is intercepted at Fabric layer, it means a
check_stop_free(cmd_kref--) has been called. The extra
check_stop_free in the Lun Reset cleanup code causes early
free. When a cmd in the Fabric layer is completed, the normal
free code adds another another free which introduce a double free.
To fix the issue:
- add a new flag/CMD_T_SENT_STATUS to track command that have
made it down to fabric layer after back end good/bad completion.
- if cmd reach Fabric Layer at Lun Reset time, add an extra
cmd_kref count to prevent premature free.
Signed-off-by: Quinn Tran <quinn.t...@qlogic.com>
Signed-off-by: Himanshu Madhani <himanshu.madh...@qlogic.com>
---
drivers/target/target_core_tmr.c | 33 +++++++++++++++++++++++++++++++-
drivers/target/target_core_transport.c | 30 +++++++++++++++++++++++++++++
include/target/target_core_base.h | 1 +
3 files changed, 63 insertions(+), 1 deletions(-)
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 28fb301..41f8b57 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -243,7 +243,9 @@ static void core_tmr_drain_state_list(
{
LIST_HEAD(drain_task_list);
struct se_cmd *cmd, *next;
- unsigned long flags;
+ unsigned long flags, flags2;
+ int rmkref;
+ struct se_session *se_sess;
/*
* Complete outstanding commands with TASK_ABORTED SAM status.
@@ -282,6 +284,16 @@ static void core_tmr_drain_state_list(
if (prout_cmd == cmd)
continue;
+ se_sess = cmd->se_sess;
+ /* take an extra kref to prevent cmd free race condition. */
+ spin_lock_irqsave(&se_sess->sess_cmd_lock, flags2);
+ if (!kref_get_unless_zero(&cmd->cmd_kref)) {
+ /* cmd is already in the free process */
+ spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags2);
+ continue;
+ }
+ spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags2);
+
list_move_tail(&cmd->state_list, &drain_task_list);
cmd->state_active = false;
}
@@ -320,9 +332,28 @@ static void core_tmr_drain_state_list(
target_stop_cmd(cmd, &flags);
cmd->transport_state |= CMD_T_ABORTED;
+
+ /* CMD_T_SENT_STATUS: cmd is down in fabric layer.
+ * A check stop has been called. Keep the extra kref
+ * from above because core_tmr_handle_tas_abort will
+ * generate another check_stop.
+ *
+ * !CMD_T_SENT_STATUS: cmd intercepted at back end.
+ * Remove the extra kref from above because only
+ * 1 check_stop is required or generated by
+ * core_tmr_handle_tas_abort()
+ */
+ rmkref = 0;
+ if (!((cmd->t_state == TRANSPORT_COMPLETE) &&
+ (cmd->transport_state & CMD_T_SENT_STATUS)))
+ rmkref = 1;
+
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
core_tmr_handle_tas_abort(tmr_nacl, cmd, tas);
+
+ if (rmkref)
+ target_put_sess_cmd(cmd);
}
}
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index 4fdcee2..cdd18bf 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -639,9 +639,14 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int
remove)
static void target_complete_failure_work(struct work_struct *work)
{
struct se_cmd *cmd = container_of(work, struct se_cmd, work);
+ unsigned long flags;
transport_generic_request_failure(cmd,
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE);
+
+ spin_lock_irqsave(&cmd->t_state_lock, flags);
+ cmd->transport_state |= CMD_T_SENT_STATUS;
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
}
/*
@@ -1659,6 +1664,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
sense_reason_t sense_reason)
{
int ret = 0, post_ret = 0;
+ unsigned long flags;
pr_debug("-----[ Storage Engine Exception for cmd: %p ITT: 0x%08llx"
" CDB: 0x%02x\n", cmd, cmd->tag, cmd->t_task_cdb[0]);
@@ -1670,6 +1676,10 @@ void transport_generic_request_failure(struct se_cmd
*cmd,
(cmd->transport_state & CMD_T_STOP) != 0,
(cmd->transport_state & CMD_T_SENT) != 0);
+ spin_lock_irqsave(&cmd->t_state_lock, flags);
+ cmd->transport_state |= CMD_T_SENT_STATUS;
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+
/*
* For SAM Task Attribute emulation for failed struct se_cmd
*/
@@ -1951,6 +1961,7 @@ static void transport_complete_task_attr(struct se_cmd
*cmd)
static void transport_complete_qf(struct se_cmd *cmd)
{
int ret = 0;
+ unsigned long flags;
transport_complete_task_attr(cmd);
@@ -1986,6 +1997,10 @@ out:
}
transport_lun_remove_cmd(cmd);
transport_cmd_check_stop_to_fabric(cmd);
+
+ spin_lock_irqsave(&cmd->t_state_lock, flags);
+ cmd->transport_state |= CMD_T_SENT_STATUS;
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
}
static void transport_handle_queue_full(
@@ -2032,6 +2047,7 @@ static void target_complete_ok_work(struct work_struct
*work)
{
struct se_cmd *cmd = container_of(work, struct se_cmd, work);
int ret;
+ unsigned long flags;
/*
* Check if we need to move delayed/dormant tasks from cmds on the
@@ -2060,6 +2076,10 @@ static void target_complete_ok_work(struct work_struct
*work)
transport_lun_remove_cmd(cmd);
transport_cmd_check_stop_to_fabric(cmd);
+
+ spin_lock_irqsave(&cmd->t_state_lock, flags);
+ cmd->transport_state |= CMD_T_SENT_STATUS;
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
return;
}
/*
@@ -2086,6 +2106,11 @@ static void target_complete_ok_work(struct work_struct
*work)
transport_lun_remove_cmd(cmd);
transport_cmd_check_stop_to_fabric(cmd);
+
+ spin_lock_irqsave(&cmd->t_state_lock, flags);
+ cmd->transport_state |= CMD_T_SENT_STATUS;
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+
return;
}
}
@@ -2136,6 +2161,7 @@ queue_rsp:
ret = cmd->se_tfo->queue_status(cmd);
if (ret == -EAGAIN || ret == -ENOMEM)
goto queue_full;
+
break;
default:
break;
@@ -2143,6 +2169,10 @@ queue_rsp:
transport_lun_remove_cmd(cmd);
transport_cmd_check_stop_to_fabric(cmd);
+
+ spin_lock_irqsave(&cmd->t_state_lock, flags);
+ cmd->transport_state |= CMD_T_SENT_STATUS;
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
return;
queue_full:
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index aabf0ac..efccd71 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -490,6 +490,7 @@ struct se_cmd {
#define CMD_T_DEV_ACTIVE (1 << 7)
#define CMD_T_REQUEST_STOP (1 << 8)
#define CMD_T_BUSY (1 << 9)
+#define CMD_T_SENT_STATUS (1 << 10)
spinlock_t t_state_lock;
struct kref cmd_kref;
struct completion t_transport_stop_comp;
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
