All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: target-devel <target-devel@vger.kernel.org>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Nicholas Bellinger <nab@linux-iscsi.org>,
	Mike Christie <mchristi@redhat.com>,
	Hannes Reinecke <hare@suse.com>,
	Bart Van Assche <bart.vanassche@sandisk.com>
Subject: [PATCH 2/6] target: Fix caw_sem leak in transport_generic_request_failure
Date: Wed,  8 Nov 2017 04:31:48 +0000	[thread overview]
Message-ID: <1510115512-15617-3-git-send-email-nab@linux-iscsi.org> (raw)
In-Reply-To: <1510115512-15617-1-git-send-email-nab@linux-iscsi.org>

From: Nicholas Bellinger <nab@linux-iscsi.org>

With the recent addition of transport_check_aborted_status() within
transport_generic_request_failure() to avoid sending a SCSI status
exception after CMD_T_ABORTED w/ TAS=1 has occured, it introduced
a COMPARE_AND_WRITE early failure regression.

Namely when COMPARE_AND_WRITE fails and se_device->caw_sem has
been taken by sbc_compare_and_write(), if the new check for
transport_check_aborted_status() returns true and exits,
cmd->transport_complete_callback() -> compare_and_write_post()
is skipped never releasing se_device->caw_sem.

This regression was originally introduced by:

  commit e3b88ee95b4e4bf3e9729a4695d695b9c7c296c8
  Author: Bart Van Assche <bart.vanassche@sandisk.com>
  Date:   Tue Feb 14 16:25:45 2017 -0800

      target: Fix handling of aborted failed commands

