Re: [PATCH v9 7/9] scsi: ibmvfc: fix IRQ leak and guard deregister on channel reg failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Stale or uninitialized scrq->cookie is not zeroed on registration error paths, leading to hypervisor double-free vulnerabilities that can destroy actively reallocated resources (like NVMe channels).
--

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

    scsi: ibmvfc: fix IRQ leak and guard deregister on channel reg failure
    
    If request_irq() fails during ibmvfc_register_channel(), the error path
    disposes of the IRQ mapping via irq_dispose_mapping() but leaves scrq->irq
    populated with the stale virq number. A subsequent call to
    ibmvfc_deregister_channel() during teardown or reset cleanup then
    unconditionally invokes free_irq() and irq_dispose_mapping() on the stale
    IRQ descriptor, triggering kernel warnings.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index fa312a58471fa..8eeb125ee13b8 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> @@ -6933,9 +6934,11 @@ static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
>  
>  	ENTER;
>  
> -	free_irq(scrq->irq, scrq);
> -	irq_dispose_mapping(scrq->irq);
> -	scrq->irq = 0;
> +	if (scrq->irq) {
> +		free_irq(scrq->irq, scrq);
> +		irq_dispose_mapping(scrq->irq);
> +		scrq->irq = 0;
> +	}
>  
>  	do {
>  		rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,

[Severity: High]
This is a pre-existing issue, but does the sub-CRQ cookie need a similar
check to prevent a double-free against the hypervisor?

If h_reg_sub_crq() fails during ibmvfc_register_channel(), scrq->cookie
is populated with uninitialized stack data from retbuf:

h_reg_sub_crq() {
	unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
	long rc;

	rc = plpar_hcall(H_REG_SUB_CRQ, retbuf, unit_address, ioba, length);
	*cookie = retbuf[0];
        ...
}

Additionally, if request_irq() fails in ibmvfc_register_channel(), the error
path calls H_FREE_SUB_CRQ but leaves the stale cookie in scrq->cookie:

ibmvfc_register_channel() {
        ...
irq_failed:
	do {
		hcall_rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
	} while (hcall_rc == H_BUSY || H_IS_LONG_BUSY(hcall_rc));
        ...
}

Since ibmvfc_deregister_channel() is unconditionally called for all queues
during teardown, it appears this could pass uninitialized data or a stale
cookie to a second H_FREE_SUB_CRQ call, potentially destroying unrelated,
actively reallocated sub-CRQ resources.

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