Re: [PATCH 1/2] scsi: target: iscsi: Complete an aborted WRITE stuck before the first R2T

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iscsi: double completion and workqueue corruption for aborted WRITEs on DataOut path
--

commit 7b04f6989363db2c2b75d8c212ee852153982c8d
Author: Laurence Oberman <loberman@redhat.com>
    
scsi: target: iscsi: Complete an aborted WRITE stuck before the first R2T

This patch resolves an issue where an aborted WRITE command that arrives with
immediate data exceeding FirstBurstLength gets stuck in TRANSPORT_WRITE_PENDING.
It accomplishes this by completing the command in iscsit_execute_cmd() and
introducing a new iscsit_claim_completion_locked() helper to avoid a double
completion race with iscsit_release_commands_from_conn().

> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 124ff269b8e75..aab9237a67768 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -4198,10 +4198,18 @@ static void iscsit_release_commands_from_conn(struct iscsit_conn *conn)
>  		if (cmd->se_cmd.t_state == TRANSPORT_WRITE_PENDING) {
>  			/*
>  			 * We never submitted the cmd to LIO core, so we have
> -			 * to tell LIO to perform the completion process.
> +			 * to tell LIO to perform the completion process --
> +			 * unless iscsit_execute_cmd() already did.  t_state
> +			 * stays TRANSPORT_WRITE_PENDING across that
> +			 * completion, so the claim is the only thing that can
> +			 * tell us.
>  			 */
> +			bool claimed = iscsit_claim_completion_locked(cmd);
> +
>  			spin_unlock_irq(&se_cmd->t_state_lock);
> -			target_complete_cmd(&cmd->se_cmd, SAM_STAT_TASK_ABORTED);
> +			if (claimed)
> +				target_complete_cmd(&cmd->se_cmd,
> +						    SAM_STAT_TASK_ABORTED);

[Severity: High]
Does this code miss the DataOut paths?

Because target_complete_cmd() is called directly in __iscsit_check_dataout_hdr()
and target_execute_cmd() is called in iscsit_check_dataout_payload() for aborted
commands without setting the completion_claimed flag.

If a command is aborted on the DataOut path:

drivers/target/iscsi/iscsi_target.c:__iscsit_check_dataout_hdr() {
    ...
		if (se_cmd->transport_state & CMD_T_ABORTED) {
			if (hdr->flags & ISCSI_FLAG_CMD_FINAL &&
			    --cmd->outstanding_r2ts < 1) {
				iscsit_stop_dataout_timer(cmd);
				target_complete_cmd(se_cmd, SAM_STAT_TASK_ABORTED);
			}
    ...
}

Because completion_claimed is never set, target_complete_cmd() delegates
to target_cmd_interrupted() which queues target_abort_work but leaves
the command in TRANSPORT_WRITE_PENDING.

If the connection drops before target_abort_work removes the command
from the connection list, iscsit_release_commands_from_conn() will find it,
iscsit_claim_completion_locked() will return true, and it will call
target_complete_cmd() a second time.

Can this double call to target_complete_cmd() cause target_cmd_interrupted()
to re-initialize and re-queue cmd->work while it is already pending on the
workqueue, leading to workqueue linked-list corruption and a use-after-free
when target_handle_abort() runs twice?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014143932.39453-1-loberman@redhat.com?part=1