Linux SCSI subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Dave Marquardt" <davemarq@linux.ibm.com>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events
Date: Fri, 10 Jul 2026 19:32:20 +0000	[thread overview]
Message-ID: <20260710193220.A227C1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260710-ibmvfc-fpin-support-v4-7-ef031ac19520@linux.ibm.com>

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

Pre-existing issues:
- [High] Failure to clear `scrq->irq` on error and unconditional teardown of sub-CRQs causes double-free of IRQs and hypervisor resources.
--

--- Patch [7]: [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events ---
commit 381f7f27a85750da81e10927e42035d25ca1d22c
Author: Dave Marquardt <davemarq@linux.ibm.com>

ibmvfc: register and use asynchronous sub-queue for events

Complete async sub-queue integration by setting up interrupt handling,
registering the queue as a channel, and enabling its use during NPIV login.
Register async_scrq during channel initialization and unregister during
cleanup.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index ee56f13f1a979..eb786ac242745 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -6470,7 +6571,8 @@ static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *vhost,
>  		return;
>  
>  	for (i = 0; i < channels->max_queues; i++)
> -		ibmvfc_deregister_channel(vhost, channels, i);
> +		ibmvfc_deregister_channel(vhost, channels, &channels->scrqs[i]);
> +	ibmvfc_deregister_channel(vhost, channels, channels->async_scrq);

[Severity: High]
This isn't a bug introduced by this patch, but does adding the async_scrq to
this unconditional deregistration loop extend a double-free bug?

If request_irq() fails during sub-CRQ registration in
ibmvfc_register_channel_common(), it calls irq_dispose_mapping(scrq->irq)
and goes to irq_failed to free the sub-CRQ. However, it doesn't clear
scrq->irq or scrq->cookie.

Later, ibmvfc_dereg_sub_crqs() iterates unconditionally over all queues
(including the failed one) and calls ibmvfc_deregister_channel(), which
again calls free_irq(), irq_dispose_mapping(), and
plpar_hcall_norets(H_FREE_SUB_CRQ).

Could this lead to IRQ domain corruption and a hypervisor resource
double-free?

>  
>  	LEAVE;
>  }

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

  reply	other threads:[~2026-07-10 19:32 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
2026-07-10 19:16 ` [PATCH v4 1/8] ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
2026-07-10 19:31   ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 2/8] ibmvfc: Add NOOP command support Dave Marquardt via B4 Relay
2026-07-10 19:31   ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric Dave Marquardt via B4 Relay
2026-07-10 19:30   ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue Dave Marquardt via B4 Relay
2026-07-10 19:40   ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 5/8] ibmvfc: allocate " Dave Marquardt via B4 Relay
2026-07-10 19:36   ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events Dave Marquardt via B4 Relay
2026-07-10 19:37   ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events Dave Marquardt via B4 Relay
2026-07-10 19:32   ` sashiko-bot [this message]
2026-07-10 19:16 ` [PATCH v4 8/8] ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay
2026-07-10 19:40   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260710193220.A227C1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=davemarq@linux.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox