[PATCH] scsi: target: Clear cmd_cnt when initial counter enrollment fails

[PATCH] scsi: target: Clear cmd_cnt when initial counter enrollment fails

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

When target_get_sess_cmd() fails during session shutdown because
percpu_ref_tryget_live() returns false, the command keeps the
se_cmd->cmd_cnt pointer that __target_init_cmd() assigned earlier without
owning a reference. Final release through target_release_cmd_kref() then
issues an unmatched percpu_ref_put().

Commit 8e288be8606a ("scsi: target: Pass in cmd counter to use during cmd
setup") moved the cmd_cnt assignment ahead of the reference acquisition.
Clear se_cmd->cmd_cnt whenever the initial target_get_sess_cmd() fails in
target_init_cmd() and target_submit_tmr(), so release performs exactly one
matching put per acquired reference.

Fixes: 8e288be8606a ("scsi: target: Pass in cmd counter to use during cmd setup")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Found by sashiko.
---
 drivers/target/target_core_transport.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index fad03a15c969..dcfe94594916 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1734,6 +1734,7 @@ int target_init_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
 		    u32 data_length, int task_attr, int data_dir, int flags)
 {
 	struct se_portal_group *se_tpg;
+	int ret;
 
 	se_tpg = se_sess->se_tpg;
 	BUG_ON(!se_tpg);
@@ -1763,7 +1764,11 @@ int target_init_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
 	 * necessary for fabrics using TARGET_SCF_ACK_KREF that expect a second
 	 * kref_put() to happen during fabric packet acknowledgement.
 	 */
-	return target_get_sess_cmd(se_cmd, flags & TARGET_SCF_ACK_KREF);
+	ret = target_get_sess_cmd(se_cmd, flags & TARGET_SCF_ACK_KREF);
+	if (ret)
+		se_cmd->cmd_cnt = NULL;
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(target_init_cmd);
 
@@ -2039,8 +2044,10 @@ int target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
 	 * allocation failure.
 	 */
 	ret = core_tmr_alloc_req(se_cmd, fabric_tmr_ptr, tm_type, gfp);
-	if (ret < 0)
+	if (ret < 0) {
+		se_cmd->cmd_cnt = NULL;
 		return -ENOMEM;
+	}
 
 	if (tm_type == TMR_ABORT_TASK)
 		se_cmd->se_tmr_req->ref_task_tag = tag;
@@ -2048,6 +2055,7 @@ int target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
 	/* See target_submit_cmd for commentary */
 	ret = target_get_sess_cmd(se_cmd, flags & TARGET_SCF_ACK_KREF);
 	if (ret) {
+		se_cmd->cmd_cnt = NULL;
 		core_tmr_release_req(se_cmd->se_tmr_req);
 		return ret;
 	}

---
base-commit: 74f49255492a62658f36bf2578d7916f1c6ffad1
change-id: 20260721-reference-count-underflow-in-target-a7e580e549ca

Best regards,
--  
Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH] scsi: target: Clear cmd_cnt when initial counter enrollment fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] iSCSI target ignores target_get_sess_cmd() failure, causing reference count underflow and use-after-free
- [High] qla2xxx target driver leaks cmd_cnt->refcnt references on every SRR processing event
--

--- Patch [1]: [PATCH] scsi: target: Clear cmd_cnt when initial counter enrollment fails ---
commit 91357eb9be8775486a1fba8ba21b9b933e790d5b
Author: Leon Romanovsky <leon@kernel.org>

scsi: target: Clear cmd_cnt when initial counter enrollment fails

When target_get_sess_cmd() fails during session shutdown, the command keeps
the se_cmd->cmd_cnt pointer without owning a reference. Final release through
target_release_cmd_kref() then issues an unmatched percpu_ref_put(). This
commit clears se_cmd->cmd_cnt whenever the initial target_get_sess_cmd()
fails so release performs exactly one matching put per acquired reference.

> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index fad03a15c969e..dcfe945949167 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c

[ ... ]

> @@ -1763,7 +1764,11 @@ int target_init_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
>  	 * necessary for fabrics using TARGET_SCF_ACK_KREF that expect a second
>  	 * kref_put() to happen during fabric packet acknowledgement.
>  	 */
> -	return target_get_sess_cmd(se_cmd, flags & TARGET_SCF_ACK_KREF);
> +	ret = target_get_sess_cmd(se_cmd, flags & TARGET_SCF_ACK_KREF);
> +	if (ret)
> +		se_cmd->cmd_cnt = NULL;
> +
> +	return ret;
>  }

[Severity: Critical]
This is a pre-existing issue, but I noticed the iSCSI target driver manually
calls target_get_sess_cmd() and ignores its return value.

If target_get_sess_cmd() fails (e.g. during session shutdown), it drops the
cmd_kref back down to 1. Since iscsit_setup_scsi_cmd() ignores this failure:

drivers/target/iscsi/iscsi_target.c:iscsit_setup_scsi_cmd() {
    ...
    target_get_sess_cmd(&cmd->se_cmd, true);
    ...
}

The same happens in the TMR path:

drivers/target/iscsi/iscsi_target.c:iscsit_handle_task_mgt_cmd() {
    ...
    target_get_sess_cmd(&cmd->se_cmd, true);
    ...
}

When the command completes, iscsit_send_status() calls target_put_sess_cmd(),
which drops the cmd_kref to 0 and prematurely frees the command. This triggers
an unmatched percpu_ref_put(&cmd_cnt->refcnt) and underflows the session
reference. Later, iscsit_free_cmd() causes a second target_put_sess_cmd() on
the freed command, resulting in a use-after-free.

Can this be triggered by a remote unprivileged initiator sending commands
concurrently with session teardown?


[Severity: High]
This isn't a bug introduced by this patch, but looking at how the qla2xxx
driver uses target_get_sess_cmd(), could there be a reference leak during SRR
processing?

The qla2xxx driver uses target_get_sess_cmd() to acquire an extra reference
for Sequence Retransmission Request (SRR) processing:

drivers/scsi/qla2xxx/tcm_qla2xxx.c:tcm_qla2xxx_get_cmd_ref() {
    ...
    return target_get_sess_cmd(&cmd->se_cmd, true);
}

This increments both cmd_kref and cmd_cnt->refcnt:

drivers/target/target_core_transport.c:target_get_sess_cmd() {
    ...
    if (se_cmd->cmd_cnt) {
        if (!percpu_ref_tryget_live(&se_cmd->cmd_cnt->refcnt))
    ...
}

However, when releasing the extra reference, the driver only drops cmd_kref:

drivers/scsi/qla2xxx/tcm_qla2xxx.c:tcm_qla2xxx_put_cmd_ref() {
    ...
    target_put_sess_cmd(&cmd->se_cmd);
}

The extra cmd_cnt->refcnt increment is never balanced because
target_release_cmd_kref() is only called once at the end of the command
lifecycle.

Does this permanently leak cmd_cnt->refcnt references and cause
target_wait_for_sess_cmds() to hang infinitely during session teardown?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-reference-count-underflow-in-target-v1-1-63ab664f12fd@nvidia.com?part=1

Re: [PATCH] scsi: target: Clear cmd_cnt when initial counter enrollment fails

[Leon Romanovsky]

On Wed, Jul 22, 2026 at 06:48:42AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] iSCSI target ignores target_get_sess_cmd() failure, causing reference count underflow and use-after-free
> - [High] qla2xxx target driver leaks cmd_cnt->refcnt references on every SRR processing event

Interesting, I run clade review with Chris's prompts and got this
verdict:

False positives eliminated during analysis: (1) “fix belongs in target_get_sess_cmd()” — refuted by the qla2xxx SRR additional-reference case;
(2) “iscsi/qla2xxx callers left unfixed” — refuted by iscsi teardown ordering and by qla2xxx/tcm_fc/efct using tag/pool free rather than target_release_cmd_kref.

Thanks

> --
> 
> --- Patch [1]: [PATCH] scsi: target: Clear cmd_cnt when initial counter enrollment fails ---
> commit 91357eb9be8775486a1fba8ba21b9b933e790d5b
> Author: Leon Romanovsky <leon@kernel.org>
> 
> scsi: target: Clear cmd_cnt when initial counter enrollment fails
> 
> When target_get_sess_cmd() fails during session shutdown, the command keeps
> the se_cmd->cmd_cnt pointer without owning a reference. Final release through
> target_release_cmd_kref() then issues an unmatched percpu_ref_put(). This
> commit clears se_cmd->cmd_cnt whenever the initial target_get_sess_cmd()
> fails so release performs exactly one matching put per acquired reference.
> 
> > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> > index fad03a15c969e..dcfe945949167 100644
> > --- a/drivers/target/target_core_transport.c
> > +++ b/drivers/target/target_core_transport.c
> 
> [ ... ]
> 
> > @@ -1763,7 +1764,11 @@ int target_init_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
> >  	 * necessary for fabrics using TARGET_SCF_ACK_KREF that expect a second
> >  	 * kref_put() to happen during fabric packet acknowledgement.
> >  	 */
> > -	return target_get_sess_cmd(se_cmd, flags & TARGET_SCF_ACK_KREF);
> > +	ret = target_get_sess_cmd(se_cmd, flags & TARGET_SCF_ACK_KREF);
> > +	if (ret)
> > +		se_cmd->cmd_cnt = NULL;
> > +
> > +	return ret;
> >  }
> 
> [Severity: Critical]
> This is a pre-existing issue, but I noticed the iSCSI target driver manually
> calls target_get_sess_cmd() and ignores its return value.
> 
> If target_get_sess_cmd() fails (e.g. during session shutdown), it drops the
> cmd_kref back down to 1. Since iscsit_setup_scsi_cmd() ignores this failure:
> 
> drivers/target/iscsi/iscsi_target.c:iscsit_setup_scsi_cmd() {
>     ...
>     target_get_sess_cmd(&cmd->se_cmd, true);
>     ...
> }
> 
> The same happens in the TMR path:
> 
> drivers/target/iscsi/iscsi_target.c:iscsit_handle_task_mgt_cmd() {
>     ...
>     target_get_sess_cmd(&cmd->se_cmd, true);
>     ...
> }
> 
> When the command completes, iscsit_send_status() calls target_put_sess_cmd(),
> which drops the cmd_kref to 0 and prematurely frees the command. This triggers
> an unmatched percpu_ref_put(&cmd_cnt->refcnt) and underflows the session
> reference. Later, iscsit_free_cmd() causes a second target_put_sess_cmd() on
> the freed command, resulting in a use-after-free.
> 
> Can this be triggered by a remote unprivileged initiator sending commands
> concurrently with session teardown?
> 
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but looking at how the qla2xxx
> driver uses target_get_sess_cmd(), could there be a reference leak during SRR
> processing?
> 
> The qla2xxx driver uses target_get_sess_cmd() to acquire an extra reference
> for Sequence Retransmission Request (SRR) processing:
> 
> drivers/scsi/qla2xxx/tcm_qla2xxx.c:tcm_qla2xxx_get_cmd_ref() {
>     ...
>     return target_get_sess_cmd(&cmd->se_cmd, true);
> }
> 
> This increments both cmd_kref and cmd_cnt->refcnt:
> 
> drivers/target/target_core_transport.c:target_get_sess_cmd() {
>     ...
>     if (se_cmd->cmd_cnt) {
>         if (!percpu_ref_tryget_live(&se_cmd->cmd_cnt->refcnt))
>     ...
> }
> 
> However, when releasing the extra reference, the driver only drops cmd_kref:
> 
> drivers/scsi/qla2xxx/tcm_qla2xxx.c:tcm_qla2xxx_put_cmd_ref() {
>     ...
>     target_put_sess_cmd(&cmd->se_cmd);
> }
> 
> The extra cmd_cnt->refcnt increment is never balanced because
> target_release_cmd_kref() is only called once at the end of the command
> lifecycle.
> 
> Does this permanently leak cmd_cnt->refcnt references and cause
> target_wait_for_sess_cmds() to hang infinitely during session teardown?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260722-reference-count-underflow-in-target-v1-1-63ab664f12fd@nvidia.com?part=1