Re: [PATCH v6 33/56] scsi: qla2xxx: Add abort command handling for 29xx series

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Potential use-after-free of the original SRB (`orig_sp`) during asynchronous abort completion if the original command completes concurrently.
--

--- Patch [33]: [PATCH v6 33/56] scsi: qla2xxx: Add abort command handling for 29xx series ---
commit 7e8bef4c0a160dfafa23b4b48905c50cc258d592
Author: Nilesh Javali <njavali@marvell.com>

scsi: qla2xxx: Add abort command handling for 29xx series

Extend the abort-IOCB code path to support the 29xx extended
abort_entry_24xx_ext structure alongside the existing abort_entry_24xx.

> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -3973,7 +3973,14 @@ qla24xx_abort_iocb_entry(scsi_qla_host_t *vha, struct req_que *req,
>  		return;
>  
>  	abt = &sp->u.iocb_cmd;
> -	abt->u.abt.comp_status = pkt->comp_status;
> +	/*
> +	 * abort_entry_24xx_ext overlays abort_entry_24xx through the
> +	 * nport_handle/comp_status union at offset 8, so reading
> +	 * comp_status is stride-agnostic and goes through the 24xx view.
> +	 */
> +	abt->u.abt.comp_status =
> +	    ((struct abort_entry_24xx *)pkt)->comp_status;
> +
>  	orig_sp = sp->cmd_sp;
>  	/* Need to pass original sp */
>  	if (orig_sp)

[Severity: High]
This isn't a bug introduced by this patch, but can orig_sp be accessed after
it has been freed here?

If an asynchronous command (e.g., SRB_LOGIN_CMD) times out, the timeout handler
qla2x00_async_iocb_timeout() creates an abort SRB holding a pointer to the
original SRB (abt_sp->cmd_sp = sp) without incrementing its kref. It then
drops its timer reference on the original SRB.

If the firmware completes the original command concurrently, the ISR will
process the completion and drop the initial reference. This can drop the
refcount to 0, freeing the original SRB.

Later, when the abort IOCB completes, qla24xx_abort_iocb_entry() reads
orig_sp = sp->cmd_sp (which now points to freed memory) and passes it to
qla_nvme_abort_process_comp_status(), where dereferencing orig_sp->fcport or
orig_sp->type would cause a use-after-free.

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