Re: [PATCH v12 04/11] scsi: ibmvfc: add basic FPIN support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: ibmvfc: unbalanced scsi_host_put() and UAF in kunit tests due to ignored scsi_host_get() result
--

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

scsi: ibmvfc: add basic FPIN support

This commit implements support for a basic level of Fabric Performance
Impact Notifications (FPIN) in the ibmvfc driver. It adds an async
event handler for IBMVFC_AE_FPIN events that offloads FPIN processing
to a dedicated workqueue, and introduces KUnit tests for validation.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index cf775f0673a0f..e0fa9b547cbfe 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c

[ ... ]

> @@ -7120,5 +7393,21 @@ static void __exit ibmvfc_module_exit(void)
>  	fc_release_transport(ibmvfc_transport_template);
>  }
>  
> +#if IS_ENABLED(CONFIG_KUNIT)
> +VISIBLE_IF_KUNIT struct ibmvfc_host *ibmvfc_get_first_vhost(void)
> +{
> +	struct ibmvfc_host *vhost = NULL;
> +
> +	spin_lock(&ibmvfc_driver_lock);
> +	if (!list_empty(&ibmvfc_head))
> +		vhost = list_first_entry(&ibmvfc_head, struct ibmvfc_host, queue);
> +	if (vhost)
> +		scsi_host_get(vhost->host);

[Severity: Medium]
Does this code need to check the return value of scsi_host_get()? 

If it fails (for example, if the module is being concurrently unloaded and
the host state becomes SHOST_DEL before vhost is removed from ibmvfc_head),
this function will ignore the failure and return an unreferenced vhost
pointer to the caller.

> +	spin_unlock(&ibmvfc_driver_lock);
> +	return vhost;
> +}
> +EXPORT_SYMBOL_IF_KUNIT(ibmvfc_get_first_vhost);
> +#endif

[ ... ]

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
> new file mode 100644
> index 0000000000000..7cc9d423d4a79
> --- /dev/null
> +++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c

[ ... ]

> +static void ibmvfc_async_fpin_test(struct kunit *test)
> +{

[ ... ]

> +	KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED],
> +			post[IBMVFC_AE_FPIN_CONGESTION_CLEARED]);
> +
> +	kref_put(&tgt->kref, ibmvfc_release_tgt);
> +	put_device(&rport->dev);
> +	scsi_host_put(vhost->host);

[Severity: Medium]
Could this unconditional scsi_host_put() cause a refcount underflow if the
earlier scsi_host_get() call failed?

The test might also perform a use-after-free by accessing members of the
unreferenced vhost if it was already freed concurrently by the unload path.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-ibmvfc-fpin-support-v12-0-53a3fff7d77b@linux.ibm.com?part=4