Re: gigaset: memory leak in gigaset_initcshw
From: Dmitry Vyukov <dvyukov@google.com>
Date: 2016-02-05 13:29:22
Also in:
lkml
On Thu, Feb 4, 2016 at 4:06 PM, Paul Bolle [off-list ref] wrote:
On do, 2016-02-04 at 15:54 +0100, Dmitry Vyukov wrote:quoted
One TIOCSETD is enough to trigger the leak. I've tested with different line disciplines and only N_GIGASET_M101 triggers the leak.So things appear to be just on my plate now. I'll see what I can come up with. Feel free to prod me if I stay silent for too long. Thanks for narrowing things down!
quoted hunk ↗ jump to hunk
0) It's way too late here. And I can't really reproduce, and too many things jumps to 100% when running your reproducer. Anyhow, this is all I came up with (for drivers/isdn/gigaset/common.c):@@ -427,7 +427,11 @@ exit: static void free_cs(struct cardstate *cs) { - cs->flags = 0; + unsigned long flags; + struct gigaset_driver *drv = cs->driver; + spin_lock_irqsave(&drv->lock, flags); + cs->flags &= ~VALID_MINOR; + spin_unlock_irqrestore(&drv->lock, flags); } static void make_valid(struct cardstate *cs, unsigned mask)1) On my side the logs do seem more sensible, for what it's worth. But does this fix the leak? 2) Note that I fear your reproducer allows to DOS the box (even if the leak turns out to be fixed) so the mess might be getting much worse. I need a clear hear to think this through. Ie, I need some sleep.
No, it does not help. I wonder why you don't see the leak I am seeing... are you suing qemu or real hardware? I am using qemu. I've added the following change:
--- a/drivers/isdn/gigaset/ser-gigaset.c
+++ b/drivers/isdn/gigaset/ser-gigaset.c@@ -396,6 +396,7 @@ static int gigaset_initcshw(struct cardstate *cs) pr_err("out of memory\n"); return -ENOMEM; } + WARN_ON(cs->hw.ser != NULL); cs->hw.ser = scs; cs->hw.ser->dev.name = GIGASET_MODULENAME;
and it does fire. Can it be a case that free_cs() runs before gigaset_device_release()? If that would happen, then cs can be reused while the previous cs->hw.ser is not freed yet. Just a guess.