linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roland Dreier <roland@kernel.org>
To: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: target-devel@vger.kernel.org, linux-scsi@vger.kernel.org,
	Roland Dreier <roland@purestorage.com>
Subject: [PATCH 1/5] target: Don't let abort handling free pending write commands too soon
Date: Wed,  2 Jan 2013 12:47:57 -0800	[thread overview]
Message-ID: <1357159681-24477-2-git-send-email-roland@kernel.org> (raw)
In-Reply-To: <1357159681-24477-1-git-send-email-roland@kernel.org>

From: Roland Dreier <roland@purestorage.com>

The following sequence happens for write commands (or any other
commands with a data out phase):

 - The transport calls target_submit_cmd(), which sets CMD_T_ACTIVE in
   cmd->transport_state and sets cmd->t_state to TRANSPORT_NEW_CMD.
 - Things go on transport_generic_new_cmd(), which notices that the
   command needs to transfer data, so it sets cmd->t_state to
   TRANSPORT_WRITE_PENDING and calls transport_cmd_check_stop().
 - transport_cmd_check_stop() clears CMD_T_ACTIVE in cmd->transport_state
   and returns in the normal case.
 - Then we continue on to call ->se_tfo->write_pending().
 - The data comes back from the initiator, and the transport calls
   target_execute_cmd(), which sets cmd->t_state to TRANSPORT_PROCESSING
   and calls into the backend to actually write the data.

At this point, the backend might take a long time to complete the
command, since it has to do real IO.  If an abort request comes in for
this command at this point, it will not wait for the command to finish
since CMD_T_ACTIVE is not set.  Then when the command does finally
finish, we blow up with use-after-free.

Avoid this by setting CMD_T_ACTIVE in target_execute_cmd() so that
transport_wait_for_tasks() waits for the command to finish executing.
This matches the behavior from before commit 1389533ef944 ("target:
remove transport_generic_handle_data"), when data was signaled via
transport_generic_handle_data(), which set CMD_T_ACTIVE because it
called transport_add_cmd_to_queue().

Signed-off-by: Roland Dreier <roland@purestorage.com>
---
 drivers/target/target_core_transport.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index c23c76c..1dd9d66 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1688,6 +1688,7 @@ void target_execute_cmd(struct se_cmd *cmd)
 	}
 
 	cmd->t_state = TRANSPORT_PROCESSING;
+	cmd->transport_state |= CMD_T_ACTIVE;
 	spin_unlock_irq(&cmd->t_state_lock);
 
 	if (!target_handle_task_attr(cmd))
-- 
1.8.0


  reply	other threads:[~2013-01-02 20:48 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-02 20:47 [PATCH 0/5] target task management fixes and cleanups Roland Dreier
2013-01-02 20:47 ` Roland Dreier [this message]
2013-01-11  4:21   ` [PATCH 1/5] target: Don't let abort handling free pending write commands too soon Nicholas A. Bellinger
2013-01-02 20:47 ` [PATCH 2/5] target: Fix use-after-free in LUN RESET handling Roland Dreier
2013-01-11  4:33   ` Nicholas A. Bellinger
2013-01-02 20:47 ` [PATCH 3/5] target: Release se_cmd when LUN lookup fails for TMR Roland Dreier
2013-01-11  4:39   ` Nicholas A. Bellinger
2013-01-02 20:48 ` [PATCH 4/5] target: Remove useless if statement Roland Dreier
2013-01-11  4:41   ` Nicholas A. Bellinger
2013-01-02 20:48 ` [PATCH 5/5] target: Remove never-used TMR_FABRIC_TMR enum value Roland Dreier
2013-01-11  4:42   ` 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=1357159681-24477-2-git-send-email-roland@kernel.org \
    --to=roland@kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=nab@linux-iscsi.org \
    --cc=roland@purestorage.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 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).