Messages in this thread | | | Subject | Re: gigaset: memory leak in gigaset_initcshw | From | Paul Bolle <> | Date | Fri, 05 Feb 2016 17:06:27 +0100 |
| |
Hi Dmitry,
(If anyone is confused by this conversation: Dmitry replied to an off list message.)
On vr, 2016-02-05 at 14:28 +0100, Dmitry Vyukov wrote: > I wonder why you don't see the leak I am seeing...
So do I, for a few days now.
> are you suing qemu or real hardware? I am using qemu.
Real hardware (a ThinkPad). Probably less powerful that your VM.
What is the rate you're seeing leakage of a struct ser_cardstate? I'm running your latest test at about 2.000 TIOCSETD's per second - which is by itself not very useful for our driver - and notice no _obvious_ leakage when I do that for a few minutes. I do note the hardware screaming to just keep up with the abuse, though.
> 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()?
gigaset_device_release() is the release operation that is run when our struct device goes away. The core code is responsible for calling it, we can't be certain when that will happen. At least, we should not expect it to happen directly after calling platform_device_unregister().
(It was actually syzkaller that warned us that we did just that until recently. See commit 4c5e354a9742 ("ser_gigaset: fix deallocation of platform device structure").)
> If that would happen, then cs can be reused while the previous > cs->hw.ser is not freed yet. Just a guess.
I'll have to ponder on that a bit, sorry.
Paul Bolle
| |