Re: [PATCH v6 1/8] lpfc: Fix use-after-free in lpfc_cmpl_ct_cmd_vmid

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: lpfc: evaluation of uninitialized response payload on transport error
--

commit c46419e38a4d473126adc74a9b8003eb31bdd1e9
Author: Nigel Kirkland <nkirkland2304@gmail.com>

lpfc: Fix use-after-free in lpfc_cmpl_ct_cmd_vmid

This commit resolves a use-after-free issue in lpfc_cmpl_ct_cmd_vmid by
removing an early call to lpfc_ct_free_iocb for SLI_CTAS_DALLAPP_ID commands.
The previous code freed the IOCB and then continued to access the response
buffer.

> diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
> index 0734ab3be3e30..f709e50875770 100644
> --- a/drivers/scsi/lpfc/lpfc_ct.c
> +++ b/drivers/scsi/lpfc/lpfc_ct.c
> @@ -3576,8 +3576,6 @@ lpfc_cmpl_ct_cmd_vmid(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
>  	int i;
>  
>  	cmd = be16_to_cpu(ctcmd->CommandResponse.bits.CmdRsp);
> -	if (cmd == SLI_CTAS_DALLAPP_ID)
> -		lpfc_ct_free_iocb(phba, cmdiocb);
>  
>  	if (lpfc_els_chk_latt(vport) || get_job_ulpstatus(phba, rspiocb)) {
>  		if (cmd != SLI_CTAS_DALLAPP_ID)

[Severity: Medium]
Does this code evaluate uninitialized memory if there is a transport error?

If get_job_ulpstatus() indicates a transport error, the device did not populate
the response buffer. Since the error handler is intentionally bypassed for
SLI_CTAS_DALLAPP_ID:

drivers/scsi/lpfc/lpfc_ct.c:lpfc_cmpl_ct_cmd_vmid() {
	if (lpfc_els_chk_latt(vport) || get_job_ulpstatus(phba, rspiocb)) {
		if (cmd != SLI_CTAS_DALLAPP_ID)
			goto free_res;
	}
	...

The code will then fall through to the next check:

	if (be16_to_cpu(rsp) == SLI_CT_RESPONSE_FS_RJT) {
	...

And evaluate rsp, ctrsp->ReasonCode, and ctrsp->Explanation from the outp->virt
buffer. Since lpfc_alloc_ct_rsp() allocates this buffer via lpfc_mbuf_alloc()
without zero-initialization, this reads uninitialized memory.

Could this lead to non-deterministic behavior, such as spuriously retrying the
command or incorrectly assuming success and deregistering all VMID app IDs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013001648.62289-1-nkirkland2304@gmail.com?part=1