Re: [PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path
On Sat, 29 Jan 2022 08:16:04 +0100, Christophe JAILLET wrote: > The error handling path of mpc52xx_lpbfifo_probe() and a request_irq() is > not balanced by a corresponding free_irq(). > > Add the missing call, as already done in the remove function. > > Applied to powerpc/next. [1/1] powerpc: platforms: 52xx: Fix a resource leak in an error handling path https://git.kernel.org/powerpc/c/5836947613ef33d311b4eff6a32d019580a214f5 cheers
Re: [PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path
On Sat, Jan 29, 2022 at 08:16:04AM +0100, Christophe JAILLET wrote:
> The error handling path of mpc52xx_lpbfifo_probe() and a request_irq() is
> not balanced by a corresponding free_irq().
>
> Add the missing call, as already done in the remove function.
>
> Fixes: 3c9059d79f5e ("powerpc/5200: add LocalPlus bus FIFO device driver")
> Signed-off-by: Christophe JAILLET
> ---
> Another strange thing is that the remove function has:
> /* Release the bestcomm transmit task */
> free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo);
> but I've not been able to find a corresponding request_irq().
>
> Is it dead code? Is there something missing in the probe?
> (...Is it working?...)
I think you're right that the tx_task IRQ is never allocated.
I'm pretty sure that if you free a zero IRQ then it's a no-op. It won't
find the 0 in the radix tree so irq_to_desc() returns NULL and free_irq()
returns early.
regards,
dan carpenter
Re: [PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path
On Sat, 29 Jan 2022 08:16:04 +0100
Christophe JAILLET christophe.jail...@wanadoo.fr wrote:
>The error handling path of mpc52xx_lpbfifo_probe() and a request_irq() is
>not balanced by a corresponding free_irq().
>
>Add the missing call, as already done in the remove function.
>
>Fixes: 3c9059d79f5e ("powerpc/5200: add LocalPlus bus FIFO device driver")
>Signed-off-by: Christophe JAILLET
>---
>Another strange thing is that the remove function has:
> /* Release the bestcomm transmit task */
> free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo);
>but I've not been able to find a corresponding request_irq().
This driver does not request the tx task irq itself, but a fifo
client driver can request/free tx interrupts for submitted
fifo write tasks, like mpc5200 fec and pata drivers do, so
it is okay.
>Is it dead code? Is there something missing in the probe?
No.
--
Anatolij
[PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path
The error handling path of mpc52xx_lpbfifo_probe() and a request_irq() is
not balanced by a corresponding free_irq().
Add the missing call, as already done in the remove function.
Fixes: 3c9059d79f5e ("powerpc/5200: add LocalPlus bus FIFO device driver")
Signed-off-by: Christophe JAILLET
---
Another strange thing is that the remove function has:
/* Release the bestcomm transmit task */
free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo);
but I've not been able to find a corresponding request_irq().
Is it dead code? Is there something missing in the probe?
(...Is it working?...)
---
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
index b91ebebd9ff2..e0049b7df212 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -530,6 +530,7 @@ static int mpc52xx_lpbfifo_probe(struct platform_device *op)
err_bcom_rx_irq:
bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
err_bcom_rx:
+ free_irq(lpbfifo.irq, &lpbfifo);
err_irq:
iounmap(lpbfifo.regs);
lpbfifo.regs = NULL;
--
2.32.0
