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

[Leon Romanovsky <leon@kernel.org>]

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