Re: [PATCH 3/3] PCI: endpoint: pci-ep-msi: Let non-first EPFs use embedded doorbells

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing cleanup of doorbell IRQ and message memory in `pci_epf_test_unbind` leads to a Use-After-Free.
--

--- Patch [3]: [PATCH 3/3] PCI: endpoint: pci-ep-msi: Let non-first EPFs use embedded doorbells ---
commit 5b37917c476823bd8dce57c08e171daf54ed33f5
Author: Koichiro Den <den@valinux.co.jp>

PCI: endpoint: pci-ep-msi: Let non-first EPFs use embedded doorbells

This patch allows non-first endpoint functions attached to an endpoint
controller to utilize embedded doorbells. It removes the restriction that
only the first endpoint function can allocate doorbells, falling back to
the embedded doorbell path directly for subsequent functions.

> diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> index a123a189b4e65..5ea7eef9794f7 100644
> --- a/drivers/pci/endpoint/pci-ep-msi.c
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -199,28 +199,31 @@ static int pci_epf_alloc_doorbell_msi(struct pci_epf *epf, u16 num_db)
>  int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
>  {

[ ... ]

>  	/*
> -	 * Fall back to embedded doorbell only when platform MSI is unavailable
> -	 * for this EPC.
> +	 * The MSI-backed doorbell path currently targets the first EPF attached
> +	 * to the EPC. Let non-first EPFs try the embedded doorbell instead.
>  	 */
> -	if (ret != -ENODEV)
> -		return ret;
> +	first_epf = list_first_entry_or_null(&epc->pci_epf, struct pci_epf,
> +					     list);
> +	if (first_epf == epf) {
> +		ret = pci_epf_alloc_doorbell_msi(epf, num_db);
> +		if (!ret)
> +			return 0;
> +
> +		/*
> +		 * Fall back to embedded doorbell only when platform MSI is
> +		 * unavailable for this EPC.
> +		 */
> +		if (ret != -ENODEV)
> +			return ret;
> +	}
>  
>  	ret = pci_epf_alloc_doorbell_embedded(epf, num_db);
>  	if (ret) {

[Severity: High]
This is a pre-existing issue, but as this patch makes it possible for more
devices to allocate doorbells, can this lead to a use-after-free when an
endpoint is unbound while a doorbell is active?

Looking at pci_epf_test_unbind() in drivers/pci/endpoint/functions/pci-epf-test.c:

drivers/pci/endpoint/functions/pci-epf-test.c:pci_epf_test_unbind() {
    struct pci_epf_test *epf_test = epf_get_drvdata(epf);
    struct pci_epc *epc = epf->epc;

    cancel_delayed_work_sync(&epf_test->cmd_handler);
    if (epc->init_complete) {
        pci_epf_test_clean_dma_chan(epf_test);
        pci_epf_test_clear_bar(epf);
    }
    pci_epf_test_free_space(epf);
}

If the host sends a command to enable the doorbell, a doorbell is allocated
and an IRQ is requested via request_threaded_irq().

If the user then unbinds the driver via configfs, pci_epf_test_unbind()
does not check for or clean up the active doorbell. This means free_irq()
and pci_epf_free_doorbell() are not called.

When the endpoint function device is later destroyed via configfs rmdir,
the epf_test structure is freed. However, the IRQ handler remains
registered with the freed epf_test pointer.

Does this allow a remote host to trigger a use-after-free kernel panic by
sending an MSI doorbell interrupt to the endpoint after unbind?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728172306.2751813-1-den@valinux.co.jp?part=3