Re: [PATCH v5 51/56] scsi: qla2xxx: Initialize NVMe abort_work once at submission

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent calls to qla_nvme_fcp_abort or qla_nvme_ls_abort leak cmd_kref references because the return value of schedule_work() is ignored, leading to a permanent transport hang.
--

--- Patch [51]: [PATCH v5 51/56] scsi: qla2xxx: Initialize NVMe abort_work once at submission ---
commit ae623a4cb67f2592c5287e656c5d7842f196c53f
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Initialize NVMe abort_work once at submission

This commit moves the initialization of priv->abort_work from the abort path to
the command submission path. This prevents workqueue corruption when concurrent
aborts are issued for the same command.

> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index 0038b6274d444..3b2f255a5d7d4 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -463,7 +463,6 @@ static void qla_nvme_ls_abort(struct nvme_fc_local_port *lport,
>  	}
>  	spin_unlock_irqrestore(&priv->cmd_lock, flags);
>  
> -	INIT_WORK(&priv->abort_work, qla_nvme_abort_work);
>  	schedule_work(&priv->abort_work);

[Severity: High]
Does this code leak the cmd_kref reference if schedule_work() returns false?

Since INIT_WORK() was removed, schedule_work() can now return false if the work
item is already pending (e.g., from concurrent aborts). However, a reference was
unconditionally acquired earlier in the function via kref_get_unless_zero().

If schedule_work() returns false, the work function won't execute an extra time
to drop this reference, which could permanently hang NVMe-FC transport teardown.

>  }
>  

[ ... ]

> @@ -545,7 +545,6 @@ static void qla_nvme_fcp_abort(struct nvme_fc_local_port *lport,
>  	}
>  	spin_unlock_irqrestore(&priv->cmd_lock, flags);
>  
> -	INIT_WORK(&priv->abort_work, qla_nvme_abort_work);
>  	schedule_work(&priv->abort_work);

[Severity: High]
This appears to have the exact same reference leak pattern as qla_nvme_ls_abort()
when schedule_work() returns false. Should we drop the kref if schedule_work()
fails to queue the work?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717152904.3155730-1-njavali@marvell.com?part=51