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

[Koichiro Den]

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@xxxxxxxxxxxxx>
> > ---
> >  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