[PATCH v3 0/4] scsi: target: iscsi: fix stalled and doubly completed aborted WRITEs

[PATCH v3 0/4] scsi: target: iscsi: fix stalled and doubly completed aborted WRITEs

[Laurence Oberman]

These four patches apply on top of commit d5869dae5080 ("scsi: target: iscsi: Fix hang for aborted WRITE_PENDING commands").

A WRITE command that is still in TRANSPORT_WRITE_PENDING has never been
passed to the backend, so when a TMR aborts it the iSCSI fabric is the one
that has to complete it. That commit fixed the case where the final
DataOut PDU for an already-issued R2T arrives after the abort. Three more
paths can leave such a command with nothing to complete it, or let it be
completed twice:

 - Patch 1 is the hang this work started from. A WRITE that arrives with
   immediate data, aborted after that data has been received but before
   the first R2T is built, is never completed by anything: no R2T has been
   sent so the initiator cannot send more data, and no DataOut timer is
   armed so the ERL=0 timeout cannot fire either. core_tmr_lun_reset()
   then blocks for as long as the connection stays up.

 - Patch 2 closes the equivalent window where a new R2T would have been
   sent for an already-aborted command.

 - Patches 3 and 4 fix two existing sites that complete an aborted
   WRITE_PENDING command without recording that they have done so, and
   which connection teardown can therefore complete a second time.

Patch 1 also adds a one-shot completion claim. target_complete_cmd() on a
command that already has CMD_T_ABORTED set goes into
target_cmd_interrupted(), which queues target_abort_work and returns
without setting CMD_T_COMPLETE and without changing t_state, so target
core records nothing that would stop a second completion. The command
stays in TRANSPORT_WRITE_PENDING and on conn_cmd_list until
target_abort_work reaches iscsit_aborted_task(), and
iscsit_release_commands_from_conn() completes anything it finds in
TRANSPORT_WRITE_PENDING. The claim is what makes exactly one site
complete the command.

Changes since v2:
 - New patch 3 routes the completion added by that commit through the
   claim, with Fixes: and Cc: stable. Raised in the review of v2, and by
   the Sashiko bot against both v1 and v2.
 - New patch 4 does the same for the DATAOUT_SEND_TO_TRANSPORT path into
   target_execute_cmd(), which has the same shape and is older.
 - Patch 1: reworded the completion_claimed comment. It said the flag is
   set once target_complete_cmd() has been called; it is set before the
   call, by whichever site takes the claim.
 - Patch 2: moved the "never observed in the field" paragraph below the
   --- line. Code unchanged.
 - Patches 1 and 2 are otherwise identical to v2.

v2: https://lore.kernel.org/linux-scsi/20261016175351.126373-1-loberman@redhat.com/
v1: https://lore.kernel.org/linux-scsi/20261014143932.39453-1-loberman@redhat.com/

Two corrections to things I said during the v2 review:

I wrote that if the outstanding_r2ts read in patch 2 is stale low, "the
later completion in __iscsit_check_dataout_hdr() becomes a no-op". That
was wrong. In v2 that site calls target_complete_cmd() directly and never
looks at the claim, so the claim cannot make it a no-op. What actually
prevents a second completion there is that outstanding_r2ts is a u32, so
--cmd->outstanding_r2ts on a counter already at zero wraps to 0xffffffff
and fails the < 1 test. That is an accident of the existing code rather
than a guarantee, and it leaves the counter corrupted. Patch 3 removes
the need to rely on it. I have not tried to fix the underflow itself
here; say the word if you would like a patch for it.

I also said the pre-existing sites were best left to a follow-up. For the
record on the other half of that exchange, that commit is not confined
to 7.3/scsi-staging: it is in mainline, it carries CVE-2026-97951, and it
has been backported to 5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y, 6.18.y and
7.2.y. That is the reason patch 3 has Cc: stable -- and also a reason it
may be better taken as a standalone fix for those branches than as part
of this series, since none of them carry patch 1's helper. I will send
whichever form you prefer.

On fixing this in target core instead:

The root cause is that target_cmd_interrupted() queues target_abort_work
and records nothing under t_state_lock, so a test-and-set there would
cover every fabric rather than iSCSI alone, and would make the
completion_claimed field in this series unnecessary. I think that is the
better end state and I am willing to do it. I did not do it here for two
reasons. It changes the path that every fabric completes through, and I
have no way to test iSER, FC or SRP. And the field evidence behind this
series is evidence for an iSCSI-local change, not for a core one. I would
rather land the narrow fix for a hang that is being hit in production and
then propose the core cleanup on its own merits, in front of the people
who own that code.

How this was found and tested:

The hang was reported against an Oracle RAC cluster on two-session iSCSI
LIO targets, where one node's LUN_RESET aborts WRITEs belonging to the
other node's session. It was not reproducible in a lab, so the diagnosis
came from instrumented kernels in the reporter's environment, logging
every site that completes or waits on an aborted command.

Across two hosts every stall had the identical signature given in patch 1,
and the stall only ever ended when the initiator gave up and dropped the
connection. 217 of 222 LUN_RESET drains completed in under a second; the
rest took 46 to 54 seconds, which is where the hung-task panics came from.
One 53.7 second stall recovered on its own and the host ran for a further
19 hours.

An earlier attempt at this fix completed the command from
core_tmr_drain_state_list() instead. That is not safe: at that point the
initiator may still be sending DataOut for the command, and the result was
a double completion -- re-queued se_cmd->work, list_del corruption and a
refcount underflow. Mentioning it in case the same idea comes up in
review.

