[PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path

[PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path

[Christophe JAILLET]

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 <christophe.jaillet@wanadoo.fr>
---
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

Re: [PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path

[Anatolij Gustschin]

On Sat, 29 Jan 2022 08:16:04 +0100
Christophe JAILLET christophe.jaillet@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 <christophe.jaillet@wanadoo.fr>
>---
>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

Re: [PATCH] powerpc: platforms: 52xx: Fix a resource leak in an error handling path

[Dan Carpenter]

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 <christophe.jaillet@wanadoo.fr>
> ---
> 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

[Michael Ellerman]

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