Skip to contentSkip to conversation

EDAC, thunderx: memory leak in thunderx_l2c_threaded_isr()

PATCH Applied Dan Carpenter 3 participants 4 messages Oct 13, 2018 – Oct 15, 2018

Subsystems (3)
Kernel janitors EDAC-core EDAC-cavium thunderx
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 EDAC, thunderx: memory leak in thunderx_l2c_threaded_isr()

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 <[email protected]>

---
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(-)
drivers/edac/thunderx_edac.c +2 -2
index c009d94f40c5..34be60fe6892 100644
@@ -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);
Signature · 1 line
2.18.0
maintainer Applied
#1 lore raw
Quoted 11 lines · 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 <[email protected]>

---
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.

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
#2 lore raw
In reply to Dan Carpenter
Quoted 10 lines · Borislav Petkov
Quoted 9 lines · 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 <[email protected]>

---
It's also possible that I have misread the code. It seems like an obvious fix but I haven't tested it.

Quoted 7 lines
Quoted 2 lines
 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

maintainer
#3 lore raw
In reply to Dan Carpenter

Hi,

Quoted 2 lines · Jan Glauber

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.

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.