linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
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
Subject: False positive kmemleak report for dtb properties names on powerpc
Date: Fri, 18 Feb 2022 21:45:51 +0200	[thread overview]
Message-ID: <9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com> (raw)

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!



             reply	other threads:[~2022-02-18 19:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-18 19:45 Ariel Marcovitch [this message]
2022-03-23 17:22 ` False positive kmemleak report for dtb properties names on powerpc 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
2022-02-24 22:27 Ariel Marcovitch
2022-03-18 19:44 ` Ariel Marcovitch

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=9dd08bb5-f39e-53d8-f88d-bec598a08c93@gmail.com \
    --to=arielmarcovitch@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --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).