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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <249d79be-deed-4277-96c3-845bafcec37a@ti.com>
Date: Thu, 4 Sep 2025 14:42:45 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Jiri Slaby <jirislaby@...nel.org>
CC: Siddharth Vadapalli <s-vadapalli@...com>, <lpieralisi@...nel.org>,
        <kwilczynski@...nel.org>, <mani@...nel.org>, <robh@...nel.org>,
        <bhelgaas@...gle.com>, <jingoohan1@...il.com>, <fan.ni@...sung.com>,
        <quic_wenbyao@...cinc.com>, <namcao@...utronix.de>,
        <mayank.rana@....qualcomm.com>, <thippeswamy.havalige@....com>,
        <quic_schintav@...cinc.com>, <shradha.t@...sung.com>,
        <inochiama@...il.com>, <cassel@...nel.org>, <kishon@...nel.org>,
        <18255117159@....com>, <rongqianfeng@...o.com>,
        <linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <srk@...com>
Subject: Re: [PATCH 09/11] PCI: keystone: Switch to devm_request_irq() for
 "ks-pcie-error-irq" IRQ

On Thu, Sep 04, 2025 at 09:24:01AM +0200, Jiri Slaby wrote:
> On 03. 09. 25, 14:44, Siddharth Vadapalli wrote:
> > In preparation for enabling loadable module support for the driver,
> > there is motivation to switch to devm_request_irq() to simplify the
> > cleanup on driver removal. Additionally, since the interrupt handler
> > associated with the "ks-pcie-error-irq" namely "ks_pcie_handle_error_irq()
> > is only printing the error and is clearing the interrupt, there is no
> > necessity to prefer devm_request_threaded_irq() over devm_request_irq().
> > Hence, switch from request_irq() to devm_request_irq().
> > 
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
> > ---
> >   drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> > index bb93559f6468..02f9a6d0e4a8 100644
> > --- a/drivers/pci/controller/dwc/pci-keystone.c
> > +++ b/drivers/pci/controller/dwc/pci-keystone.c
> > @@ -1277,8 +1277,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
> >   	if (irq < 0)
> >   		return irq;
> > -	ret = request_irq(irq, ks_pcie_err_irq_handler, IRQF_SHARED,
> > -			  "ks-pcie-error-irq", ks_pcie);
> > +	ret = devm_request_irq(dev, irq, ks_pcie_err_irq_handler, IRQF_SHARED,
> > +			       "ks-pcie-error-irq", ks_pcie);
> >   	if (ret < 0) {
> >   		dev_err(dev, "failed to request error IRQ %d\n",
> >   			irq);
> 
> Ugh, so you are not removing any free_irq() from anywhere?
> 
> <me checking>
> 
> Because there is none...
> 
> So you are actually fixing an IRQ leak in case something later fails -- I
> guess this needs Fixes and Cc stable tags, right?

Yes! While working on this series and unloading the module, I had seen
the issue and narrowed it down to the absence of a free_irq() in the
driver's .remove callback. Since the driver cannot be unloaded prior to
this series (driver can only be built-in), I had felt that a separate
fix may not be required for adding the missing free_irq() in the .remove
callback. However, after you pointed it out now, I realize that there
are other places during the driver's probe where we may exit in the
event of an error, without freeing the interrupt. Thank you for noticing
this. The commit to be fixed is:
0790eb175ee0 PCI: keystone: Cleanup error_irq configuration
Link to the commit is:
https://github.com/torvalds/linux/commit/0790eb175ee0
I will include the Fixes tag for the above commit and also Cc stable when
I post the v2 series. I will wait for feedback on the remaining patches
in the series before posting the v2 series.

Regards,
Siddharth.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