Re: [PATCH 1/4] PCI: endpoint: pci-epf-vntb: Fix MSI doorbell IRQ unwind

Koichiro Den
2026-02-16T14:03:01Z

On Mon, Feb 16, 2026 at 12:56:18PM +0100, Niklas Cassel wrote:
> On Mon, Feb 16, 2026 at 12:09:11AM +0900, Koichiro Den wrote:
> > epf_ntb_db_bar_init_msi_doorbell() requests ntb->db_count doorbell IRQs
> > and then performs additional MSI doorbell setup that may still fail.
> > The error path unwinds the requested IRQs, but it uses a loop variable
> > that is reused later in the function. When a later step fails, the
> > unwind can run with an unexpected index value and leave some IRQs
> > requested.
> >
> > Track the number of successfully requested IRQs separately and use that
> > counter for the unwind so all previously requested IRQs are freed on
> > failure.
> >
> > Fixes: dc693d606644 ("PCI: endpoint: pci-epf-vntb: Add MSI doorbell support")
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> > drivers/pci/endpoint/functions/pci-epf-vntb.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index 20a400e83439..20efa27325f1 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -523,6 +523,7 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > enum pci_barno barno)
> > {
> > struct pci_epf *epf = ntb->epf;
> > + unsigned int req;
> > dma_addr_t low, high;
> > struct msi_msg *msg;
> > size_t sz;
> > @@ -533,14 +534,14 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > if (ret)
> > return ret;
> >
> > - for (i = 0; i < ntb->db_count; i++) {
> > - ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> > + for (req = 0; req < ntb->db_count; req++) {
> > + ret = request_irq(epf->db_msg[req].virq, epf_ntb_doorbell_handler,
> > 0, "pci_epf_vntb_db", ntb);
> >
> > if (ret) {
> > dev_err(&epf->dev,
> > "Failed to request doorbell IRQ: %d\n",
> > - epf->db_msg[i].virq);
> > + epf->db_msg[req].virq);
> > goto err_free_irq;
> > }
> > }
> > @@ -598,8 +599,8 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > return 0;
> >
> > err_free_irq:
> > - for (i--; i >= 0; i--)
> > - free_irq(epf->db_msg[i].virq, ntb);
> > + while (req)
> > + free_irq(epf->db_msg[--req].virq, ntb);
>
> Please keep the for-loop.
> Or if you want to change it, do so in a separate patch.
>
>
> I understand that you need a separate variable for this,
> since "i" is (re-)used in other places in the function,
> but changing the for loop to a while is distracting from
> the actual fix.

In that case, would you prefer the new "req" to be an int rather than unsigned
int, so the for-loop can remain safely unchanged?

I don't have a strong opinion on this. The reuse of "i" is the real problem part
and the main point of this fix.

Best regards,
Koichiro

>
>
> Kind regards,
> Niklas