What is and is not tested:

Patches 1 and 2, backported to a 5.14-based distro kernel, have run 300
LUN_RESET cycles with no hang, in two independent runs: one instrumented
and one with the tracing removed. The same reproducer previously failed at
95 and 196 cycles.

Only patch 1's window was ever observed. The instrumented kernels recorded
no completions at patch 2's site in either run, and patches 3 and 4 are
closed by inspection. The reporter's environment is a production cluster
and is the only place this has ever reproduced, so I cannot offer an
upstream -rc run or a MaxOutstandingR2T > 1 run from there. If that is a
blocker for patches 2 to 4, I would rather you took patch 1 on its own
than held the series.

Laurence Oberman (4):
  scsi: target: iscsi: Complete an aborted WRITE stuck before the first
    R2T
  scsi: target: iscsi: Complete an aborted WRITE when a new R2T would be
    sent
  scsi: target: iscsi: Claim the completion in
    __iscsit_check_dataout_hdr()
  scsi: target: iscsi: Claim the completion on the DataOut submit path

 drivers/target/iscsi/iscsi_target.c      | 44 +++++++++++++++++++----
 drivers/target/iscsi/iscsi_target_erl1.c | 10 +++++-
 drivers/target/iscsi/iscsi_target_util.c | 46 ++++++++++++++++++++++++
 drivers/target/iscsi/iscsi_target_util.h |  2 ++
 include/target/iscsi/iscsi_target_core.h |  6 ++++
 5 files changed, 101 insertions(+), 7 deletions(-)

-- 
2.43.0

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

[Laurence Oberman]

A WRITE that arrives with immediate data, and whose data length exceeds
FirstBurstLength, is left in TRANSPORT_WRITE_PENDING by
iscsit_execute_cmd() until the first R2T is built. If a LUN_RESET or
ABORT_TASK from another session sets CMD_T_ABORTED in that window,
iscsit_execute_cmd() returns without sending the R2T.

Nothing then completes the command. No R2T has been sent, so the
initiator cannot legally send more data; no DataOut timer was armed, so
the ERL=0 timeout cannot fire either. The command stays in
TRANSPORT_WRITE_PENDING with CMD_T_ABORTED set, and
core_tmr_drain_state_list() blocks in target_put_cmd_and_wait() on it for
as long as the connection stays up:

  INFO: task kworker/5:2 blocked for more than 46 seconds.
   target_put_cmd_and_wait+0x6d/0xb0 [target_core_mod]
   core_tmr_drain_state_list+0x2dc/0x380 [target_core_mod]
   core_tmr_lun_reset+0x93/0x1a0 [target_core_mod]

Commit aaa00cc93c1d ("scsi: target/core: Fix TAS handling for
aborted commands") is what left it that way. Before it, this site
called transport_check_aborted_status(), which sent the delayed
TASK_ABORTED status for the command; that commit replaced the call
with a bare CMD_T_ABORTED test and a return, on the basis that the
target core would send TASK_ABORTED from the ABORT TMF context
instead. That holds only for commands the core can finish on its
own. A WRITE still in TRANSPORT_WRITE_PENDING has never been passed
to the backend, so the core's abort path is itself waiting in
target_put_cmd_and_wait() for the fabric to complete it, and since
that commit the fabric no longer does.

Complete it in iscsit_execute_cmd() instead. No DataOut path can race
with this: without an R2T the initiator cannot send further data for the
command, and the branch is only reached when no unsolicited DataOut is
expected either.

That introduces a second site which can complete a command that
iscsit_release_commands_from_conn() would also complete during connection
teardown, because t_state is still TRANSPORT_WRITE_PENDING there.
target_complete_cmd() on a command that already has CMD_T_ABORTED set
goes straight into target_cmd_interrupted(), which queues
target_abort_work and returns without setting CMD_T_COMPLETE and without
changing t_state, so nothing in target core records that a completion has
happened. A second target_complete_cmd() re-initialises and re-queues
se_cmd->work while it is still queued and drops a command reference that
was never taken. Add a one-shot claim, taken under se_cmd->t_state_lock,
so that only one of the two sites completes the command.

This was reported against an Oracle RAC cluster on two-session iSCSI LIO
targets, where one node's LUN_RESET aborts WRITEs belonging to the other
node's session. It was not reproducible in a lab, so the diagnosis came
from instrumented kernels in the reporter's environment. Across two
hosts, every stalled command had the same signature: DMA_TO_DEVICE,
TRANSPORT_WRITE_PENDING, CMD_T_ABORTED | CMD_T_TAS, outstanding_r2ts == 0,
write_data_done == first_burst_len, and an empty cmd_r2t_list. 217 of 222
LUN_RESET drains completed in under a second; the rest took 46 to 54
seconds, which is where the hung-task panics came from. One 53.7 second
stall recovered on its own and the host ran for a further 19 hours. The
stall only ever ended when the initiator gave up and dropped the
connection, which let iscsit_release_commands_from_conn() complete the
command.

Fixes: aaa00cc93c1d ("scsi: target/core: Fix TAS handling for aborted commands")
Cc: stable@vger.kernel.org
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
 drivers/target/iscsi/iscsi_target.c      | 12 +++++--
 drivers/target/iscsi/iscsi_target_erl1.c | 10 +++++-
 drivers/target/iscsi/iscsi_target_util.c | 46 ++++++++++++++++++++++++
 drivers/target/iscsi/iscsi_target_util.h |  2 ++
 include/target/iscsi/iscsi_target_core.h |  6 ++++
 5 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 124ff26..aab9237 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);
 			continue;
 		}
 		spin_unlock_irq(&se_cmd->t_state_lock);
diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c
index d8ca06e..cf72c79 100644
--- a/drivers/target/iscsi/iscsi_target_erl1.c
+++ b/drivers/target/iscsi/iscsi_target_erl1.c
@@ -927,8 +927,16 @@ int iscsit_execute_cmd(struct iscsit_cmd *cmd, int ooo)
 
 			if (!(cmd->cmd_flags &
 					ICF_NON_IMMEDIATE_UNSOLICITED_DATA)) {
-				if (cmd->se_cmd.transport_state & CMD_T_ABORTED)
+				/*
+				 * No R2T has been sent for this command yet,
+				 * so returning here leaves the initiator with
+				 * nothing to send and no DataOut timer armed.
+				 * Complete it, or nothing ever will.
+				 */
+				if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
+					iscsit_complete_aborted_cmd(cmd);
 					return 0;
+				}
 
 				iscsit_set_dataout_sequence_values(cmd);
 				conn->conn_transport->iscsit_get_dataout(conn, cmd, false);
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index c1888c4..31248cb 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -14,6 +14,7 @@
 #include <scsi/scsi_tcq.h>
 #include <scsi/iscsi_proto.h>
 #include <target/target_core_base.h>
+#include <target/target_core_backend.h>
 #include <target/target_core_fabric.h>
 #include <target/iscsi/iscsi_transport.h>
 
@@ -186,6 +187,51 @@ struct iscsit_cmd *iscsit_allocate_cmd(struct iscsit_conn *conn, int state)
 }
 EXPORT_SYMBOL(iscsit_allocate_cmd);
 
+/*
+ * A WRITE command still in TRANSPORT_WRITE_PENDING has never been passed to
+ * the backend, so when a TMR aborts it the fabric has to complete it.  More
+ * than one site can reach that conclusion for the same command, and
+ * target_complete_cmd() on a command that already has CMD_T_ABORTED set
+ * neither sets CMD_T_COMPLETE nor changes t_state, so target core keeps no
+ * state that would stop a second completion.  Serialise them here: whichever
+ * site claims the completion performs it and every other site leaves the
+ * command alone.
+ *
+ * Caller must hold cmd->se_cmd.t_state_lock.
+ */
+bool iscsit_claim_completion_locked(struct iscsit_cmd *cmd)
+{
+	lockdep_assert_held(&cmd->se_cmd.t_state_lock);
+
+	if (cmd->completion_claimed)
+		return false;
+
+	cmd->completion_claimed = true;
+	return true;
+}
+
+/* Complete an aborted command at most once.  Must not hold t_state_lock. */
+void iscsit_complete_aborted_cmd(struct iscsit_cmd *cmd)
+{
+	struct se_cmd *se_cmd = &cmd->se_cmd;
+	unsigned long flags;
+	bool claimed;
+
+	spin_lock_irqsave(&se_cmd->t_state_lock, flags);
+	claimed = iscsit_claim_completion_locked(cmd);
+	spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
+
+	if (!claimed)
+		return;
+
+	/*
+	 * No-op unless a DataOut timer was armed for this command; callers
+	 * that complete before the first R2T have none.
+	 */
+	iscsit_stop_dataout_timer(cmd);
+	target_complete_cmd(se_cmd, SAM_STAT_TASK_ABORTED);
+}
+
 struct iscsi_seq *iscsit_get_seq_holder_for_datain(
 	struct iscsit_cmd *cmd,
 	u32 seq_send_order)
diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h
index 7ae48a8..8f3fe74 100644
--- a/drivers/target/iscsi/iscsi_target_util.h
+++ b/drivers/target/iscsi/iscsi_target_util.h
@@ -18,6 +18,8 @@ extern struct iscsi_r2t *iscsit_get_r2t_from_list(struct iscsit_cmd *);
 extern void iscsit_free_r2t(struct iscsi_r2t *, struct iscsit_cmd *);
 extern void iscsit_free_r2ts_from_list(struct iscsit_cmd *);
 extern struct iscsit_cmd *iscsit_allocate_cmd(struct iscsit_conn *, int);
+extern bool iscsit_claim_completion_locked(struct iscsit_cmd *cmd);
+extern void iscsit_complete_aborted_cmd(struct iscsit_cmd *cmd);
 extern struct iscsi_seq *iscsit_get_seq_holder_for_datain(struct iscsit_cmd *, u32);
 extern struct iscsi_seq *iscsit_get_seq_holder_for_r2t(struct iscsit_cmd *);
 extern struct iscsi_r2t *iscsit_get_holder_for_r2tsn(struct iscsit_cmd *, u32);
diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 51ca80a..5eb3cf2 100644
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -496,6 +496,12 @@ struct iscsit_cmd {
 	u32			first_data_sg_off;
 	u32			kmapped_nents;
 	sense_reason_t		sense_reason;
+	/*
+	 * Set once one site has claimed the completion of this command.
+	 * The claim is taken before target_complete_cmd() is called.
+	 * Protected by se_cmd.t_state_lock.
+	 */
+	bool			completion_claimed;
 }  ____cacheline_aligned;
 
 struct iscsi_tmr_req {
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: target: iscsi: UAF on cmd after iscsit_sequence_cmd() aborts write
--

commit bf534eddda8811708a661c23a1fc1d455c52b52d
Author: Laurence Oberman <loberman@redhat.com>

scsi: target: iscsi: Complete an aborted WRITE stuck before the first R2T

This commit completes WRITE commands in TRANSPORT_WRITE_PENDING that were
aborted before the first R2T is sent in iscsit_execute_cmd(). It also adds
a completion claim flag under se_cmd.t_state_lock so that only one of
iscsit_execute_cmd() or iscsit_release_commands_from_conn() completes the
command.

> diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c
> index d8ca06e697d63..cf72c79be355a 100644
> --- a/drivers/target/iscsi/iscsi_target_erl1.c
> +++ b/drivers/target/iscsi/iscsi_target_erl1.c
> @@ -927,8 +927,16 @@ int iscsit_execute_cmd(struct iscsit_cmd *cmd, int ooo)
>  
>  			if (!(cmd->cmd_flags &
>  					ICF_NON_IMMEDIATE_UNSOLICITED_DATA)) {
> -				if (cmd->se_cmd.transport_state & CMD_T_ABORTED)
> +				/*
> +				 * No R2T has been sent for this command yet,
> +				 * so returning here leaves the initiator with
> +				 * nothing to send and no DataOut timer armed.
> +				 * Complete it, or nothing ever will.
> +				 */
> +				if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
> +					iscsit_complete_aborted_cmd(cmd);
>  					return 0;
> +				}

[Severity: High]
Can completing the command here cause a use-after-free on cmd in the
callers of iscsit_sequence_cmd()?

When an iSCSI WRITE arrives with immediate data whose total data length
exceeds FirstBurstLength and with ISCSI_FLAG_CMD_FINAL set on the Command
PDU (so cmd->immediate_data == 1 and cmd->unsolicited_data == 0),
iscsit_process_scsi_cmd() calls transport_generic_new_cmd(), which adds
cmd->se_cmd to dev->queues[cpuid].state_list with cmd_kref == 2 (initial
kref plus SCF_ACK_KREF).

If an ABORT_TASK TMR, a same-session LUN_RESET, or a cross-session
LUN_RESET with emulate_tas == 0 aborts the command before
iscsit_execute_cmd() runs, __target_check_io_state() sets CMD_T_ABORTED
without CMD_T_TAS.

When iscsit_get_immediate_data() or cxgbit_get_immediate_data() then calls
iscsit_sequence_cmd() -> iscsit_execute_cmd(), iscsit_complete_aborted_cmd()
calls target_complete_cmd() -> target_cmd_interrupted(), queueing
target_abort_work on target_completion_wq. On another CPU,
target_abort_work -> target_handle_abort() runs iscsit_aborted_task(),
drops SCF_ACK_KREF via target_put_sess_cmd(), and calls
transport_cmd_check_stop_to_fabric() -> lio_check_stop_free() ->
target_put_sess_cmd() -> iscsit_release_cmd() -> target_free_tag(), which
returns the cmd slot to se_sess->sess_tag_pool.

However, after iscsit_sequence_cmd() returns CMDSN_NORMAL_OPERATION (0),
both drivers/target/iscsi/iscsi_target.c:iscsit_get_immediate_data() and
drivers/target/iscsi/cxgbit/cxgbit_target.c:cxgbit_get_immediate_data()
still dereference cmd without holding a reference on cmd->se_cmd:

drivers/target/iscsi/iscsi_target.c:iscsit_get_immediate_data() {
    ...
		cmdsn_ret = iscsit_sequence_cmd(cmd->conn, cmd,
					(unsigned char *)hdr, hdr->cmdsn);
		if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
			return -1;

		if (cmd->sense_reason || cmdsn_ret == CMDSN_LOWER_THAN_EXP) {
			target_put_sess_cmd(&cmd->se_cmd);

			return 0;
		} else if (cmd->unsolicited_data)
			iscsit_set_unsolicited_dataout(cmd);
    ...
}

If the tag slot has already been reallocated from se_sess->sess_tag_pool,
wouldn't reading cmd->sense_reason or cmd->unsolicited_data inspect the
new command and potentially call target_put_sess_cmd() or
iscsit_set_unsolicited_dataout() on the reallocated command?

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

[PATCH v3 2/4] scsi: target: iscsi: Complete an aborted WRITE when a new R2T would be sent

[Laurence Oberman]

iscsit_check_dataout_payload() hands a command to iscsit_get_dataout() to
request the next burst whenever iscsit_check_post_dataout() returns
DATAOUT_SEND_R2T, without checking CMD_T_ABORTED first.

If a TMR aborted the command while the burst that just finished was in
flight, no further DataOut will arrive: the initiator is about to be told
the command was aborted. The CMD_T_ABORTED handling in
__iscsit_check_dataout_hdr() does not cover this, because it only runs
for DataOut PDUs belonging to R2Ts that were already issued, and
outstanding_r2ts has already been decremented to zero for the burst that
completed. The command is left in TRANSPORT_WRITE_PENDING with nothing to
complete it.

Complete it here instead, through iscsit_complete_aborted_cmd() so that
the completion is claimed. target_complete_cmd() on a command that
already has CMD_T_ABORTED set goes to target_cmd_interrupted(), which
queues target_abort_work and returns without setting CMD_T_COMPLETE or
changing t_state. The command therefore stays in TRANSPORT_WRITE_PENDING
and on conn_cmd_list until target_abort_work reaches
iscsit_aborted_task(), and a connection drop in that window would let
iscsit_release_commands_from_conn() complete it a second time. Guard on
outstanding_r2ts so that with MaxOutstandingR2T > 1 the completion is
still left to the last R2T to drain in __iscsit_check_dataout_hdr().

Signed-off-by: Laurence Oberman <loberman@redhat.com>
---

Notes:
    This window was never observed in the field: instrumented kernels on two
    hosts recorded no completions at this site across 300 reset cycles, and
    the reproducer that triggered patch 1 never reached it.  It is closed by
    code inspection, for symmetry with the other paths that can leave an
    aborted WRITE_PENDING command stranded.

 drivers/target/iscsi/iscsi_target.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index aab9237..3dad7f6 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1669,6 +1669,22 @@ iscsit_check_dataout_payload(struct iscsit_cmd *cmd, struct iscsi_data *hdr,
 	if ((rc == DATAOUT_NORMAL) || (rc == DATAOUT_WITHIN_COMMAND_RECOVERY))
 		return 0;
 	else if (rc == DATAOUT_SEND_R2T) {
+		/*
+		 * A new R2T would never be answered for an aborted command.
+		 * The CMD_T_ABORTED handling in __iscsit_check_dataout_hdr()
+		 * does not cover this: it only runs for DataOut PDUs
+		 * belonging to R2Ts that were already issued, and
+		 * iscsit_dataout_update_r2t() has already decremented
+		 * outstanding_r2ts for the burst that just finished.  Guard
+		 * on outstanding_r2ts so that with MaxOutstandingR2T > 1 the
+		 * completion is still left to the last R2T to drain.
+		 */
+		if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
+			if (cmd->outstanding_r2ts < 1)
+				iscsit_complete_aborted_cmd(cmd);
+			return 0;
+		}
+
 		iscsit_set_dataout_sequence_values(cmd);
 		conn->conn_transport->iscsit_get_dataout(conn, cmd, false);
 	} else if (rc == DATAOUT_SEND_TO_TRANSPORT) {
-- 
2.43.0

Re: [PATCH v3 2/4] scsi: target: iscsi: Complete an aborted WRITE when a new R2T would be sent

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] scsi: target: iscsi: UAF of OOO cmd in iscsit_check_dataout_payload()
--

commit 93e4d74df05b6ac07e171f5586d09d2036dc2494
Author: Laurence Oberman <loberman@redhat.com>

scsi: target: iscsi: Complete an aborted WRITE when a new R2T would be sent

This commit completes an aborted WRITE command via
iscsit_complete_aborted_cmd() in iscsit_check_dataout_payload() when
iscsit_check_post_dataout() returns DATAOUT_SEND_R2T and no outstanding R2Ts
remain.

> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index aab9237a67768..3dad7f69f3bae 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -1669,6 +1669,22 @@ iscsit_check_dataout_payload(struct iscsit_cmd *cmd, struct iscsi_data *hdr,
>  	if ((rc == DATAOUT_NORMAL) || (rc == DATAOUT_WITHIN_COMMAND_RECOVERY))
>  		return 0;
>  	else if (rc == DATAOUT_SEND_R2T) {
> +		/*
> +		 * A new R2T would never be answered for an aborted command.
> +		 * The CMD_T_ABORTED handling in __iscsit_check_dataout_hdr()
> +		 * does not cover this: it only runs for DataOut PDUs
> +		 * belonging to R2Ts that were already issued, and
> +		 * iscsit_dataout_update_r2t() has already decremented
> +		 * outstanding_r2ts for the burst that just finished.  Guard
> +		 * on outstanding_r2ts so that with MaxOutstandingR2T > 1 the
> +		 * completion is still left to the last R2T to drain.
> +		 */
> +		if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
> +			if (cmd->outstanding_r2ts < 1)
> +				iscsit_complete_aborted_cmd(cmd);
> +			return 0;
> +		}

[Severity: Critical]
Can completing the command here cause a use-after-free of cmd if
ICF_OOO_CMDSN is set?

When ImmediateData=Yes and InitialR2T=No, a WRITE command can arrive with
immediate data and expect additional unsolicited DataOut PDUs in the first
burst followed by R2T-solicited data (payload_length < FirstBurstLength <
se_cmd.data_length, so cmd->unsolicited_data == 1 and
ICF_NON_IMMEDIATE_UNSOLICITED_DATA is set).

In drivers/target/iscsi/iscsi_target.c:iscsit_process_scsi_cmd(),
transport_generic_new_cmd(&cmd->se_cmd) is called before CmdSN sequencing:

	cmd->sense_reason = transport_generic_new_cmd(&cmd->se_cmd);
	if (cmd->sense_reason)
		return 1;

This places cmd->se_cmd on dev->queues[cpuid].state_list in
TRANSPORT_WRITE_PENDING. When iscsit_get_immediate_data() subsequently calls
iscsit_sequence_cmd() and cmd->cmd_sn > sess->exp_cmd_sn,
drivers/target/iscsi/iscsi_target_erl1.c:iscsit_handle_ooo_cmdsn() queues
an entry on sess->sess_ooo_cmdsn_list holding a raw pointer to cmd:

	cmd->cmd_flags			|= ICF_OOO_CMDSN;
	...
	ooo_cmdsn->cmd			= cmd;

Because cmd->unsolicited_data is set, iscsit_get_immediate_data() then calls
iscsit_set_unsolicited_dataout(cmd).

If a concurrent TMR aborts cmd while it is still waiting on
sess->sess_ooo_cmdsn_list, __target_check_io_state() sets CMD_T_ABORTED on
cmd->se_cmd. When the final unsolicited DataOut PDU of the first burst
arrives, __iscsit_check_dataout_hdr() takes the cmd->unsolicited_data == 1
path, and iscsit_dataout_post_crc_passed() clears cmd->unsolicited_data = 0
and returns DATAOUT_SEND_R2T.

In this new DATAOUT_SEND_R2T branch in iscsit_check_dataout_payload(),
cmd->outstanding_r2ts is 0, so iscsit_complete_aborted_cmd(cmd) is called
without checking ICF_OOO_CMDSN or removing cmd from
sess->sess_ooo_cmdsn_list (unlike the adjacent DATAOUT_SEND_TO_TRANSPORT
branch, which checks cmd->cmd_flags & ICF_OOO_CMDSN).

Once target_complete_cmd() schedules target_abort_work ->
target_handle_abort(), cmd is completed and freed via iscsit_release_cmd().
When the missing ExpCmdSN PDU later arrives,
drivers/target/iscsi/iscsi_target_erl1.c:iscsit_execute_ooo_cmdsns() will
dereference the dangling ooo_cmdsn->cmd pointer:

		cmd = ooo_cmdsn->cmd;
		cmd->i_state = cmd->deferred_i_state;
		ooo_count++;
		sess->exp_cmd_sn++;
		...
		iscsit_remove_ooo_cmdsn(sess, ooo_cmdsn);

		if (iscsit_execute_cmd(cmd, 1) < 0)
			return -1;

Should the DATAOUT_SEND_R2T abort path also account for commands with
ICF_OOO_CMDSN set before completing cmd?

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

Re: [PATCH v3 2/4] scsi: target: iscsi: Complete an aborted WRITE when a new R2T would be sent

[Laurence Oberman]

On Mon, 2026-10-26 at 13:13 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Critical] scsi: target: iscsi: UAF of OOO cmd in
> iscsit_check_dataout_payload()
> --
> 
> commit 93e4d74df05b6ac07e171f5586d09d2036dc2494
> Author: Laurence Oberman <loberman@redhat.com>
> 
> scsi: target: iscsi: Complete an aborted WRITE when a new R2T would
> be sent
> 
> This commit completes an aborted WRITE command via
> iscsit_complete_aborted_cmd() in iscsit_check_dataout_payload() when
> iscsit_check_post_dataout() returns DATAOUT_SEND_R2T and no
> outstanding R2Ts
> remain.
> 
> > diff --git a/drivers/target/iscsi/iscsi_target.c
> > b/drivers/target/iscsi/iscsi_target.c
> > index aab9237a67768..3dad7f69f3bae 100644
> > --- a/drivers/target/iscsi/iscsi_target.c
> > +++ b/drivers/target/iscsi/iscsi_target.c
> > @@ -1669,6 +1669,22 @@ iscsit_check_dataout_payload(struct
> > iscsit_cmd *cmd, struct iscsi_data *hdr,
> >  	if ((rc == DATAOUT_NORMAL) || (rc ==
> > DATAOUT_WITHIN_COMMAND_RECOVERY))
> >  		return 0;
> >  	else if (rc == DATAOUT_SEND_R2T) {
> > +		/*
> > +		 * A new R2T would never be answered for an
> > aborted command.
> > +		 * The CMD_T_ABORTED handling in
> > __iscsit_check_dataout_hdr()
> > +		 * does not cover this: it only runs for DataOut
> > PDUs
> > +		 * belonging to R2Ts that were already issued, and
> > +		 * iscsit_dataout_update_r2t() has already
> > decremented
> > +		 * outstanding_r2ts for the burst that just
> > finished.  Guard
> > +		 * on outstanding_r2ts so that with
> > MaxOutstandingR2T > 1 the
> > +		 * completion is still left to the last R2T to
> > drain.
> > +		 */
> > +		if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
> > +			if (cmd->outstanding_r2ts < 1)
> > +				iscsit_complete_aborted_cmd(cmd);
> > +			return 0;
> > +		}
> 
> [Severity: Critical]
> Can completing the command here cause a use-after-free of cmd if
> ICF_OOO_CMDSN is set?
> 
> When ImmediateData=Yes and InitialR2T=No, a WRITE command can arrive
> with
> immediate data and expect additional unsolicited DataOut PDUs in the
> first
> burst followed by R2T-solicited data (payload_length <
> FirstBurstLength <
> se_cmd.data_length, so cmd->unsolicited_data == 1 and
> ICF_NON_IMMEDIATE_UNSOLICITED_DATA is set).
> 
> In drivers/target/iscsi/iscsi_target.c:iscsit_process_scsi_cmd(),
> transport_generic_new_cmd(&cmd->se_cmd) is called before CmdSN
> sequencing:
> 
> 	cmd->sense_reason = transport_generic_new_cmd(&cmd->se_cmd);
> 	if (cmd->sense_reason)
> 		return 1;
> 
> This places cmd->se_cmd on dev->queues[cpuid].state_list in
> TRANSPORT_WRITE_PENDING. When iscsit_get_immediate_data()
> subsequently calls
> iscsit_sequence_cmd() and cmd->cmd_sn > sess->exp_cmd_sn,
> drivers/target/iscsi/iscsi_target_erl1.c:iscsit_handle_ooo_cmdsn()
> queues
> an entry on sess->sess_ooo_cmdsn_list holding a raw pointer to cmd:
> 
> 	cmd->cmd_flags			|= ICF_OOO_CMDSN;
> 	...
> 	ooo_cmdsn->cmd			= cmd;
> 
> Because cmd->unsolicited_data is set, iscsit_get_immediate_data()
> then calls
> iscsit_set_unsolicited_dataout(cmd).
> 
> If a concurrent TMR aborts cmd while it is still waiting on
> sess->sess_ooo_cmdsn_list, __target_check_io_state() sets
> CMD_T_ABORTED on
> cmd->se_cmd. When the final unsolicited DataOut PDU of the first
> burst
> arrives, __iscsit_check_dataout_hdr() takes the cmd->unsolicited_data
> == 1
> path, and iscsit_dataout_post_crc_passed() clears cmd-
> >unsolicited_data = 0
> and returns DATAOUT_SEND_R2T.
> 
> In this new DATAOUT_SEND_R2T branch in
> iscsit_check_dataout_payload(),
> cmd->outstanding_r2ts is 0, so iscsit_complete_aborted_cmd(cmd) is
> called
> without checking ICF_OOO_CMDSN or removing cmd from
> sess->sess_ooo_cmdsn_list (unlike the adjacent
> DATAOUT_SEND_TO_TRANSPORT
> branch, which checks cmd->cmd_flags & ICF_OOO_CMDSN).
> 
> Once target_complete_cmd() schedules target_abort_work ->
> target_handle_abort(), cmd is completed and freed via
> iscsit_release_cmd().
> When the missing ExpCmdSN PDU later arrives,
> drivers/target/iscsi/iscsi_target_erl1.c:iscsit_execute_ooo_cmdsns()
> will
> dereference the dangling ooo_cmdsn->cmd pointer:
> 
> 		cmd = ooo_cmdsn->cmd;
> 		cmd->i_state = cmd->deferred_i_state;
> 		ooo_count++;
> 		sess->exp_cmd_sn++;
> 		...
> 		iscsit_remove_ooo_cmdsn(sess, ooo_cmdsn);
> 
> 		if (iscsit_execute_cmd(cmd, 1) < 0)
> 			return -1;
> 
> Should the DATAOUT_SEND_R2T abort path also account for commands with
> ICF_OOO_CMDSN set before completing cmd?

I will drop patch 2 for this series as this turns out out be a real
issue and follow up with a v4 for the series. Dropping patch 2 will not
affect the validity of the fix with the over 300 reset loop tests we
ran which resolved the original customer issue.

Sashiko complaints for patches 1 and 4 are pre-existing and wont be
handled in this series.

It has taken many weeks to get to stable outcome for a customer that
included multiple long vmcore analysis tasks and instrumentation.


Thanks
Laurence

[PATCH v3 3/4] scsi: target: iscsi: Claim the completion in __iscsit_check_dataout_hdr()

[Laurence Oberman]

Commit d5869dae5080 ("scsi: target: iscsi: Fix hang for aborted
WRITE_PENDING commands") made __iscsit_check_dataout_hdr() complete an
aborted WRITE once the final DataOut PDU for the last outstanding R2T has
been received. That call records nothing to say the command has been
completed.

target_complete_cmd() on a command that already has CMD_T_ABORTED set goes
to target_cmd_interrupted(), which queues target_abort_work and returns
without setting CMD_T_COMPLETE and without changing t_state. The command
therefore stays in TRANSPORT_WRITE_PENDING and on conn_cmd_list until
target_abort_work reaches iscsit_aborted_task(). If the connection drops in
that window, iscsit_release_commands_from_conn() finds the command still in
TRANSPORT_WRITE_PENDING and completes it a second time, which
re-initialises and re-queues an se_cmd->work that is already queued.

Route this site through iscsit_complete_aborted_cmd() so that the
completion is claimed and connection teardown leaves the command alone. The
helper makes the same iscsit_stop_dataout_timer() and target_complete_cmd()
calls when it takes the claim, so the completing path is unchanged.

Fixes: d5869dae5080 ("scsi: target: iscsi: Fix hang for aborted WRITE_PENDING commands")
Cc: stable@vger.kernel.org
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---

Notes:
    This depends on patch 1 of this series, which adds
    iscsit_complete_aborted_cmd().  d5869dae5080 has been backported to 5.10.y,
    5.15.y, 6.1.y, 6.6.y, 6.12.y, 6.18.y and 7.2.y, none of which carry the
    helper, so a backport to those branches needs patch 1 as well, or the claim
    open-coded.  Happy to send a standalone version for stable if you would
    rather not carry patch 1 there.

 drivers/target/iscsi/iscsi_target.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 3dad7f6..83184de 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1533,10 +1533,8 @@ __iscsit_check_dataout_hdr(struct iscsit_conn *conn, void *buf,
 		 */
 		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);
-			}
+			    --cmd->outstanding_r2ts < 1)
+				iscsit_complete_aborted_cmd(cmd);
 
 			return iscsit_dump_data_payload(conn, payload_length, 1);
 		}
