Re: [PATCH v2 14/20] scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: ls_abort validity check defeated by stale ls_req
--

commit 0d745a9ce0a68c1aa95c0283c60b12feb6ac43d9
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

scsi: ibmvfc: fix UAF and stall in NVMe LS abort callback

This commit fixes a use-after-free and a blocking wait in the NVMe LS abort
callback. It moves event validation inside the host_lock and replaces the
synchronous wait with an asynchronous callback.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> index d23e5f31f8b51..475177cda103a 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-nvme.c
[ ... ]
> @@ -202,34 +204,36 @@ static void ibmvfc_nvme_ls_abort(struct nvme_fc_local_port *lport,
>  {
>  	struct ibmvfc_host *vhost = lport->private;
>  	struct ibmvfc_target *tgt = rport->private;
> -	struct ibmvfc_event *evt;
> -	union ibmvfc_iu rsp;
> +	struct ibmvfc_event *evt, *abt_evt;
>  	unsigned long flags;
> -	u16 status = IBMVFC_MAD_CRQ_ERROR;
> +
> +	spin_lock_irqsave(&vhost->host->host_lock, flags);
> +
> +	/*
> +	 * If the original LS has already completed naturally, abt_evt will
> +	 * have been freed back to the pool (evt->free set to 1 under
> +	 * host_lock by ibmvfc_free_event()).  Nothing left to cancel.
> +	 */
> +	abt_evt = ls_abort->private;
> +	if (!abt_evt || atomic_read(&abt_evt->free) || abt_evt->ls_req != ls_abort) {

[Severity: High]
If an LS request fails during send, ibmvfc_vfc_eh_done() frees the event without
clearing evt->ls_req:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_vfc_eh_done() {
	...
	} else if (ls_req) {
		ls_req->done(ls_req, -EIO);
		kref_put(&evt->tgt->kref, ibmvfc_release_tgt);
	}

	ibmvfc_free_event(evt);
}

If this event is subsequently reallocated for a new command, evt->free
becomes 0 but evt->ls_req remains stale.

If the NVMe core then calls ibmvfc_nvme_ls_abort() concurrently
(before the new command calls ibmvfc_init_event()), wouldn't this
validation bypass the check since abt_evt->ls_req still matches
ls_abort?

If the check is bypassed, the abort path will dereference abt_evt->tgt
which was already released by the error path, potentially causing a
use-after-free or sending an unintended abort MAD for an active command.

> +		spin_unlock_irqrestore(&vhost->host->host_lock, flags);
> +		return;
> +	}

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