Re: gigaset: memory leak in gigaset_initcshw
Hi Dmitry,
On vr, 2016-02-05 at 17:06 +0100, Paul Bolle wrote:
> 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.
0) I finally managed to reliably trigger this leak on an i686, single
core machine (yet another ThinkPad).
1) Note that on that machine the leak was noticeable under the kmalloc
-512 line (struct ser_cardstate is 456 bytes on that machine). I'm
_guessing_ the kmalloc-2048 line, which I stared at for quite some time,
is only relevant here for x86_64 and when there's a bit of
instrumentation, or whatever, added to the slab objects (as they are in
your VM?).
2) More important was that this i686 machine ran a tree that actually
included the offending commit:
25cad69f21f5 ("base/platform: Fix platform drivers with no probe
callback").
See, after staring at the gigaset code for way too long I decided to
just use brute force. Ie, I bisected this issue.
2) Anyhow, thanks again for the report. Now on to the next question: how
on earth does that commit make ser_gigaset leak struct ser_cardstate?
To be continued,
Paul Bolle
Re: gigaset: memory leak in gigaset_initcshw
On vr, 2016-02-05 at 22:25 +0100, Dmitry Vyukov wrote: > On Fri, Feb 5, 2016 at 7:36 PM, Paul Bolle wrote: > > Does that make any difference? > Nope. > Almost 500 objects leaked in less than 10 seconds: Too bad. Still a nice (potential) clean up though. Thanks, Paul Bolle
Re: gigaset: memory leak in gigaset_initcshw
On Fri, Feb 5, 2016 at 7:36 PM, Paul Bolle wrote:
> On vr, 2016-02-05 at 17:06 +0100, Paul Bolle wrote:
>> 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.
>
> This is from the hit-the-code-until-it-confesses department:
> --- a/drivers/isdn/gigaset/ser-gigaset.c
> +++ b/drivers/isdn/gigaset/ser-gigaset.c
> @@ -373,13 +373,9 @@ static void gigaset_freecshw(struct cardstate *cs)
>
> static void gigaset_device_release(struct device *dev)
> {
> - struct cardstate *cs = dev_get_drvdata(dev);
> -
> - if (!cs)
> - return;
> + struct ser_cardstate *scs = dev_get_drvdata(dev);
> dev_set_drvdata(dev, NULL);
> - kfree(cs->hw.ser);
> - cs->hw.ser = NULL;
> + kfree(scs);
> }
>
> /*
> @@ -408,7 +404,7 @@ static int gigaset_initcshw(struct cardstate *cs)
> cs->hw.ser = NULL;
> return rc;
> }
> - dev_set_drvdata(&cs->hw.ser->dev.dev, cs);
> + dev_set_drvdata(&cs->hw.ser->dev.dev, scs);
>
> tasklet_init(&cs->write_tasklet,
> gigaset_modem_fill, (unsigned long) cs);
>
> Does that make any difference?
Nope.
Almost 500 objects leaked in less than 10 seconds:
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20481992 2015 2520 138 : tunables00
0 : slabdata155155 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482024 2041 2520 138 : tunables00
0 : slabdata157157 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482061 2080 2520 138 : tunables00
0 : slabdata160160 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482091 2119 2520 138 : tunables00
0 : slabdata163163 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482147 2171 2520 138 : tunables00
0 : slabdata167167 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482228 2236 2520 138 : tunables00
0 : slabdata172172 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482261 2288 2520 138 : tunables00
0 : slabdata176176 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482289 2301 2520 138 : tunables00
0 : slabdata177177 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482316 2340 2520 138 : tunables00
0 : slabdata180180 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482324 2366 2520 138 : tunables00
0 : slabdata182182 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482356 2379 2520 138 : tunables00
0 : slabdata183183 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482450 2509 2520 138 : tunables00
0 : slabdata193193 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482450 2509 2520 138 : tunables00
0 : slabdata193193 0
# cat /proc/slabinfo | egrep "^kmalloc-2048"
kmalloc-20482450 2509 2520 138 : tunables00
0 : slabdata193193 0
Re: gigaset: memory leak in gigaset_initcshw
On vr, 2016-02-05 at 17:06 +0100, Paul Bolle wrote:
> 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.
This is from the hit-the-code-until-it-confesses department:
--- a/drivers/isdn/gigaset/ser-gigaset.c
+++ b/drivers/isdn/gigaset/ser-gigaset.c
@@ -373,13 +373,9 @@ static void gigaset_freecshw(struct cardstate *cs)
static void gigaset_device_release(struct device *dev)
{
- struct cardstate *cs = dev_get_drvdata(dev);
-
- if (!cs)
- return;
+ struct ser_cardstate *scs = dev_get_drvdata(dev);
dev_set_drvdata(dev, NULL);
- kfree(cs->hw.ser);
- cs->hw.ser = NULL;
+ kfree(scs);
}
/*
@@ -408,7 +404,7 @@ static int gigaset_initcshw(struct cardstate *cs)
cs->hw.ser = NULL;
return rc;
}
- dev_set_drvdata(&cs->hw.ser->dev.dev, cs);
+ dev_set_drvdata(&cs->hw.ser->dev.dev, scs);
tasklet_init(&cs->write_tasklet,
gigaset_modem_fill, (unsigned long) cs);
Does that make any difference?
Paul Bolle
Re: gigaset: memory leak in gigaset_initcshw
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
Re: gigaset: memory leak in gigaset_initcshw
On Thu, Feb 4, 2016 at 4:06 PM, Paul Bolle 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.
Re: gigaset: memory leak in gigaset_initcshw
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! Paul Bolle
Re: gigaset: memory leak in gigaset_initcshw
On Thu, Feb 4, 2016 at 2:46 PM, Paul Bolle wrote: > Hi Dmitry, > > On do, 2016-02-04 at 14:15 +0100, Dmitry Vyukov wrote: >> On Thu, Feb 4, 2016 at 2:09 PM, Paul Bolle wrote: >> > What are you seeing here? >> >> I see that active_objs is slowly, constantly growing. >> >> I've attached my config file, please try with it. You mentioned that >> "16 is N_GIGASET_M101, while 7 is N_6PACK", probably one of these ttys >> is not enabled in your config, > > Both are. (A 6pack module is loaded when I run the reproducer. I have no > idea what 6pack is good for. ser_gigaset is familiar, to me.) > >> and so the reproducer is not doing >> anything useful. > > I actually wonder whether N_6PACK is relevant. Do you also see this > issue with another line discipline in the second TIOCSETD ioctl? > > Maybe it even triggers with two totally different line disciplines in > both calls? A (slightly edited) copy of tty.h is pasted below. It lists > the useful values for the TIOCSETD ioctl. > > Would you mind testing a few combinations? > > Thanks, > > > Paul Bolle > > /* line disciplines */ > #define N_TTY 0 > #define N_SLIP 1 > #define N_MOUSE 2 > #define N_PPP 3 > /* 4 is obsolete */ > #define N_AX25 5 > #define N_X25 6 /* X.25 async */ > #define N_6PACK 7 > /* 8 is obsolete */ > #define N_R3964 9 /* Simatic R3964 */ > /* 10 is obsolete */ > #define N_IRDA 11 /* Linux IrDa - http://irda.sourceforge.net/ > */ > /* 12 is obsolete */ > #define N_HDLC 13 /* synchronous HDLC */ > #define N_SYNC_PPP 14 /* synchronous PPP */ > #define N_HCI 15 /* Bluetooth HCI UART */ > #define N_GIGASET_M101 16 /* Siemens Gigaset M101 serial DECT adapter */ > #define N_SLCAN 17 /* Serial / USB serial CAN Adaptors */ > #define N_PPS 18 /* Pulse per Second */ > #define N_V253 19 /* Codec control over voice modem */ > #define N_CAIF 20 /* CAIF protocol for talking to modems */ > #define N_GSM0710 21 /* GSM 0710 Mux */ > #define N_TI_WL 22 /* for TI's WL BT, FM, GPS combo chips */ > #define N_TRACESINK 23 /* Trace data routing for MIPI P1149.7 */ > #define N_TRACEROUTER 24 /* Trace data routing for MIPI P1149.7 */ > #define N_NCI 25 /* NFC NCI UART */ One TIOCSETD is enough to trigger the leak. I've tested with different line disciplines and only N_GIGASET_M101 triggers the leak. The program prints: ioctl failed with 19 ioctl failed with 19 ioctl failed with 19 ioctl failed with 19 ioctl failed with 19 ioctl failed with 19 ioctl failed with 19 ioctl failed with 19 And console output looks as follows: [ 107.311623] driver: 'ser_gigaset': driver_bound: bound to device 'ser_gigaset.0' [ 107.312502] bus: 'platform': really_probe: bound device ser_gigaset.0 to driver ser_gigaset [ 107.313789] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.314335] gigaset: maximum number of devices exceeded [ 107.318272] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.318794] gigaset: maximum number of devices exceeded [ 107.319541] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.320072] gigaset: maximum number of devices exceeded [ 107.320792] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.321834] gigaset: maximum number of devices exceeded [ 107.322782] kcapi: controller [001] "ser_gigaset" ready. [ 107.324414] kcapi: controller [001] down. [ 107.327864] bus: 'platform': remove device ser_gigaset.0 [ 107.328733] kcapi: controller [001]: ser_gigaset unregistered [ 107.331246] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.331934] kcapi: controller [001]: ser_gigaset attached [ 107.337741] Registering platform device 'ser_gigaset.0'. Parent at platform [ 107.340149] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.340664] gigaset: maximum number of devices exceeded [ 107.341470] bus: 'platform': add device ser_gigaset.0 [ 107.342189] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.342702] gigaset: maximum number of devices exceeded [ 107.344661] bus: 'platform': driver_probe_device: matched device ser_gigaset.0 with driver ser_gigaset [ 107.345420] bus: 'platform': really_probe: probing driver ser_gigaset with device ser_gigaset.0 [ 107.346185] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.346187] gigaset: maximum number of devices exceeded [ 107.351893] ser_gigaset: Serial Driver for Gigaset 307x using Siemens M101 [ 107.352888] gigaset: maximum number of devices exceeded [ 107.359171] devices_kset: Moving ser_gigaset.0 to end of list [ 107.359867] driver: 'ser_gigaset': driver_bound: bound to
Re: gigaset: memory leak in gigaset_initcshw
Hi Dmitry, On do, 2016-02-04 at 14:15 +0100, Dmitry Vyukov wrote: > On Thu, Feb 4, 2016 at 2:09 PM, Paul Bolle wrote: > > What are you seeing here? > > I see that active_objs is slowly, constantly growing. > > I've attached my config file, please try with it. You mentioned that > "16 is N_GIGASET_M101, while 7 is N_6PACK", probably one of these ttys > is not enabled in your config, Both are. (A 6pack module is loaded when I run the reproducer. I have no idea what 6pack is good for. ser_gigaset is familiar, to me.) > and so the reproducer is not doing > anything useful. I actually wonder whether N_6PACK is relevant. Do you also see this issue with another line discipline in the second TIOCSETD ioctl? Maybe it even triggers with two totally different line disciplines in both calls? A (slightly edited) copy of tty.h is pasted below. It lists the useful values for the TIOCSETD ioctl. Would you mind testing a few combinations? Thanks, Paul Bolle /* line disciplines */ #define N_TTY 0 #define N_SLIP 1 #define N_MOUSE 2 #define N_PPP 3 /* 4 is obsolete */ #define N_AX25 5 #define N_X25 6 /* X.25 async */ #define N_6PACK 7 /* 8 is obsolete */ #define N_R3964 9 /* Simatic R3964 */ /* 10 is obsolete */ #define N_IRDA 11 /* Linux IrDa - http://irda.sourceforge.net/ */ /* 12 is obsolete */ #define N_HDLC 13 /* synchronous HDLC */ #define N_SYNC_PPP 14 /* synchronous PPP */ #define N_HCI 15 /* Bluetooth HCI UART */ #define N_GIGASET_M101 16 /* Siemens Gigaset M101 serial DECT adapter */ #define N_SLCAN 17 /* Serial / USB serial CAN Adaptors */ #define N_PPS 18 /* Pulse per Second */ #define N_V253 19 /* Codec control over voice modem */ #define N_CAIF 20 /* CAIF protocol for talking to modems */ #define N_GSM0710 21 /* GSM 0710 Mux */ #define N_TI_WL 22 /* for TI's WL BT, FM, GPS combo chips */ #define N_TRACESINK 23 /* Trace data routing for MIPI P1149.7 */ #define N_TRACEROUTER 24 /* Trace data routing for MIPI P1149.7 */ #define N_NCI 25 /* NFC NCI UART */
Re: gigaset: memory leak in gigaset_initcshw
On do, 2016-02-04 at 11:40 +0100, Dmitry Vyukov wrote:
> Forgot to mention that you need to run it in a parallel loop, sorry.
I see.
> This one should do:
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
>
> void work()
> {
> long r[7];
> memset(r, -1, sizeof(r));
> r[0] = syscall(SYS_mmap, 0x2000ul, 0x1ul, 0x3ul, 0x32ul,
> 0xul, 0x0ul);
> r[2] = syscall(SYS_open, "/dev/ptmx", 0x8002ul, 0x0ul, 0, 0, 0);
> *(uint32_t*)0x20002b1e = (uint32_t)0x10;
> r[4] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20002b1eul, 0, 0, 0);
> *(uint32_t*)0x20009000 = (uint32_t)0x7;
> r[6] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20009000ul, 0, 0, 0);
> }
>
> int main() {
> int running, status;
(gcc complained about "running" being used uninitialized, though a few
mock runs suggest it got initialized to 0 anyhow. I initialized
"running" to 0 explicitly for the real runs.)
> for (;;) {
> while (running < 32) {
> if (fork() == 0) {
> work();
> exit(0);
> }
> running++;
> }
> if (wait(&status) > 0)
> running--;
> }
> }
(Note to self: this hammers my laptop with about 2.000 runs per second.
After some time systemd's logging appears to have trouble handling the
output this reproducer generates, so maybe messages end up getting
dropped.)
> While running it, sample/proc/slabinfo with:
>
> # cat /proc/slabinfo | egrep "^kmalloc-2048"
>
> It constantly grows.
I don't really know how /proc/slabinfo should be interpreted, sorry. But
the interesting fields appear to be "" and "".
"" seems to be stable here (during the runs of a few minutes
that I dare to inflict on my laptop). "" is more volatile.
But I also saw it going down while the reproducer was running.
What are you seeing here?
Thanks,
Paul Bolle
Re: gigaset: memory leak in gigaset_initcshw
On Wed, Feb 3, 2016 at 8:11 PM, Paul Bolle wrote:
> Hi Dmitry,
>
> On wo, 2016-02-03 at 17:16 +0100, Paul Bolle wrote:
>> The above should provide me with enough information to figure out
>> what's going on here.
>
> I've instrumented ser_gigaset with some printk's. Basically I added the
> stuff pasted at the end of this message. In 10.000 runs of the program
> syzkaller generated the added printk's suggest that struct ser_cardstate
> is freed every time.
>
> (Note that this was done on a machine that, probably like the VM
> syzkaller was running in, doesn't have the clunky hardware that this
> driver manages attached.)
>
> Before I dive deeper into this: can you reproduce this leak? Is it
> perhaps a one in gazillion runs thing? Do you have the logs of a run
> that warned about this leak at hand?
>
> Thanks,
>
>
> Paul Bolle
>
> @@ -375,9 +377,12 @@ static void gigaset_device_release(struct device *dev)
> {
> struct cardstate *cs = dev_get_drvdata(dev);
>
> - if (!cs)
> + if (!cs) {
> + pr_info("%s: no cardstate", __func__);
> return;
> + }
> dev_set_drvdata(dev, NULL);
> + pr_info("%s: kfree(%p)", __func__, cs->hw.ser);
> kfree(cs->hw.ser);
> cs->hw.ser = NULL;
> }
> @@ -392,6 +397,7 @@ static int gigaset_initcshw(struct cardstate *cs)
> struct ser_cardstate *scs;
>
> scs = kzalloc(sizeof(struct ser_cardstate), GFP_KERNEL);
> + pr_info("%s: scs = %p", __func__, scs);
> if (!scs) {
> pr_err("out of memory\n");
> return -ENOMEM;
Forgot to mention that you need to run it in a parallel loop, sorry.
This one should do:
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include
#include
#include
#include
#include
#include
#include
#include
void work()
{
long r[7];
memset(r, -1, sizeof(r));
r[0] = syscall(SYS_mmap, 0x2000ul, 0x1ul, 0x3ul, 0x32ul,
0xul, 0x0ul);
r[2] = syscall(SYS_open, "/dev/ptmx", 0x8002ul, 0x0ul, 0, 0, 0);
*(uint32_t*)0x20002b1e = (uint32_t)0x10;
r[4] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20002b1eul, 0, 0, 0);
*(uint32_t*)0x20009000 = (uint32_t)0x7;
r[6] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20009000ul, 0, 0, 0);
}
int main() {
int running, status;
for (;;) {
while (running < 32) {
if (fork() == 0) {
work();
exit(0);
}
running++;
}
if (wait(&status) > 0)
running--;
}
}
While running it, sample/proc/slabinfo with:
# cat /proc/slabinfo | egrep "^kmalloc-2048"
It constantly grows.
Re: gigaset: memory leak in gigaset_initcshw
Hi Dmitry,
On wo, 2016-02-03 at 17:16 +0100, Paul Bolle wrote:
> The above should provide me with enough information to figure out
> what's going on here.
I've instrumented ser_gigaset with some printk's. Basically I added the
stuff pasted at the end of this message. In 10.000 runs of the program
syzkaller generated the added printk's suggest that struct ser_cardstate
is freed every time.
(Note that this was done on a machine that, probably like the VM
syzkaller was running in, doesn't have the clunky hardware that this
driver manages attached.)
Before I dive deeper into this: can you reproduce this leak? Is it
perhaps a one in gazillion runs thing? Do you have the logs of a run
that warned about this leak at hand?
Thanks,
Paul Bolle
@@ -375,9 +377,12 @@ static void gigaset_device_release(struct device *dev)
{
struct cardstate *cs = dev_get_drvdata(dev);
- if (!cs)
+ if (!cs) {
+ pr_info("%s: no cardstate", __func__);
return;
+ }
dev_set_drvdata(dev, NULL);
+ pr_info("%s: kfree(%p)", __func__, cs->hw.ser);
kfree(cs->hw.ser);
cs->hw.ser = NULL;
}
@@ -392,6 +397,7 @@ static int gigaset_initcshw(struct cardstate *cs)
struct ser_cardstate *scs;
scs = kzalloc(sizeof(struct ser_cardstate), GFP_KERNEL);
+ pr_info("%s: scs = %p", __func__, scs);
if (!scs) {
pr_err("out of memory\n");
return -ENOMEM;
Re: gigaset: memory leak in gigaset_initcshw
On wo, 2016-02-03 at 16:31 +0100, Dmitry Vyukov wrote:
> The following program causes a memory leak of ser_cardstate object
> allocated in gigaset_initcshw:
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include
> #include
> #include
> #include
> #include
>
> int main()
> {
> long r[7];
> memset(r, -1, sizeof(r));
> r[0] = syscall(SYS_mmap, 0x2000ul, 0x1ul, 0x3ul, 0x32ul,
> 0xul, 0x0ul);
> r[2] = syscall(SYS_open, "/dev/ptmx", 0x8002ul, 0x0ul, 0, 0, 0);
> *(uint32_t*)0x20002b1e = (uint32_t)0x10;
> r[4] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20002b1eul, 0, 0, 0);
> *(uint32_t*)0x20009000 = (uint32_t)0x7;
> r[6] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20009000ul, 0, 0, 0);
strace translated this for me to
mmap(0x2000, 65536, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x2000
open("/dev/ptmx", O_RDWR|O_LARGEFILE) = 3
ioctl(3, TIOCSETD, [16])= 0
ioctl(3, TIOCSETD, [7]) = 0
Where 16 is N_GIGASET_M101, while 7 is N_6PACK.
> return 0;
> }
>
> unreferenced object 0x88002b4109d8 (size 2048):
> comm "a.out", pid 9565, jiffies 4301785161 (age 10.646s)
> hex dump (first 32 bytes):
> e0 30 0a 87 ff ff ff ff 00 00 00 00 00 00 00 00 .0..
> 80 af 0d 88 ff ff ff ff 88 08 5e 00 00 88 ff ff ..^.
> backtrace:
> [< inline >] kzalloc include/linux/slab.h:607
> [] gigaset_initcshw+0x4b/0x2b0
> drivers/isdn/gigaset/ser-gigaset.c:394
> [] gigaset_initcs+0xf82/0x1660
> drivers/isdn/gigaset/common.c:748
> [] gigaset_tty_open+0x9b/0x460
> drivers/isdn/gigaset/ser-gigaset.c:516
> [] tty_ldisc_open.isra.2+0x78/0xd0
> drivers/tty/tty_ldisc.c:454
> [] tty_set_ldisc+0x292/0x8a0
> drivers/tty/tty_ldisc.c:561
> [< inline >] tiocsetd drivers/tty/tty_io.c:2656
> [] tty_ioctl+0xb2e/0x2160
> drivers/tty/tty_io.c:2911
> [< inline >] vfs_ioctl fs/ioctl.c:43
> [] do_vfs_ioctl+0x18c/0xfb0 fs/ioctl.c:674
> [< inline >] SYSC_ioctl fs/ioctl.c:689
> [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:680
> [] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
The above should provide me with enough information to figure out what's
going on here.
> On commit 34229b277480f46c1e9a19f027f30b074512e68b.
That's "Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net",
of two days ago.
Thanks for the report,
Paul Bolle
gigaset: memory leak in gigaset_initcshw
Hello,
The following program causes a memory leak of ser_cardstate object
allocated in gigaset_initcshw:
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include
#include
#include
#include
#include
int main()
{
long r[7];
memset(r, -1, sizeof(r));
r[0] = syscall(SYS_mmap, 0x2000ul, 0x1ul, 0x3ul, 0x32ul,
0xul, 0x0ul);
r[2] = syscall(SYS_open, "/dev/ptmx", 0x8002ul, 0x0ul, 0, 0, 0);
*(uint32_t*)0x20002b1e = (uint32_t)0x10;
r[4] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20002b1eul, 0, 0, 0);
*(uint32_t*)0x20009000 = (uint32_t)0x7;
r[6] = syscall(SYS_ioctl, r[2], 0x5423ul, 0x20009000ul, 0, 0, 0);
return 0;
}
unreferenced object 0x88002b4109d8 (size 2048):
comm "a.out", pid 9565, jiffies 4301785161 (age 10.646s)
hex dump (first 32 bytes):
e0 30 0a 87 ff ff ff ff 00 00 00 00 00 00 00 00 .0..
80 af 0d 88 ff ff ff ff 88 08 5e 00 00 88 ff ff ..^.
backtrace:
[< inline >] kzalloc include/linux/slab.h:607
[] gigaset_initcshw+0x4b/0x2b0
drivers/isdn/gigaset/ser-gigaset.c:394
[] gigaset_initcs+0xf82/0x1660
drivers/isdn/gigaset/common.c:748
[] gigaset_tty_open+0x9b/0x460
drivers/isdn/gigaset/ser-gigaset.c:516
[] tty_ldisc_open.isra.2+0x78/0xd0
drivers/tty/tty_ldisc.c:454
[] tty_set_ldisc+0x292/0x8a0 drivers/tty/tty_ldisc.c:561
[< inline >] tiocsetd drivers/tty/tty_io.c:2656
[] tty_ioctl+0xb2e/0x2160 drivers/tty/tty_io.c:2911
[< inline >] vfs_ioctl fs/ioctl.c:43
[] do_vfs_ioctl+0x18c/0xfb0 fs/ioctl.c:674
[< inline >] SYSC_ioctl fs/ioctl.c:689
[] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:680
[] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
On commit 34229b277480f46c1e9a19f027f30b074512e68b.