-- 
2.43.0

[PATCH v3 4/4] scsi: target: iscsi: Claim the completion on the DataOut submit path

[Laurence Oberman]

When the final DataOut PDU for a WRITE has been received,
iscsit_check_dataout_payload() submits the command with
target_execute_cmd(). For a command that has already been aborted,
target_execute_cmd() calls target_cmd_interrupted() before it touches
t_state, so it queues target_abort_work and returns with the command still
in TRANSPORT_WRITE_PENDING and still on conn_cmd_list, recording nothing to
say that a completion has been started.

A connection drop before target_abort_work reaches iscsit_aborted_task()
then lets iscsit_release_commands_from_conn() complete the command a second
time.

Complete aborted commands through iscsit_complete_aborted_cmd() rather than
handing them to target_execute_cmd(). Both reach target_cmd_interrupted()
and queue target_abort_work; the work target_execute_cmd() does after that
test is not reached for an aborted command either way. The dataout timer
has already been stopped a few lines above, so the helper's call to
iscsit_stop_dataout_timer() is a no-op here.

Fixes: ea87981a0ee8 ("scsi: target: iscsi: Handle abort for WRITE_PENDING cmds")
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---

Notes:
    Not observed in the field; closed by inspection after the review of v2
    pointed out that this site has the same shape as the one in
    __iscsit_check_dataout_hdr().  I have deliberately left Cc: stable off,
    since the window predates d5869dae5080 and has never been seen -- I would
    rather it sat in mainline for a cycle first.  Happy to drop this patch if
    you would prefer the series to stop at patch 3.

 drivers/target/iscsi/iscsi_target.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 83184de..e6113a5 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1699,6 +1699,16 @@ iscsit_check_dataout_payload(struct iscsit_cmd *cmd, struct iscsi_data *hdr,
 		iscsit_stop_dataout_timer(cmd);
 		if (ooo_cmdsn)
 			return 0;
+		/*
+		 * target_execute_cmd() tests CMD_T_ABORTED before it touches
+		 * t_state, so for an aborted command it would queue
+		 * target_abort_work and return with the command still in
+		 * TRANSPORT_WRITE_PENDING and the completion unclaimed.
+		 */
+		if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
+			iscsit_complete_aborted_cmd(cmd);
+			return 0;
+		}
 		target_execute_cmd(&cmd->se_cmd);
 		return 0;
 	} else /* DATAOUT_CANNOT_RECOVER */