To address this bug, move the transport_check_aborted_status()
call after transport_complete_task_attr() and
cmd->transport_complete_callback().

Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Bart Van Assche <bart.vanassche@sandisk.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_transport.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index c33d1e9..d02218c 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1729,9 +1729,6 @@ void transport_generic_request_failure(struct se_cmd *cmd,
 {
 	int ret = 0, post_ret = 0;
 
-	if (transport_check_aborted_status(cmd, 1))
-		return;
-
 	pr_debug("-----[ Storage Engine Exception; sense_reason %d\n",
 		 sense_reason);
 	target_show_cmd("-----[ ", cmd);
@@ -1740,6 +1737,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
 	 * For SAM Task Attribute emulation for failed struct se_cmd
 	 */
 	transport_complete_task_attr(cmd);
+
 	/*
 	 * Handle special case for COMPARE_AND_WRITE failure, where the
 	 * callback is expected to drop the per device ->caw_sem.
@@ -1748,6 +1746,9 @@ void transport_generic_request_failure(struct se_cmd *cmd,
 	     cmd->transport_complete_callback)
 		cmd->transport_complete_callback(cmd, false, &post_ret);
 
+	if (transport_check_aborted_status(cmd, 1))
+		return;
+
 	switch (sense_reason) {
 	case TCM_NON_EXISTENT_LUN:
 	case TCM_UNSUPPORTED_SCSI_OPCODE:
-- 
1.9.1

WARNING: multiple messages have this Message-ID (diff)
From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: target-devel <target-devel@vger.kernel.org>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Nicholas Bellinger <nab@linux-iscsi.org>,
	Mike Christie <mchristi@redhat.com>,
	Hannes Reinecke <hare@suse.com>,
	Bart Van Assche <bart.vanassche@sandisk.com>
Subject: [PATCH 2/6] target: Fix caw_sem leak in transport_generic_request_failure
Date: Wed, 08 Nov 2017 04:31:48 +0000	[thread overview]
Message-ID: <1510115512-15617-3-git-send-email-nab@linux-iscsi.org> (raw)
In-Reply-To: <1510115512-15617-1-git-send-email-nab@linux-iscsi.org>

From: Nicholas Bellinger <nab@linux-iscsi.org>

With the recent addition of transport_check_aborted_status() within
transport_generic_request_failure() to avoid sending a SCSI status
exception after CMD_T_ABORTED w/ TAS=1 has occured, it introduced
a COMPARE_AND_WRITE early failure regression.

Namely when COMPARE_AND_WRITE fails and se_device->caw_sem has
been taken by sbc_compare_and_write(), if the new check for
transport_check_aborted_status() returns true and exits,
cmd->transport_complete_callback() -> compare_and_write_post()
is skipped never releasing se_device->caw_sem.

This regression was originally introduced by:

  commit e3b88ee95b4e4bf3e9729a4695d695b9c7c296c8
  Author: Bart Van Assche <bart.vanassche@sandisk.com>
  Date:   Tue Feb 14 16:25:45 2017 -0800

      target: Fix handling of aborted failed commands

To address this bug, move the transport_check_aborted_status()
call after transport_complete_task_attr() and
cmd->transport_complete_callback().

Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Bart Van Assche <bart.vanassche@sandisk.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_transport.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index c33d1e9..d02218c 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1729,9 +1729,6 @@ void transport_generic_request_failure(struct se_cmd *cmd,
 {
 	int ret = 0, post_ret = 0;
 
-	if (transport_check_aborted_status(cmd, 1))
-		return;
-
 	pr_debug("-----[ Storage Engine Exception; sense_reason %d\n",
 		 sense_reason);
 	target_show_cmd("-----[ ", cmd);
@@ -1740,6 +1737,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
 	 * For SAM Task Attribute emulation for failed struct se_cmd
 	 */
 	transport_complete_task_attr(cmd);
+
 	/*
 	 * Handle special case for COMPARE_AND_WRITE failure, where the
 	 * callback is expected to drop the per device ->caw_sem.
@@ -1748,6 +1746,9 @@ void transport_generic_request_failure(struct se_cmd *cmd,
 	     cmd->transport_complete_callback)
 		cmd->transport_complete_callback(cmd, false, &post_ret);
 
+	if (transport_check_aborted_status(cmd, 1))
+		return;
+
 	switch (sense_reason) {
 	case TCM_NON_EXISTENT_LUN:
 	case TCM_UNSUPPORTED_SCSI_OPCODE:
-- 
1.9.1


  parent reply	other threads:[~2017-11-08  4:31 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-08  4:31 [PATCH 0/6] target fixes for v4.15-rc1 Nicholas A. Bellinger
2017-11-08  4:31 ` Nicholas A. Bellinger
2017-11-08  4:31 ` [PATCH 1/6] target: Fix QUEUE_FULL + SCSI task attribute handling Nicholas A. Bellinger
2017-11-08  4:31   ` Nicholas A. Bellinger
2017-11-08 21:29   ` Bryant G. Ly
2017-11-08 21:29     ` Bryant G. Ly
2017-11-08  4:31 ` Nicholas A. Bellinger [this message]
2017-11-08  4:31   ` [PATCH 2/6] target: Fix caw_sem leak in transport_generic_request_failure Nicholas A. Bellinger
2017-11-08  4:31 ` [PATCH 3/6] target: Fix quiese during transport_write_pending_qf endless loop Nicholas A. Bellinger
2017-11-08  4:31   ` Nicholas A. Bellinger
2017-11-08 21:29   ` Bryant G. Ly
2017-11-08 21:29     ` Bryant G. Ly
2017-11-08  4:31 ` [PATCH 4/6] target: Avoid early CMD_T_PRE_EXECUTE failures during ABORT_TASK Nicholas A. Bellinger
2017-11-08  4:31   ` Nicholas A. Bellinger
2017-11-08  4:31 ` [PATCH 5/6] iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref Nicholas A. Bellinger
2017-11-08  4:31   ` Nicholas A. Bellinger
2017-11-08  4:31 ` [PATCH 6/6] iscsi-target: Fix non-immediate TMR reference leak Nicholas A. Bellinger
2017-11-08  4:31   ` 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=1510115512-15617-3-git-send-email-nab@linux-iscsi.org \
    --to=nab@linux-iscsi.org \
    --cc=bart.vanassche@sandisk.com \
    --cc=hare@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mchristi@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.