Date:   Fri, 18 Feb 2022 21:45:51 +0200
From:   Ariel Marcovitch <arielmarcovitch@...il.com>
To:     catalin.marinas@....com, akpm@...ux-foundation.org,
        mpe@...erman.id.au, benh@...nel.crashing.org, paulus@...ba.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org
Subject: False positive kmemleak report for dtb properties names on powerpc

Hello!

I was running a powerpc 32bit kernel (built using 
qemu_ppc_mpc8544ds_defconfig
buildroot config, with enabling DEBUGFS+KMEMLEAK+HIGHMEM in the kernel 
config)
on qemu and invoked the kmemleak scan (twice. for some reason the first 
time wasn't enough).

(Actually the problem will probably reproduce on every ppc kernel with
HIGHMEM enabled, but I only checked this config)

I got 97 leak reports, all similar to the following:

```

unreferenced object 0xc1803840 (size 16):
   comm "swapper", pid 1, jiffies 4294892303 (age 39.320s)
   hex dump (first 16 bytes):
     64 65 76 69 63 65 5f 74 79 70 65 00 00 00 00 00 device_type.....
   backtrace:
     [<(ptrval)>] kstrdup+0x40/0x98
     [<(ptrval)>] __of_add_property_sysfs+0xa4/0x10c
     [<(ptrval)>] __of_attach_node_sysfs+0xc0/0x110
     [<(ptrval)>] of_core_init+0xa8/0x15c
     [<(ptrval)>] driver_init+0x24/0x3c
     [<(ptrval)>] kernel_init_freeable+0xb8/0x23c
     [<(ptrval)>] kernel_init+0x24/0x14c
     [<(ptrval)>] ret_from_kernel_thread+0x5c/0x64
```

The objects in the reports are the names of the sysfs files created for 
the dtb
nodes and properties.

These are definitely not leaked, as they are even visible to the user as 
the sysfs file names.

These strings (for dtb properties, in the case of the shown report, but 
the case with dtb nodes is very similar) are created in 
__of_add_property_sysfs() and the pointer to them is stored in 
pp->attr.attr.name (so, actually stored in the memory pointed by pp)

pp is one of the dtb property objects which are allocated in 
early_init_dt_alloc_memory_arch() in of/fdt.c using memblock_alloc. This 
happens very early, in setup_arch()->unflatten_device_tree().

memblock_alloc lets kmemleak know about the allocated memory using 
kmemleak_alloc_phys (in mm/memblock.c:memblock_alloc_range_nid()).

The problem is with the following code (mm/kmemleak.c):

```c

void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
                                gfp_t gfp)
{
         if (!IS_ENABLED(CONFIG_HIGHMEM) || PHYS_PFN(phys) < max_low_pfn)
                 kmemleak_alloc(__va(phys), size, min_count, gfp);
}

```

When CONFIG_HIGHMEM is enabled, the pfn of the allocated memory is 
checked against max_low_pfn, to make sure it is not in the HIGHMEM zone.

However, when called through unflatten_device_tree(), max_low_pfn is not 
yet initialized in powerpc.

max_low_pfn is initialized (when NUMA is disabled) in 
arch/powerpc/mm/mem.c:mem_topology_setup() which is called only after 
unflatten_device_tree() is called in the same function (setup_arch()).

Because max_low_pfn is global it is 0 before initialization, so as far 
as kmemleak_alloc_phys() is concerned, every memory is HIGHMEM (: and 
the allocated memory is not tracked by kmemleak, causing references to 
objects allocated later with kmalloc() to be ignored and these objects 
are marked as leaked.

I actually tried to find out whether this happen on other arches as 
well, and it seems like arm64 also have this problem when dtb is used 
instead of acpi, although I haven't had the chance to confirm this.

I don't suppose I can just shuffle the calls in setup_arch() around, so 
I wanted to hear your opinions first

Thanks!

Powered by blists - more mailing lists