| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANAwSgQ20ANRh9wJ3E-T9yNi=g1g129mXq3cZYvPnK1bMx+w7g@mail.gmail.com>
Date: Tue, 11 Feb 2025 01:09:04 +0530
From: Anand Moon <linux.amoon@...il.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: Kevin Xie <kevin.xie@...rfivetech.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
Conor Dooley <conor.dooley@...rochip.com>, Minda Chen <minda.chen@...rfivetech.com>,
"open list:PCIE DRIVER FOR STARFIVE JH71x0" <linux-pci@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] PCI: starfive: Fix kmemleak in StarFive PCIe driver's
IRQ handling
Hi Manivannan
On Mon, 10 Feb 2025 at 23:14, Manivannan Sadhasivam
<manivannan.sadhasivam@...aro.org> wrote:
>
> 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.....@.......
> > 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?
>
The Microchip PCIe host driver [1] handles PCI, SEC, DEBUG, and LOCAL
hardware events
through a dedicated framework [2]. This framework allows the core driver [3]
to monitor and wait for these specific events.
[1]: https://github.com/torvalds/linux/blob/master/drivers/pci/controller/plda/pcie-microchip-host.c#L90-L292
[2]: https://github.com/torvalds/linux/blob/master/drivers/pci/controller/plda/pcie-microchip-host.c#L374-L499
[3]: https://github.com/torvalds/linux/blob/master/drivers/pci/controller/plda/pcie-plda-host.c#L448-L466
StarFive PCIe driver currently lacks the necessary `request_event_irq`
implementation
to integrate with this event-handling mechanism, which prevents the core driver
from properly responding to these events on StarFive platforms.
static const struct plda_event mc_event = {
. request_event_irq = mc_request_event_irq,
.intx_event = EVENT_LOCAL_PM_MSI_INT_INTX,
.msi_event = EVENT_LOCAL_PM_MSI_INT_MSI,
};
This patch implements dummy `request_event_irq` for the StarFive PCIe driver,
Since the core [3] driver is monitoring for these events
> > 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?
>
Yes, you are correct, the core event monitoring framework [3] triggered a
kernel memory leak. This patch adds a dummy IRQ callback as a
placeholder for proper event handling, which can be implemented in a
future patch.
> - Mani
>
Thanks
-Anand
> > Fixes: 647690479660 ("PCI: microchip: Add request_event_irq() callback function")
> > Cc: Minda Chen <minda.chen@...rfivetech.com>
> > Signed-off-by: Anand Moon <linux.amoon@...il.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
> >
>
> --
> மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists