EDAC, thunderx: memory leak in thunderx_l2c_threaded_isr()

EDAC, thunderx: memory leak in thunderx_l2c_threaded_isr()

[Dan Carpenter]

I found this memory leak with static analysis, but it looks like it
might be pretty bad because it affects the success path.

Fixes: 41003396f932 ("EDAC, thunderx: Add Cavium ThunderX EDAC driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
It's also possible that I have misread the code.  It seems like an
obvious fix but I haven't tested it.

 drivers/edac/thunderx_edac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index c009d94f40c5..34be60fe6892 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -1884,7 +1884,7 @@ static irqreturn_t thunderx_l2c_threaded_isr(int irq, void *irq_id)
 	default:
 		dev_err(&l2c->pdev->dev, "Unsupported device: %04x\n",
 			l2c->pdev->device);
-		return IRQ_NONE;
+		goto err_free;
 	}
 
 	while (CIRC_CNT(l2c->ring_head, l2c->ring_tail,
@@ -1906,7 +1906,7 @@ static irqreturn_t thunderx_l2c_threaded_isr(int irq, void *irq_id)
 		l2c->ring_tail++;
 	}
 
-	return IRQ_HANDLED;
+	ret = IRQ_HANDLED;
 
 err_free:
 	kfree(other);

EDAC, thunderx: memory leak in thunderx_l2c_threaded_isr()

[Borislav Petkov]

Hi,

On Mon, Oct 15, 2018 at 12:47:24PM +0000, Jan Glauber wrote:
> the allocations happen only in the threaded function handlers, not in
> hard IRQ context, is that still an issue?

Nah, I take that back. I'm being told you can do almost everything in
threaded handler context! :-)

Thx.

EDAC, thunderx: memory leak in thunderx_l2c_threaded_isr()

[Jan Glauber]

On Sat, Oct 13, 2018 at 02:28:04PM +0200, Borislav Petkov wrote:
> On Sat, Oct 13, 2018 at 01:28:43PM +0300, Dan Carpenter wrote:
> > I found this memory leak with static analysis, but it looks like it
> > might be pretty bad because it affects the success path.
> >
> > Fixes: 41003396f932 ("EDAC, thunderx: Add Cavium ThunderX EDAC driver")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > It's also possible that I have misread the code.  It seems like an
> > obvious fix but I haven't tested it.
> >
> >  drivers/edac/thunderx_edac.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Applied, thanks.
> 
> Jan, David, as a further cleanup, you could consider moving that
> allocation out of the ISR...

[-David who left Cavium]

Hi Boris,

the allocations happen only in the threaded function handlers, not in
hard IRQ context, is that still an issue?

thanks,
Jan

EDAC, thunderx: memory leak in thunderx_l2c_threaded_isr()

[Borislav Petkov]

On Sat, Oct 13, 2018 at 01:28:43PM +0300, Dan Carpenter wrote:
> I found this memory leak with static analysis, but it looks like it
> might be pretty bad because it affects the success path.
> 
> Fixes: 41003396f932 ("EDAC, thunderx: Add Cavium ThunderX EDAC driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> It's also possible that I have misread the code.  It seems like an
> obvious fix but I haven't tested it.
> 
>  drivers/edac/thunderx_edac.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

Jan, David, as a further cleanup, you could consider moving that
allocation out of the ISR...

Thx.