lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 09 Nov 2011 13:31:25 -0800
From:	Greg KH <gregkh@...e.de>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc:	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	alan@...rguk.ukuu.org.uk, Roland Dreier <roland@...estorage.com>,
	Andy Grover <agrover@...hat.com>,
	Nicholas Bellinger <nab@...ingtidesystems.com>
Subject: [038/264] target: Prevent cmd->se_queue_node double add

3.1-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Roland Dreier <roland@...estorage.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@...hat.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@...estorage.com>
Cc: Andy Grover <agrover@...hat.com>
Signed-off-by: Nicholas Bellinger <nab@...ingtidesystems.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.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);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