On Thu, Feb 4, 2016 at 4:06 PM, Paul Bolle <pebo...@tiscali.nl> wrote:
> On do, 2016-02-04 at 15:54 +0100, Dmitry Vyukov wrote:
>> 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!
> 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.
