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

在 2026/8/19 19:52, [email protected] 写道:
> 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.
The bot is right.  Please drop this patch; 1/2 is independent and still
stands on its own.

If the wait times out, fnic_remove() continues and eventually frees fnic,
which also frees the embedded iport still referenced by lport->private.
A later ->localport_delete() callback would therefore read nv_tmpl from
freed memory before passing it to kfree().

The underlying problem is that the bounded wait allows fnic to be freed
while the NVMe/FC transport still holds pointers into it.  I will revisit
that teardown separately.

Sorry for the noise on 2/2.


Thanks,
Linmao

>
> 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 <[email protected]>
>
> 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);