Messages in this thread | | | Date | Fri, 18 Mar 2022 21:44:38 +0200 | Subject | Re: False positive kmemleak report for dtb properties names on powerpc | From | Ariel Marcovitch <> |
| |
Pinging again :)
On 25/02/2022 0:27, Ariel Marcovitch wrote: > Ping :) > > On 18/02/2022 21:45, Ariel Marcovitch wrote: >> 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! >>
| |