-- 
2.43.0

Re: [PATCH v3 4/4] scsi: target: iscsi: Claim the completion on the DataOut submit path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: target: iscsi: CMD_T_ABORTED race in iscsit_check_dataout_payload()
--

commit 106a0e34d325c477082712ef599ef2736992fc34
Author: Laurence Oberman <loberman@redhat.com>

scsi: target: iscsi: Claim the completion on the DataOut submit path

This commit checks for CMD_T_ABORTED in iscsit_check_dataout_payload() before
submitting the command to target_execute_cmd(), completing already-aborted
commands via iscsit_complete_aborted_cmd() so that completion_claimed is set
and connection teardown does not complete the command a second time.

> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 83184de284825..e6113a549e053 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -1699,6 +1699,16 @@ iscsit_check_dataout_payload(struct iscsit_cmd *cmd, struct iscsi_data *hdr,
>  		iscsit_stop_dataout_timer(cmd);
>  		if (ooo_cmdsn)
>  			return 0;
> +		/*
> +		 * target_execute_cmd() tests CMD_T_ABORTED before it touches
> +		 * t_state, so for an aborted command it would queue
> +		 * target_abort_work and return with the command still in
> +		 * TRANSPORT_WRITE_PENDING and the completion unclaimed.
> +		 */
> +		if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
> +			iscsit_complete_aborted_cmd(cmd);
> +			return 0;
> +		}
>  		target_execute_cmd(&cmd->se_cmd);

