From: Ariel Marcovitch <arielmarcovitch@gmail.com>
To: catalin.marinas@arm.com, akpm@linux-foundation.org,
mpe@ellerman.id.au, benh@kernel.crashing.org, paulus@samba.org,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Cc: christophe.leroy@csgroup.eu
Subject: Re: False positive kmemleak report for dtb properties names on powerpc
Date: Fri, 18 Mar 2022 21:44:38 +0200 [thread overview]
Message-ID: <5d7caa39-9156-0db1-688c-eafe4007a492@gmail.com> (raw)
In-Reply-To: <1e976df3-2925-f6c6-6723-67f127b9e544@gmail.com>
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!
>>
next prev parent reply other threads:[~2022-03-18 19:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-24 22:27 False positive kmemleak report for dtb properties names on powerpc Ariel Marcovitch
2022-03-18 19:44 ` Ariel Marcovitch [this message]
-- strict thread matches above, loose matches on Subject: below --
2022-02-18 19:45 Ariel Marcovitch
2022-03-23 17:22 ` Catalin Marinas
2022-03-23 19:06 ` Mike Rapoport
2022-04-09 13:47 ` Ariel Marcovitch
2022-04-11 9:10 ` Christophe Leroy
2022-04-12 6:47 ` Michael Ellerman
2022-04-12 17:56 ` Mike Rapoport
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5d7caa39-9156-0db1-688c-eafe4007a492@gmail.com \
--to=arielmarcovitch@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=catalin.marinas@arm.com \
--cc=christophe.leroy@csgroup.eu \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).