Re: [PATCH v1] PCI: starfive: Fix kmemleak in StarFive PCIe driver's IRQ handling

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Sat, Feb 08, 2025 at 07:31:08PM +0530, Anand Moon wrote:
> kmemleak reported following debug log
> 
> $ sudo cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffffffd6c47c2600 (size 128):
>   comm "kworker/u16:2", pid 38, jiffies 4294942263
>   hex dump (first 32 bytes):
>     cc 7c 5a 8d ff ff ff ff 40 b0 47 c8 d6 ff ff ff  .|Z.....@.G.....
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace (crc 4f07ff07):
>     __create_object+0x2a/0xfc
>     kmemleak_alloc+0x38/0x98
>     __kmalloc_cache_noprof+0x296/0x444
>     request_threaded_irq+0x168/0x284
>     devm_request_threaded_irq+0xa8/0x13c
>     plda_init_interrupts+0x46e/0x858
>     plda_pcie_host_init+0x356/0x468
>     starfive_pcie_probe+0x2f6/0x398
>     platform_probe+0x106/0x150
>     really_probe+0x30e/0x746
>     __driver_probe_device+0x11c/0x2c2
>     driver_probe_device+0x5e/0x316
>     __device_attach_driver+0x296/0x3a4
>     bus_for_each_drv+0x1d0/0x260
>     __device_attach+0x1fa/0x2d6
>     device_initial_probe+0x14/0x28
> unreferenced object 0xffffffd6c47c2900 (size 128):
>   comm "kworker/u16:2", pid 38, jiffies 4294942281
> 
> This patch addresses a kmemleak reported during StarFive PCIe driver
> initialization. The leak was observed with kmemleak reporting
> unreferenced objects related to IRQ handling. The backtrace pointed
> to the `request_threaded_irq` and related functions within the
> `plda_init_interrupts` path, indicating that some allocated memory
> related to IRQs was not being properly freed.
> 
> The issue was that while the driver requested IRQs, it wasn't
> correctly handling the lifecycle of the associated resources.

What resources?

> This patch introduces an event IRQ handler and the necessary
> infrastructure to manage these IRQs, preventing the memory leak.
> 

These handles appear pointless to me. What purpose are they serving?

- Mani

> Fixes: 647690479660 ("PCI: microchip: Add request_event_irq() callback function")
> Cc: Minda Chen <minda.chen@starfivetech.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/pci/controller/plda/pcie-starfive.c | 39 +++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/pci/controller/plda/pcie-starfive.c b/drivers/pci/controller/plda/pcie-starfive.c
> index e73c1b7bc8ef..a57177a8be8a 100644
> --- a/drivers/pci/controller/plda/pcie-starfive.c
> +++ b/drivers/pci/controller/plda/pcie-starfive.c
> @@ -381,7 +381,46 @@ static const struct plda_pcie_host_ops sf_host_ops = {
>  	.host_deinit = starfive_pcie_host_deinit,
>  };
>  
> +/* IRQ handler for PCIe events */
> +static irqreturn_t starfive_event_handler(int irq, void *dev_id)
> +{
> +	struct plda_pcie_rp *port = dev_id;
> +	struct irq_data *data;
> +
> +	/* Retrieve IRQ data */
> +	data = irq_domain_get_irq_data(port->event_domain, irq);
> +
> +	if (data) {
> +		dev_err_ratelimited(port->dev, "Event IRQ %ld triggered\n",
> +				    data->hwirq);
> +	} else {
> +		dev_err_ratelimited(port->dev, "Invalid event IRQ %d\n", irq);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/* Request an IRQ for a specific event */
> +static int starfive_request_event_irq(struct plda_pcie_rp *plda, int event_irq,
> +				      int event)
> +{
> +	int ret;
> +
> +	/* Request the IRQ and associate it with the handler */
> +	ret = devm_request_irq(plda->dev, event_irq, starfive_event_handler,
> +			       IRQF_SHARED, "starfivepcie", plda);
> +
> +	if (ret) {
> +		dev_err(plda->dev, "Failed to request IRQ %d: %d\n", event_irq,
> +			ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
>  static const struct plda_event stf_pcie_event = {
> +	.request_event_irq = starfive_request_event_irq,
>  	.intx_event = EVENT_PM_MSI_INT_INTX,
>  	.msi_event  = EVENT_PM_MSI_INT_MSI
>  };
> 
> base-commit: bb066fe812d6fb3a9d01c073d9f1e2fd5a63403b
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்