Re: [PATCH] PCI: endpoint: pci-epf-test: Free doorbell IRQ on unbind and deinit

[Manivannan Sadhasivam]

On 2026-09-05 01:45 +0000, Fan Wu wrote:
> Neither pci_epf_test_unbind() nor pci_epf_test_epc_deinit() frees the
> doorbell IRQ. epc_deinit() clears the BARs, while unbind() can free the
> BAR backing store; if the doorbell is enabled, its IRQ action remains
> registered with epf_test as dev_id and epf->db_msg stays allocated. A
> doorbell interrupt may already have awakened the threaded handler when
> teardown starts, and since cancel_delayed_work_sync() drains only the
> command worker and clear_bar() does not wait for the IRQ thread,
> unbind() can free that backing while the handler still dereferences
> epf_test->reg[]. The leftover IRQ also makes the next doorbell
> allocation on the same function fail with -EBUSY.
> 
> Free the doorbell IRQ in both paths, guarded by a
> doorbell_irq_registered flag set once request_threaded_irq() has
> succeeded and cleared in pci_epf_test_doorbell_cleanup(), the
> chokepoint shared by the enable error path, disable_doorbell() and the
> teardown sites, so no disarm path can double-free. In unbind(), the
> drain depends on the driver's own flag, not on epc->init_complete.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: eff0c286aa91 ("PCI: endpoint: pci-epf-test: Add doorbell test support")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Codex:gpt-5.6
> Co-developed-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index d4905aa..ab8df91 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -95,6 +95,7 @@ struct pci_epf_test {
>  	const struct pci_epc_features *epc_features;
>  	struct pci_epf_bar	db_bar;
>  	bool			db_bar_programmed;
> +	bool			doorbell_irq_registered;
>  	size_t			bar_size[PCI_STD_NUM_BARS];
>  };
>  
> @@ -721,6 +722,7 @@ static void pci_epf_test_doorbell_cleanup(struct pci_epf_test *epf_test)
>  	struct pci_epf *epf = epf_test->epf;
>  
>  	reg->doorbell_bar = cpu_to_le32(NO_BAR);
> +	epf_test->doorbell_irq_registered = false;
>  
>  	pci_epf_free_doorbell(epf);
>  }
> @@ -772,6 +774,7 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
>  		goto err_doorbell_cleanup;
>  	}
>  
> +	epf_test->doorbell_irq_registered = true;
>  	reg->doorbell_data = cpu_to_le32(msg->data);
>  	reg->doorbell_bar = cpu_to_le32(bar);
>  
> @@ -1238,6 +1241,10 @@ static void pci_epf_test_epc_deinit(struct pci_epf *epf)
>  
>  	cancel_delayed_work_sync(&epf_test->cmd_handler);
>  	pci_epf_test_clean_dma_chan(epf_test);
> +	if (epf_test->doorbell_irq_registered) {

I don't think you need this extra variable to track the state of the IRQ. You
can rely on 'epf->db_msg' to be non-NULL.

- Mani