Re: [PATCH 2/2] scsi: fnic: free the NVMe port template from the delete callback

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Relocating `kfree(iport->nv_tmpl)` to the asynchronous `nvfnic_local_port_delete()` callback introduces a severe arbitrary free and memory corruption vulnerability if the unregister process times out.

Pre-existing issues:
- [Critical] The driver's bounded timeout design for NVMe FC unregistration leaves the `fnic` structure vulnerable to a Use-After-Free when deferred callbacks execute.
--

--- Patch [2]: [PATCH 2/2] scsi: fnic: free the NVMe port template from the delete callback ---
commit 99ed8cf7442e7f3171f98e582e611d65cec9862a
Author: Linmao Li <lilinmao@kylinos.cn>

scsi: fnic: free the NVMe port template from the delete callback

This commit attempts to fix a dangling pointer issue by moving the
freeing of the NVMe port template to the deferred local port delete callback,
tying its lifetime to the transport's use instead of a bounded timeout.

> diff --git a/drivers/scsi/fnic/fnic_nvme.c b/drivers/scsi/fnic/fnic_nvme.c
> index 00d9d5d439a38..a8b7b7d40b9ab 100644
> --- a/drivers/scsi/fnic/fnic_nvme.c
> +++ b/drivers/scsi/fnic/fnic_nvme.c
> @@ -1243,7 +1243,6 @@ void nvfnic_nvme_unload(struct fnic *fnic)
>  				WARN_ON(1);
>  			}
>  			iport->flags &= ~FNIC_LPORT_NVME_REGISTERED;
> -			kfree(iport->nv_tmpl);
>  		}
>  	}

[ ... ]

> @@ -1669,6 +1668,9 @@ void nvfnic_local_port_delete(struct nvme_fc_local_port *lport)
>  	FNIC_NVME_DBG(KERN_INFO, fnic, "lport delete 0x%x\n",
>  		      iport->fcid);
>  
> +	kfree(iport->nv_tmpl);

[Severity: Critical]
Does moving this kfree() here introduce an arbitrary free if the unregister
process times out?

If the bounded timeout in nvfnic_nvme_unload() expires:

nvfnic_nvme_unload()
    time_remain = wait_for_completion_timeout(fnic->nvme_lport_unreg_done,
                                              msecs_to_jiffies(time_wait));

The driver can abandon the wait and eventually free the fnic structure, which
embeds the iport. When the transport later invokes this deferred callback,
the iport pointer will point to freed memory.

Could this cause the kernel to execute an arbitrary free by passing a
corrupted iport->nv_tmpl pointer to kfree()?

> +	iport->nv_tmpl = NULL;
> +
>  	spin_lock_irqsave(&fnic->fnic_lock, flags);

[Severity: Critical]
This is a pre-existing issue, but does the bounded timeout design for NVMe FC
unregistration leave the fnic structure vulnerable to a use-after-free?

Similar to the issue above, if the wait_for_completion_timeout() in
nvfnic_nvme_unload() expires and the driver frees the fnic structure, this
callback will attempt to acquire fnic->fnic_lock on freed memory.

Since the transport can asynchronously invoke the localport_delete callback
long after the driver has given up waiting, is it safe to dereference the fnic
pointer here?

>  	if (fnic->nvme_lport_unreg_done)
>  		complete(fnic->nvme_lport_unreg_done);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819114242.3598034-1-lilinmao@kylinos.cn?part=2