Re: [PATCH v14 09/11] scsi: ibmvfc: extend channel reg/dereg helpers for async sub-CRQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] scsi: ibmvfc: race reading uninitialized hwq_id on early interrupt
--

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

scsi: ibmvfc: extend channel reg/dereg helpers for async sub-CRQ
    
This commit extends ibmvfc_register_channel() and ibmvfc_deregister_channel()
to handle IBMVFC_PROTO_ASYNC channels. It removes the __maybe_unused
annotation from ibmvfc_interrupt_async_subq and updates error messages to
include the channel protocol.

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

[ ... ]

> @@ -6882,7 +6901,8 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
>  	rc = request_irq(scrq->irq, scrq->handler, 0, scrq->name, scrq);

[Severity: Low]
Is there a potential race condition here when the interrupt is requested?

If a hardware interrupt triggers immediately after request_irq() enables it,
the newly bound handler ibmvfc_interrupt_async_subq() could run before
scrq->hwq_id is initialized:

ibmvfc_register_channel()
    ...
    rc = request_irq(scrq->irq, scrq->handler, 0, scrq->name, scrq);
    ...
    scrq->hwq_id = index;

If an early interrupt happens, the handler calls ibmvfc_toggle_scrq_irq(),
which uses scrq->hwq_id for error logging if the hypervisor call fails.

Could this result in reading an uninitialized (zeroed) scrq->hwq_id in the
dev_err() message? Would it be safer to initialize scrq->hwq_id before
calling request_irq()?

>  
>  	if (rc) {
> -		dev_err(dev, "Couldn't register sub-crq[%d] irq\n", index);
> +		dev_err(dev, "Couldn't register protocol (%d) sub-crq[%d] irq\n",
> +			channels->protocol, index);
>  		irq_dispose_mapping(scrq->irq);
>  		scrq->irq = 0;
>  		goto irq_failed;

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