blob: 07760d12f3e3d991ce4ca66a60fbc251d9f74ef7 [file] [log] [blame]
From 79a7fef26431830e22e282053d050af790117db8 Mon Sep 17 00:00:00 2001
From: Roland Dreier <roland@purestorage.com>
Date: Wed, 28 Sep 2011 22:12:07 -0700
Subject: target: Prevent cmd->se_queue_node double add
From: Roland Dreier <roland@purestorage.com>
commit 79a7fef26431830e22e282053d050af790117db8 upstream.
This patch addresses a bug with the lio-core-2.6.git conversion of
transport_add_cmd_to_queue() to use a single embedded list_head, instead
of individual struct se_queue_req allocations allowing a single se_cmd to
be added to the queue mulitple times. This was changed in the following:
commit 2a9e4d5ca5d99f4c600578d6285d45142e7e5208
Author: Andy Grover <agrover@redhat.com>
Date: Tue Apr 26 17:45:51 2011 -0700
target: Embed qr in struct se_cmd
The problem is that some target code still assumes performing multiple
adds is allowed via transport_add_cmd_to_queue(), which ends up causing
list corruption in qobj->qobj_list code. This patch addresses this
by removing an existing struct se_cmd from the list before the add, and
removes an unnecessary list walk in transport_remove_cmd_from_queue()
It also changes cmd->t_transport_queue_active to use explict sets intead
of increment/decrement to prevent confusion during exception path handling.
Signed-off-by: Roland Dreier <roland@purestorage.com>
Cc: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@risingtidesystems.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
drivers/target/target_core_tmr.c | 2 +-
drivers/target/target_core_transport.c | 31 +++++++++++++++----------------
2 files changed, 16 insertions(+), 17 deletions(-)
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -340,7 +340,7 @@ int core_tmr_lun_reset(
atomic_dec(&cmd->t_transport_queue_active);
atomic_dec(&qobj->queue_cnt);
- list_del(&cmd->se_queue_node);
+ list_del_init(&cmd->se_queue_node);
spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
pr_debug("LUN_RESET: %s from Device Queue: cmd: %p t_state:"
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -621,8 +621,6 @@ static void transport_add_cmd_to_queue(
struct se_queue_obj *qobj = &dev->dev_queue_obj;
unsigned long flags;
- INIT_LIST_HEAD(&cmd->se_queue_node);
-
if (t_state) {
spin_lock_irqsave(&cmd->t_state_lock, flags);
cmd->t_state = t_state;
@@ -631,15 +629,21 @@ static void transport_add_cmd_to_queue(
}
spin_lock_irqsave(&qobj->cmd_queue_lock, flags);
+
+ /* If the cmd is already on the list, remove it before we add it */
+ if (!list_empty(&cmd->se_queue_node))
+ list_del(&cmd->se_queue_node);
+ else
+ atomic_inc(&qobj->queue_cnt);
+
if (cmd->se_cmd_flags & SCF_EMULATE_QUEUE_FULL) {
cmd->se_cmd_flags &= ~SCF_EMULATE_QUEUE_FULL;
list_add(&cmd->se_queue_node, &qobj->qobj_list);
} else
list_add_tail(&cmd->se_queue_node, &qobj->qobj_list);
- atomic_inc(&cmd->t_transport_queue_active);
+ atomic_set(&cmd->t_transport_queue_active, 1);
spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
- atomic_inc(&qobj->queue_cnt);
wake_up_interruptible(&qobj->thread_wq);
}
@@ -656,9 +660,9 @@ transport_get_cmd_from_queue(struct se_q
}
cmd = list_first_entry(&qobj->qobj_list, struct se_cmd, se_queue_node);
- atomic_dec(&cmd->t_transport_queue_active);
+ atomic_set(&cmd->t_transport_queue_active, 0);
- list_del(&cmd->se_queue_node);
+ list_del_init(&cmd->se_queue_node);
atomic_dec(&qobj->queue_cnt);
spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
@@ -668,7 +672,6 @@ transport_get_cmd_from_queue(struct se_q
static void transport_remove_cmd_from_queue(struct se_cmd *cmd,
struct se_queue_obj *qobj)
{
- struct se_cmd *t;
unsigned long flags;
spin_lock_irqsave(&qobj->cmd_queue_lock, flags);
@@ -676,14 +679,9 @@ static void transport_remove_cmd_from_qu
spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
return;
}
-
- list_for_each_entry(t, &qobj->qobj_list, se_queue_node)
- if (t == cmd) {
- atomic_dec(&cmd->t_transport_queue_active);
- atomic_dec(&qobj->queue_cnt);
- list_del(&cmd->se_queue_node);
- break;
- }
+ atomic_set(&cmd->t_transport_queue_active, 0);
+ atomic_dec(&qobj->queue_cnt);
+ list_del_init(&cmd->se_queue_node);
spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
if (atomic_read(&cmd->t_transport_queue_active)) {
@@ -1067,7 +1065,7 @@ static void transport_release_all_cmds(s
list_for_each_entry_safe(cmd, tcmd, &dev->dev_queue_obj.qobj_list,
se_queue_node) {
t_state = cmd->t_state;
- list_del(&cmd->se_queue_node);
+ list_del_init(&cmd->se_queue_node);
spin_unlock_irqrestore(&dev->dev_queue_obj.cmd_queue_lock,
flags);
@@ -1598,6 +1596,7 @@ void transport_init_se_cmd(
INIT_LIST_HEAD(&cmd->se_delayed_node);
INIT_LIST_HEAD(&cmd->se_ordered_node);
INIT_LIST_HEAD(&cmd->se_qf_node);
+ INIT_LIST_HEAD(&cmd->se_queue_node);
INIT_LIST_HEAD(&cmd->t_task_list);
init_completion(&cmd->transport_lun_fe_stop_comp);