On Wed, 2013-01-02 at 12:47 -0800, Roland Dreier wrote:
> From: Roland Dreier <roland@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
Applied a CC' to stable with a extra comment wrt to the commit that
introduced this regression.
Thanks again Roland!
--nab
> 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))
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
