Re: [PATCH v14 07/11] scsi: ibmvfc: extend async event handlers for async sub-CRQ events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: UAF and module unload crash due to untracked async work
--

commit e22506651b699f9db9776870465be492a807fad7
Author: Dave Marquardt <davemarq@linux.ibm.com>

scsi: ibmvfc: extend async event handlers for async sub-CRQ events

This commit refactors async event handling to support both traditional async
CRQs and new asynchronous sub-queue CRQs. It introduces a tagged union
struct ibmvfc_async_crq_event to hold either event type and updates the
corresponding handlers and FPIN processing logic.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 9c710897cd546..87ee19057551e 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c

[ ... ]

> @@ -3629,14 +3711,14 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
>  			}
>  			INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
>  			aw->vhost = vhost;
> -			aw->crq = *crq;
> +			aw->event = *ae;
>  			schedule_work(&aw->async_work_s);

[Severity: High]
Does this code leave the scheduled work item untracked without a cancelation
path during module unload or device removal?

Looking at ibmvfc_remove(), it executes without flushing the system workqueue
or canceling these specific work items. It then calls ibmvfc_free_mem() which
unconditionally destroys vhost->tgt_pool via mempool_destroy():

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_remove() {
    ...
    flush_work(&vhost->rport_add_work_q);
    fc_remove_host(vhost->host);
    scsi_remove_host(vhost->host);
    ...
    ibmvfc_free_mem(vhost);
    spin_lock(&ibmvfc_driver_lock);
    list_del(&vhost->queue);
    spin_unlock(&ibmvfc_driver_lock);
    scsi_host_put(vhost->host);
}

If the work executes after the target pool is destroyed,
ibmvfc_process_async_work() looks up a target, takes a reference, and
eventually calls ibmvfc_release_tgt(), which triggers a use-after-free
by calling mempool_free() on the already-destroyed pool:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_release_tgt() {
    struct ibmvfc_target *tgt = container_of(kref, struct ibmvfc_target, kref);
    mempool_free(tgt, tgt->vhost->tgt_pool);
}

Furthermore, since scsi_host_get() does not hold a module reference, rmmod
could complete while the work is still queued. Could this lead to the system
workqueue executing unmapped memory and causing a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-ibmvfc-fpin-support-v14-0-a0b063b8d825@linux.ibm.com?part=7