[Severity: High]
Can CMD_T_ABORTED still be set after the check in
iscsit_check_dataout_payload() but before target_execute_cmd() updates
t_state, leaving the completion unclaimed?

If CMD_T_ABORTED is not yet set at this check, iscsit_check_dataout_payload()
calls target_execute_cmd() with cmd->completion_claimed still false and
cmd->se_cmd.t_state still in TRANSPORT_WRITE_PENDING.

A concurrent task management request (such as ABORT_TASK via
core_tmr_abort_task() or LUN_RESET via core_tmr_drain_state_list()) calling
__target_check_io_state() can then set CMD_T_ABORTED before
target_execute_cmd() calls target_cmd_interrupted():

drivers/target/target_core_tmr.c:__target_check_io_state()
    spin_lock_irqsave(&se_cmd->t_state_lock, flags);
    ...
    se_cmd->transport_state |= CMD_T_ABORTED;

When target_execute_cmd() runs, it calls target_cmd_interrupted() before
taking t_state_lock to transition t_state to TRANSPORT_PROCESSING:

drivers/target/target_core_transport.c:target_execute_cmd()
    if (target_cmd_interrupted(cmd))
        return;

    spin_lock_irq(&cmd->t_state_lock);
    cmd->t_state = TRANSPORT_PROCESSING;
    cmd->transport_state |= CMD_T_ACTIVE | CMD_T_SENT;
    spin_unlock_irq(&cmd->t_state_lock);

Because target_cmd_interrupted() sees CMD_T_ABORTED, it queues
target_abort_work on target_completion_wq and returns true, so
target_execute_cmd() returns with t_state still in TRANSPORT_WRITE_PENDING
and completion_claimed still false.

If the connection then drops, iscsit_release_commands_from_conn() sees
TRANSPORT_WRITE_PENDING and claims the completion:

drivers/target/iscsi/iscsi_target.c:iscsit_release_commands_from_conn()
    if (cmd->se_cmd.t_state == TRANSPORT_WRITE_PENDING) {
        bool claimed = iscsit_claim_completion_locked(cmd);

        spin_unlock_irq(&se_cmd->t_state_lock);
        if (claimed)
            target_complete_cmd(&cmd->se_cmd,
                                SAM_STAT_TASK_ABORTED);
        continue;
    }

Wouldn't that call target_complete_cmd() and re-queue cmd->se_cmd.work while
target_abort_work is already queued or running?

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