Re: [PATCH v2 29/33] ibmvfc: implement nvme-fc LS submission transport callback

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] timer_delete is used instead of timer_delete_sync during event purging, leading to a Use-After-Free in the timer callback during driver unload.
--

--- Patch [29]: [PATCH v2 29/33] ibmvfc: implement nvme-fc LS submission transport callback ---
commit 307222be453ed384bc1c2ec92213263c1df79a6a
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: implement nvme-fc LS submission transport callback
    
NVMe FC Link Service commands are required to use the ibmvfc_passthru MAD.
This patch initializes a passthru mad for the target port and sends FC4_LS
commands via the primary CRQ.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> index ecf9f4c0549c2..cf72f0682297b 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
[ ... ]
> @@ -62,10 +62,99 @@ static void ibmvfc_nvme_delete_queue(struct nvme_fc_local_port *lport, unsigned
[ ... ]
>  static int ibmvfc_nvme_ls_req(struct nvme_fc_local_port *lport,
>  			      struct nvme_fc_remote_port *rport,
>  			      struct nvmefc_ls_req *ls_req)
>  {
> +	struct ibmvfc_host *vhost = lport->private;
> +	struct ibmvfc_target *tgt = rport->private;
> +	struct ibmvfc_passthru_mad *mad;
> +	struct ibmvfc_event *evt;
[ ... ]
> +	ibmvfc_dbg(vhost, "nvme_ls_req\n");
> +	if (ibmvfc_send_event(evt, vhost, IBMVFC_FC4_LS_PLUS_CANCEL_TIMEOUT)) {

[Severity: High]
This isn't a bug introduced by this patch, but calling ibmvfc_send_event() sets
up a timer for the event.

During driver removal, the driver calls ibmvfc_purge_requests(), which fails
pending commands and cancels their timers using the non-synchronous
timer_delete():

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_fail_request() {
	...
	timer_delete(&evt->timer);
	...
}

If a timer is already executing concurrently on another CPU, timer_delete()
returns immediately without waiting for it to finish. The driver unload sequence
then proceeds to call ibmvfc_free_mem(), which completely frees the event pool.

Meanwhile, the still-executing ibmvfc_timeout callback dereferences the freed
evt structure:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_timeout() {
	struct ibmvfc_event *evt = container_of(timer, struct ibmvfc_event, timer);
	struct ibmvfc_host *vhost = evt->vhost;
	...
}

Can this lead to a use-after-free during driver unload if there are pending
requests that time out concurrently? Should the teardown path use
timer_delete_sync() to wait for executing timers to finish before freeing the
event pool?

> +		kref_put(&tgt->kref, ibmvfc_release_tgt);
> +		return -ENXIO;
> +	}
> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723000149.969416-1-tyreld@linux.ibm.com?part=29