Re: kmemleak: 1157 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

2019-05-29 Thread Catalin Marinas
On Tue, May 28, 2019 at 09:14:12PM +0200, Mathieu Malaterre wrote:
> On Tue, May 28, 2019 at 7:21 AM Michael Ellerman  wrote:
> > Mathieu Malaterre  writes:
> > > Is there a way to dump more context (somewhere in of tree
> > > flattening?). I cannot make sense of the following:
> >
> > Hmm. Not that I know of.
> >
> > Those don't look related to OF flattening/unflattening. That's just
> > sysfs setup based on the unflattened device tree.
> >
> > The allocations are happening in safe_name() AFAICS.
> >
> > int __of_add_property_sysfs(struct device_node *np, struct property *pp)
> > {
> > ...
> > pp->attr.attr.name = safe_name(>kobj, pp->name);
> >
> > And the free is in __of_sysfs_remove_bin_file():
> >
> > void __of_sysfs_remove_bin_file(struct device_node *np, struct property 
> > *prop)
> > {
> > if (!IS_ENABLED(CONFIG_SYSFS))
> > return;
> >
> > sysfs_remove_bin_file(>kobj, >attr);
> > kfree(prop->attr.attr.name);
> >
> 
> Right. That helped a lot !
> 
> > There is this check which could be failing leading to us not calling the
> > free at all:
> >
> > void __of_remove_property_sysfs(struct device_node *np, struct property 
> > *prop)
> > {
> > /* at early boot, bail here and defer setup to of_init() */
> > if (of_kset && of_node_is_attached(np))
> > __of_sysfs_remove_bin_file(np, prop);
> > }
> >
> >
> > So maybe stick a printk() in there to see if you're hitting that
> > condition, eg something like:
> >
> > if (of_kset && of_node_is_attached(np))
> > __of_sysfs_remove_bin_file(np, prop);
> > else
> > printk("%s: leaking prop %s on node %pOF\n", __func__, 
> > prop->attr.attr.name, np);
> >
> 
> If I understand correctly those are false positive. I was first
> starting to consider using something like kmemleak_not_leak, but I
> remember that I have been using kmemleak for a couple of years now.
> Those reports starting to show up only recently.
> 
> Catalin, do you have an idea why on a non-SMP machine kmemleak reports
> leaks from:
> 
> [...]
> void __init of_core_init(void)
> {
> [...]
>  for_each_of_allnodes(np)
> __of_attach_node_sysfs(np);

It's likely that they are false positives but usually, rather than just
adding a kmemleak_not_leak(), it's better to figure out why kmemleak
reports them. The strings allocated above through kstrdup() can't be
tracked starting with the root objects. I think for the of stuff, this
should be the of_root pointer.

Is it only with non-SMP that this happens? I can't reproduce it on arm64
to be able to dig further.

Even better if you could bisect to the commit that's causing this.

-- 
Catalin


Re: kmemleak: 1157 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

2019-05-28 Thread Mathieu Malaterre
Hi Michael !

Thanks for the kind help.

On Tue, May 28, 2019 at 7:21 AM Michael Ellerman  wrote:
>
> Mathieu Malaterre  writes:
> > Hi there,
> >
> > Is there a way to dump more context (somewhere in of tree
> > flattening?). I cannot make sense of the following:
>
> Hmm. Not that I know of.
>
> Those don't look related to OF flattening/unflattening. That's just
> sysfs setup based on the unflattened device tree.
>
> The allocations are happening in safe_name() AFAICS.
>
> int __of_add_property_sysfs(struct device_node *np, struct property *pp)
> {
> ...
> pp->attr.attr.name = safe_name(>kobj, pp->name);
>
> And the free is in __of_sysfs_remove_bin_file():
>
> void __of_sysfs_remove_bin_file(struct device_node *np, struct property *prop)
> {
> if (!IS_ENABLED(CONFIG_SYSFS))
> return;
>
> sysfs_remove_bin_file(>kobj, >attr);
> kfree(prop->attr.attr.name);
>

Right. That helped a lot !

> There is this check which could be failing leading to us not calling the
> free at all:
>
> void __of_remove_property_sysfs(struct device_node *np, struct property *prop)
> {
> /* at early boot, bail here and defer setup to of_init() */
> if (of_kset && of_node_is_attached(np))
> __of_sysfs_remove_bin_file(np, prop);
> }
>
>
> So maybe stick a printk() in there to see if you're hitting that
> condition, eg something like:
>
> if (of_kset && of_node_is_attached(np))
> __of_sysfs_remove_bin_file(np, prop);
> else
> printk("%s: leaking prop %s on node %pOF\n", __func__, 
> prop->attr.attr.name, np);
>

If I understand correctly those are false positive. I was first
starting to consider using something like kmemleak_not_leak, but I
remember that I have been using kmemleak for a couple of years now.
Those reports starting to show up only recently.

Catalin, do you have an idea why on a non-SMP machine kmemleak reports
leaks from:

[...]
void __init of_core_init(void)
{
[...]
 for_each_of_allnodes(np)
__of_attach_node_sysfs(np);



> cheers
>
> > kmemleak: 1157 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
> >
> > Where:
> >
> > # head -40 /sys/kernel/debug/kmemleak
> > unreferenced object 0xdf44d180 (size 8):
> >   comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
> >   hex dump (first 8 bytes):
> > 62 61 73 65 00 00 00 00  base
> >   backtrace:
> > [<0ca59825>] kstrdup+0x4c/0xb8
> > [] kobject_set_name_vargs+0x34/0xc8
> > [<661b4c86>] kobject_add+0x78/0x120
> > [] __of_attach_node_sysfs+0xa0/0x14c
> > [<2a143d10>] of_core_init+0x90/0x114
> > [] driver_init+0x30/0x48
> > [<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
> > [] kernel_init+0x20/0x110
> > [] ret_from_kernel_thread+0x14/0x1c
> > unreferenced object 0xdf44d178 (size 8):
> >   comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
> >   hex dump (first 8 bytes):
> > 6d 6f 64 65 6c 00 97 c8  model...
> >   backtrace:
> > [<0ca59825>] kstrdup+0x4c/0xb8
> > [<0eeb0a3b>] __of_add_property_sysfs+0x88/0x12c
> > [] __of_attach_node_sysfs+0xcc/0x14c
> > [<2a143d10>] of_core_init+0x90/0x114
> > [] driver_init+0x30/0x48
> > [<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
> > [] kernel_init+0x20/0x110
> > [] ret_from_kernel_thread+0x14/0x1c
> > unreferenced object 0xdf4021e0 (size 16):
> >   comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
> >   hex dump (first 16 bytes):
> > 63 6f 6d 70 61 74 69 62 6c 65 00 01 00 00 00 00  compatible..
> >   backtrace:
> > [<0ca59825>] kstrdup+0x4c/0xb8
> > [<0eeb0a3b>] __of_add_property_sysfs+0x88/0x12c
> > [] __of_attach_node_sysfs+0xcc/0x14c
> > [<2a143d10>] of_core_init+0x90/0x114
> > [] driver_init+0x30/0x48
> > [<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
> > [] kernel_init+0x20/0x110
> > [] ret_from_kernel_thread+0x14/0x1c


Re: kmemleak: 1157 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

2019-05-27 Thread Michael Ellerman
Mathieu Malaterre  writes:
> Hi there,
>
> Is there a way to dump more context (somewhere in of tree
> flattening?). I cannot make sense of the following:

Hmm. Not that I know of.

Those don't look related to OF flattening/unflattening. That's just
sysfs setup based on the unflattened device tree.

The allocations are happening in safe_name() AFAICS.

int __of_add_property_sysfs(struct device_node *np, struct property *pp)
{
...
pp->attr.attr.name = safe_name(>kobj, pp->name);

And the free is in __of_sysfs_remove_bin_file():

void __of_sysfs_remove_bin_file(struct device_node *np, struct property *prop)
{
if (!IS_ENABLED(CONFIG_SYSFS))
return;

sysfs_remove_bin_file(>kobj, >attr);
kfree(prop->attr.attr.name);


There is this check which could be failing leading to us not calling the
free at all:

void __of_remove_property_sysfs(struct device_node *np, struct property *prop)
{
/* at early boot, bail here and defer setup to of_init() */
if (of_kset && of_node_is_attached(np))
__of_sysfs_remove_bin_file(np, prop);
}


So maybe stick a printk() in there to see if you're hitting that
condition, eg something like:

if (of_kset && of_node_is_attached(np))
__of_sysfs_remove_bin_file(np, prop);
else
printk("%s: leaking prop %s on node %pOF\n", __func__, 
prop->attr.attr.name, np);


cheers

> kmemleak: 1157 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
>
> Where:
>
> # head -40 /sys/kernel/debug/kmemleak
> unreferenced object 0xdf44d180 (size 8):
>   comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
>   hex dump (first 8 bytes):
> 62 61 73 65 00 00 00 00  base
>   backtrace:
> [<0ca59825>] kstrdup+0x4c/0xb8
> [] kobject_set_name_vargs+0x34/0xc8
> [<661b4c86>] kobject_add+0x78/0x120
> [] __of_attach_node_sysfs+0xa0/0x14c
> [<2a143d10>] of_core_init+0x90/0x114
> [] driver_init+0x30/0x48
> [<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
> [] kernel_init+0x20/0x110
> [] ret_from_kernel_thread+0x14/0x1c
> unreferenced object 0xdf44d178 (size 8):
>   comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
>   hex dump (first 8 bytes):
> 6d 6f 64 65 6c 00 97 c8  model...
>   backtrace:
> [<0ca59825>] kstrdup+0x4c/0xb8
> [<0eeb0a3b>] __of_add_property_sysfs+0x88/0x12c
> [] __of_attach_node_sysfs+0xcc/0x14c
> [<2a143d10>] of_core_init+0x90/0x114
> [] driver_init+0x30/0x48
> [<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
> [] kernel_init+0x20/0x110
> [] ret_from_kernel_thread+0x14/0x1c
> unreferenced object 0xdf4021e0 (size 16):
>   comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
>   hex dump (first 16 bytes):
> 63 6f 6d 70 61 74 69 62 6c 65 00 01 00 00 00 00  compatible..
>   backtrace:
> [<0ca59825>] kstrdup+0x4c/0xb8
> [<0eeb0a3b>] __of_add_property_sysfs+0x88/0x12c
> [] __of_attach_node_sysfs+0xcc/0x14c
> [<2a143d10>] of_core_init+0x90/0x114
> [] driver_init+0x30/0x48
> [<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
> [] kernel_init+0x20/0x110
> [] ret_from_kernel_thread+0x14/0x1c


kmemleak: 1157 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

2019-05-23 Thread Mathieu Malaterre
Hi there,

Is there a way to dump more context (somewhere in of tree
flattening?). I cannot make sense of the following:

kmemleak: 1157 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

Where:

# head -40 /sys/kernel/debug/kmemleak
unreferenced object 0xdf44d180 (size 8):
  comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
  hex dump (first 8 bytes):
62 61 73 65 00 00 00 00  base
  backtrace:
[<0ca59825>] kstrdup+0x4c/0xb8
[] kobject_set_name_vargs+0x34/0xc8
[<661b4c86>] kobject_add+0x78/0x120
[] __of_attach_node_sysfs+0xa0/0x14c
[<2a143d10>] of_core_init+0x90/0x114
[] driver_init+0x30/0x48
[<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
[] kernel_init+0x20/0x110
[] ret_from_kernel_thread+0x14/0x1c
unreferenced object 0xdf44d178 (size 8):
  comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
  hex dump (first 8 bytes):
6d 6f 64 65 6c 00 97 c8  model...
  backtrace:
[<0ca59825>] kstrdup+0x4c/0xb8
[<0eeb0a3b>] __of_add_property_sysfs+0x88/0x12c
[] __of_attach_node_sysfs+0xcc/0x14c
[<2a143d10>] of_core_init+0x90/0x114
[] driver_init+0x30/0x48
[<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
[] kernel_init+0x20/0x110
[] ret_from_kernel_thread+0x14/0x1c
unreferenced object 0xdf4021e0 (size 16):
  comm "swapper", pid 1, jiffies 4294892297 (age 4766.460s)
  hex dump (first 16 bytes):
63 6f 6d 70 61 74 69 62 6c 65 00 01 00 00 00 00  compatible..
  backtrace:
[<0ca59825>] kstrdup+0x4c/0xb8
[<0eeb0a3b>] __of_add_property_sysfs+0x88/0x12c
[] __of_attach_node_sysfs+0xcc/0x14c
[<2a143d10>] of_core_init+0x90/0x114
[] driver_init+0x30/0x48
[<84ed01b1>] kernel_init_freeable+0xfc/0x3fc
[] kernel_init+0x20/0x110
[] ret_from_kernel_thread+0x14/0x1c