linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
@ 2016-08-11 11:50 Vignesh R
  2016-08-11 15:54 ` Catalin Marinas
  0 siblings, 1 reply; 8+ messages in thread
From: Vignesh R @ 2016-08-11 11:50 UTC (permalink / raw)
  To: linux-mm; +Cc: linux-kernel, linux-omap, Catalin Marinas

Hi,


I see the below message from kmemleak when booting linux-next on AM335x
GP EVM and DRA7 EVM

[    0.803934] kmemleak: Cannot insert 0xff7f1000 into the object search
tree (overlaps existing)
[    0.803950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.8.0-rc1-next-20160809 #497
[    0.803958] Hardware name: Generic DRA72X (Flattened Device Tree)
[    0.803979] [<c0110104>] (unwind_backtrace) from [<c010c24c>]
(show_stack+0x10/0x14)
[    0.803994] [<c010c24c>] (show_stack) from [<c0490df0>]
(dump_stack+0xac/0xe0)
[    0.804010] [<c0490df0>] (dump_stack) from [<c0296f88>]
(create_object+0x214/0x278)
[    0.804025] [<c0296f88>] (create_object) from [<c07c770c>]
(kmemleak_alloc_percpu+0x54/0xc0)
[    0.804038] [<c07c770c>] (kmemleak_alloc_percpu) from [<c025fb08>]
(pcpu_alloc+0x368/0x5fc)
[    0.804052] [<c025fb08>] (pcpu_alloc) from [<c0b1bfbc>]
(crash_notes_memory_init+0x10/0x40)
[    0.804064] [<c0b1bfbc>] (crash_notes_memory_init) from [<c010188c>]
(do_one_initcall+0x3c/0x178)
[    0.804075] [<c010188c>] (do_one_initcall) from [<c0b00e98>]
(kernel_init_freeable+0x1fc/0x2c8)
[    0.804086] [<c0b00e98>] (kernel_init_freeable) from [<c07c66b0>]
(kernel_init+0x8/0x114)
[    0.804098] [<c07c66b0>] (kernel_init) from [<c0107910>]
(ret_from_fork+0x14/0x24)
[    0.804106] kmemleak: Kernel memory leak detector disabled
[    0.804113] kmemleak: Object 0xfe800000 (size 16777216):
[    0.804121] kmemleak:   comm "swapper/0", pid 0, jiffies 4294937296
[    0.804127] kmemleak:   min_count = -1
[    0.804132] kmemleak:   count = 0
[    0.804138] kmemleak:   flags = 0x5
[    0.804143] kmemleak:   checksum = 0
[    0.804149] kmemleak:   backtrace:
[    0.804155]      [<c0b26a90>] cma_declare_contiguous+0x16c/0x214
[    0.804170]      [<c0b3c9c0>] dma_contiguous_reserve_area+0x30/0x64
[    0.804183]      [<c0b3ca74>] dma_contiguous_reserve+0x80/0x94
[    0.804195]      [<c0b06810>] arm_memblock_init+0x130/0x184
[    0.804207]      [<c0b04214>] setup_arch+0x590/0xc08
[    0.804217]      [<c0b00940>] start_kernel+0x58/0x3b4
[    0.804227]      [<8000807c>] 0x8000807c
[    0.804237]      [<ffffffff>] 0xffffffff



This happens early in the boot and the stack dump depends on the driver
that is being probed at that moment (and therefore I believe its a
generic issue).
Full boot log here: http://pastebin.ubuntu.com/23014650/
Config used: omap2plus_defconfig +
CONFIG_HAVE_DEBUG_KMEMLEAK=y
CONFIG_DEBUG_KMEMLEAK=y
CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE=8000


Has anyone seen this issue before?

Any help appreciated. Thanks!

-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
  2016-08-11 11:50 kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing) Vignesh R
@ 2016-08-11 15:54 ` Catalin Marinas
  2016-08-11 16:48   ` Grygorii Strashko
  0 siblings, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2016-08-11 15:54 UTC (permalink / raw)
  To: Vignesh R; +Cc: linux-mm, linux-kernel, linux-omap

On Thu, Aug 11, 2016 at 05:20:51PM +0530, Vignesh R wrote:
> I see the below message from kmemleak when booting linux-next on AM335x
> GP EVM and DRA7 EVM

Can you also reproduce it with 4.8-rc1?

> [    0.803934] kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
> [    0.803950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1-next-20160809 #497
> [    0.803958] Hardware name: Generic DRA72X (Flattened Device Tree)
> [    0.803979] [<c0110104>] (unwind_backtrace) from [<c010c24c>] (show_stack+0x10/0x14)
> [    0.803994] [<c010c24c>] (show_stack) from [<c0490df0>] (dump_stack+0xac/0xe0)
> [    0.804010] [<c0490df0>] (dump_stack) from [<c0296f88>] (create_object+0x214/0x278)
> [    0.804025] [<c0296f88>] (create_object) from [<c07c770c>] (kmemleak_alloc_percpu+0x54/0xc0)
> [    0.804038] [<c07c770c>] (kmemleak_alloc_percpu) from [<c025fb08>] (pcpu_alloc+0x368/0x5fc)
> [    0.804052] [<c025fb08>] (pcpu_alloc) from [<c0b1bfbc>] (crash_notes_memory_init+0x10/0x40)
> [    0.804064] [<c0b1bfbc>] (crash_notes_memory_init) from [<c010188c>] (do_one_initcall+0x3c/0x178)
> [    0.804075] [<c010188c>] (do_one_initcall) from [<c0b00e98>] (kernel_init_freeable+0x1fc/0x2c8)
> [    0.804086] [<c0b00e98>] (kernel_init_freeable) from [<c07c66b0>] (kernel_init+0x8/0x114)
> [    0.804098] [<c07c66b0>] (kernel_init) from [<c0107910>] (ret_from_fork+0x14/0x24)

This is the allocation stack trace, going via pcpu_alloc().

> [    0.804106] kmemleak: Kernel memory leak detector disabled
> [    0.804113] kmemleak: Object 0xfe800000 (size 16777216):
> [    0.804121] kmemleak:   comm "swapper/0", pid 0, jiffies 4294937296
> [    0.804127] kmemleak:   min_count = -1
> [    0.804132] kmemleak:   count = 0
> [    0.804138] kmemleak:   flags = 0x5
> [    0.804143] kmemleak:   checksum = 0
> [    0.804149] kmemleak:   backtrace:
> [    0.804155]      [<c0b26a90>] cma_declare_contiguous+0x16c/0x214
> [    0.804170]      [<c0b3c9c0>] dma_contiguous_reserve_area+0x30/0x64
> [    0.804183]      [<c0b3ca74>] dma_contiguous_reserve+0x80/0x94
> [    0.804195]      [<c0b06810>] arm_memblock_init+0x130/0x184
> [    0.804207]      [<c0b04214>] setup_arch+0x590/0xc08
> [    0.804217]      [<c0b00940>] start_kernel+0x58/0x3b4
> [    0.804227]      [<8000807c>] 0x8000807c
> [    0.804237]      [<ffffffff>] 0xffffffff

This seems to be the original object that was allocated via
cma_declare_contiguous(): 16MB range from 0xfe800000 to 0xff800000.
Since the pointer returned by pcpu_alloc is 0xff7f1000 falls in the 16MB
CMA range, kmemleak gets confused (it doesn't allow overlapping
objects).

So what I think goes wrong is that the kmemleak_alloc(__va(found)) call
in memblock_alloc_range_nid() doesn't get the right value for the VA of
the CMA block. The memblock_alloc_range() call in
cma_declare_contiguous() asks for memory above high_memory, hence on a
32-bit architecture with highmem enabled, __va() use is not really
valid, returning the wrong address. The existing kmemleak object is
bogus, it shouldn't have been created in the first place.

Now I'm trying to figure out how to differentiate between lowmem
memblocks and highmem ones. Ignoring the kmemleak_alloc() calls
altogether in mm/memblock.c is probably not an option as it would lead
to lots of false positives.

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
  2016-08-11 15:54 ` Catalin Marinas
@ 2016-08-11 16:48   ` Grygorii Strashko
  2016-08-11 17:08     ` Catalin Marinas
  0 siblings, 1 reply; 8+ messages in thread
From: Grygorii Strashko @ 2016-08-11 16:48 UTC (permalink / raw)
  To: Catalin Marinas, Vignesh R; +Cc: linux-mm, linux-kernel, linux-omap

On 08/11/2016 06:54 PM, Catalin Marinas wrote:
> On Thu, Aug 11, 2016 at 05:20:51PM +0530, Vignesh R wrote:
>> I see the below message from kmemleak when booting linux-next on AM335x
>> GP EVM and DRA7 EVM
> 
> Can you also reproduce it with 4.8-rc1?
> 
>> [    0.803934] kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
>> [    0.803950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1-next-20160809 #497
>> [    0.803958] Hardware name: Generic DRA72X (Flattened Device Tree)
>> [    0.803979] [<c0110104>] (unwind_backtrace) from [<c010c24c>] (show_stack+0x10/0x14)
>> [    0.803994] [<c010c24c>] (show_stack) from [<c0490df0>] (dump_stack+0xac/0xe0)
>> [    0.804010] [<c0490df0>] (dump_stack) from [<c0296f88>] (create_object+0x214/0x278)
>> [    0.804025] [<c0296f88>] (create_object) from [<c07c770c>] (kmemleak_alloc_percpu+0x54/0xc0)
>> [    0.804038] [<c07c770c>] (kmemleak_alloc_percpu) from [<c025fb08>] (pcpu_alloc+0x368/0x5fc)
>> [    0.804052] [<c025fb08>] (pcpu_alloc) from [<c0b1bfbc>] (crash_notes_memory_init+0x10/0x40)
>> [    0.804064] [<c0b1bfbc>] (crash_notes_memory_init) from [<c010188c>] (do_one_initcall+0x3c/0x178)
>> [    0.804075] [<c010188c>] (do_one_initcall) from [<c0b00e98>] (kernel_init_freeable+0x1fc/0x2c8)
>> [    0.804086] [<c0b00e98>] (kernel_init_freeable) from [<c07c66b0>] (kernel_init+0x8/0x114)
>> [    0.804098] [<c07c66b0>] (kernel_init) from [<c0107910>] (ret_from_fork+0x14/0x24)
> 
> This is the allocation stack trace, going via pcpu_alloc().
> 
>> [    0.804106] kmemleak: Kernel memory leak detector disabled
>> [    0.804113] kmemleak: Object 0xfe800000 (size 16777216):
>> [    0.804121] kmemleak:   comm "swapper/0", pid 0, jiffies 4294937296
>> [    0.804127] kmemleak:   min_count = -1
>> [    0.804132] kmemleak:   count = 0
>> [    0.804138] kmemleak:   flags = 0x5
>> [    0.804143] kmemleak:   checksum = 0
>> [    0.804149] kmemleak:   backtrace:
>> [    0.804155]      [<c0b26a90>] cma_declare_contiguous+0x16c/0x214
>> [    0.804170]      [<c0b3c9c0>] dma_contiguous_reserve_area+0x30/0x64
>> [    0.804183]      [<c0b3ca74>] dma_contiguous_reserve+0x80/0x94
>> [    0.804195]      [<c0b06810>] arm_memblock_init+0x130/0x184
>> [    0.804207]      [<c0b04214>] setup_arch+0x590/0xc08
>> [    0.804217]      [<c0b00940>] start_kernel+0x58/0x3b4
>> [    0.804227]      [<8000807c>] 0x8000807c
>> [    0.804237]      [<ffffffff>] 0xffffffff
> 
> This seems to be the original object that was allocated via
> cma_declare_contiguous(): 16MB range from 0xfe800000 to 0xff800000.
> Since the pointer returned by pcpu_alloc is 0xff7f1000 falls in the 16MB
> CMA range, kmemleak gets confused (it doesn't allow overlapping
> objects).
> 
> So what I think goes wrong is that the kmemleak_alloc(__va(found)) call
> in memblock_alloc_range_nid() doesn't get the right value for the VA of
> the CMA block. The memblock_alloc_range() call in
> cma_declare_contiguous() asks for memory above high_memory, hence on a
> 32-bit architecture with highmem enabled, __va() use is not really
> valid, returning the wrong address. The existing kmemleak object is
> bogus, it shouldn't have been created in the first place.
> 
> Now I'm trying to figure out how to differentiate between lowmem
> memblocks and highmem ones. Ignoring the kmemleak_alloc() calls
> altogether in mm/memblock.c is probably not an option as it would lead
> to lots of false positives.
> 

But cma_declare_contiguous() calls -
		/*
		 * kmemleak scans/reads tracked objects for pointers to other
		 * objects but this address isn't mapped and accessible
		 */
		kmemleak_ignore(phys_to_virt(addr));

Does it means above code is incorrect also?

It's a little bit strange that this can be seen only now, because
commit 95b0e655f9 ("ARM: mm: don't limit default CMA region only to low memor")
is pretty old.

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
  2016-08-11 16:48   ` Grygorii Strashko
@ 2016-08-11 17:08     ` Catalin Marinas
  2016-08-12  4:15       ` Vignesh R
  2016-08-12 13:50       ` Grygorii Strashko
  0 siblings, 2 replies; 8+ messages in thread
From: Catalin Marinas @ 2016-08-11 17:08 UTC (permalink / raw)
  To: Grygorii Strashko; +Cc: Vignesh R, linux-mm, linux-kernel, linux-omap

On Thu, Aug 11, 2016 at 07:48:12PM +0300, Grygorii Strashko wrote:
> On 08/11/2016 06:54 PM, Catalin Marinas wrote:
> > On Thu, Aug 11, 2016 at 05:20:51PM +0530, Vignesh R wrote:
> >> I see the below message from kmemleak when booting linux-next on AM335x
> >> GP EVM and DRA7 EVM
> > 
> > Can you also reproduce it with 4.8-rc1?
> > 
> >> [    0.803934] kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
> >> [    0.803950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1-next-20160809 #497
> >> [    0.803958] Hardware name: Generic DRA72X (Flattened Device Tree)
> >> [    0.803979] [<c0110104>] (unwind_backtrace) from [<c010c24c>] (show_stack+0x10/0x14)
> >> [    0.803994] [<c010c24c>] (show_stack) from [<c0490df0>] (dump_stack+0xac/0xe0)
> >> [    0.804010] [<c0490df0>] (dump_stack) from [<c0296f88>] (create_object+0x214/0x278)
> >> [    0.804025] [<c0296f88>] (create_object) from [<c07c770c>] (kmemleak_alloc_percpu+0x54/0xc0)
> >> [    0.804038] [<c07c770c>] (kmemleak_alloc_percpu) from [<c025fb08>] (pcpu_alloc+0x368/0x5fc)
> >> [    0.804052] [<c025fb08>] (pcpu_alloc) from [<c0b1bfbc>] (crash_notes_memory_init+0x10/0x40)
> >> [    0.804064] [<c0b1bfbc>] (crash_notes_memory_init) from [<c010188c>] (do_one_initcall+0x3c/0x178)
> >> [    0.804075] [<c010188c>] (do_one_initcall) from [<c0b00e98>] (kernel_init_freeable+0x1fc/0x2c8)
> >> [    0.804086] [<c0b00e98>] (kernel_init_freeable) from [<c07c66b0>] (kernel_init+0x8/0x114)
> >> [    0.804098] [<c07c66b0>] (kernel_init) from [<c0107910>] (ret_from_fork+0x14/0x24)
> > 
> > This is the allocation stack trace, going via pcpu_alloc().
> > 
> >> [    0.804106] kmemleak: Kernel memory leak detector disabled
> >> [    0.804113] kmemleak: Object 0xfe800000 (size 16777216):
> >> [    0.804121] kmemleak:   comm "swapper/0", pid 0, jiffies 4294937296
> >> [    0.804127] kmemleak:   min_count = -1
> >> [    0.804132] kmemleak:   count = 0
> >> [    0.804138] kmemleak:   flags = 0x5
> >> [    0.804143] kmemleak:   checksum = 0
> >> [    0.804149] kmemleak:   backtrace:
> >> [    0.804155]      [<c0b26a90>] cma_declare_contiguous+0x16c/0x214
> >> [    0.804170]      [<c0b3c9c0>] dma_contiguous_reserve_area+0x30/0x64
> >> [    0.804183]      [<c0b3ca74>] dma_contiguous_reserve+0x80/0x94
> >> [    0.804195]      [<c0b06810>] arm_memblock_init+0x130/0x184
> >> [    0.804207]      [<c0b04214>] setup_arch+0x590/0xc08
> >> [    0.804217]      [<c0b00940>] start_kernel+0x58/0x3b4
> >> [    0.804227]      [<8000807c>] 0x8000807c
> >> [    0.804237]      [<ffffffff>] 0xffffffff
> > 
> > This seems to be the original object that was allocated via
> > cma_declare_contiguous(): 16MB range from 0xfe800000 to 0xff800000.
> > Since the pointer returned by pcpu_alloc is 0xff7f1000 falls in the 16MB
> > CMA range, kmemleak gets confused (it doesn't allow overlapping
> > objects).
> > 
> > So what I think goes wrong is that the kmemleak_alloc(__va(found)) call
> > in memblock_alloc_range_nid() doesn't get the right value for the VA of
> > the CMA block. The memblock_alloc_range() call in
> > cma_declare_contiguous() asks for memory above high_memory, hence on a
> > 32-bit architecture with highmem enabled, __va() use is not really
> > valid, returning the wrong address. The existing kmemleak object is
> > bogus, it shouldn't have been created in the first place.
> > 
> > Now I'm trying to figure out how to differentiate between lowmem
> > memblocks and highmem ones. Ignoring the kmemleak_alloc() calls
> > altogether in mm/memblock.c is probably not an option as it would lead
> > to lots of false positives.
> 
> But cma_declare_contiguous() calls -
> 		/*
> 		 * kmemleak scans/reads tracked objects for pointers to other
> 		 * objects but this address isn't mapped and accessible
> 		 */
> 		kmemleak_ignore(phys_to_virt(addr));
> 
> Does it means above code is incorrect also?

Yes, as long as the phys_to_virt() use is invalid. You may get away with
this, depending on the SoC. Also, kmemleak_ignore() here is meant to
tell kmemleak not to bother with scanning or reporting such memory since
it is not meant for pointers but it still keeps track of it. The only
way to remove it from kmemleak is replace this with kmemleak_free(). But
That's more of a hack since phys_to_virt(addr) is still invalid.

> It's a little bit strange that this can be seen only now, because
> commit 95b0e655f9 ("ARM: mm: don't limit default CMA region only to low memor")
> is pretty old.

You might want to double check my scenario above but I guess we've been
lucky. So either some configuration changed and arm_dma_limit >
arm_lowmem_limit or the random VA for the CMA memory didn't overlap with
any other block.

An untested attempt to avoid the kmemleak notification for highmem
objects:

diff --git a/mm/memblock.c b/mm/memblock.c
index 483197ef613f..7d3361d53ac2 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -723,7 +723,8 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size)
 		     (unsigned long long)base + size - 1,
 		     (void *)_RET_IP_);
 
-	kmemleak_free_part(__va(base), size);
+	if (base < __pa(high_memory))
+		kmemleak_free_part(__va(base), size);
 	return memblock_remove_range(&memblock.reserved, base, size);
 }
 
@@ -1152,7 +1153,8 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
 		 * The min_count is set to 0 so that memblock allocations are
 		 * never reported as leaks.
 		 */
-		kmemleak_alloc(__va(found), size, 0, 0);
+		if (found < __pa(high_memory))
+			kmemleak_alloc(__va(found), size, 0, 0);
 		return found;
 	}
 	return 0;
@@ -1399,7 +1401,8 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size)
 	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
 		     __func__, (u64)base, (u64)base + size - 1,
 		     (void *)_RET_IP_);
-	kmemleak_free_part(__va(base), size);
+	if (base < __pa(high_memory))
+		kmemleak_free_part(__va(base), size);
 	memblock_remove_range(&memblock.reserved, base, size);
 }
 
@@ -1419,7 +1422,8 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size)
 	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
 		     __func__, (u64)base, (u64)base + size - 1,
 		     (void *)_RET_IP_);
-	kmemleak_free_part(__va(base), size);
+	if (base < __pa(high_memory))
+		kmemleak_free_part(__va(base), size);
 	cursor = PFN_UP(base);
 	end = PFN_DOWN(base + size);
 
-- 
Catalin

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
  2016-08-11 17:08     ` Catalin Marinas
@ 2016-08-12  4:15       ` Vignesh R
  2016-08-12 10:04         ` Catalin Marinas
  2016-08-12 13:50       ` Grygorii Strashko
  1 sibling, 1 reply; 8+ messages in thread
From: Vignesh R @ 2016-08-12  4:15 UTC (permalink / raw)
  To: Catalin Marinas, Strashko, Grygorii; +Cc: linux-mm, linux-kernel, linux-omap

Hi Catalin,

Thanks for the response!

On Thursday 11 August 2016 10:38 PM, Catalin Marinas wrote:
> On Thu, Aug 11, 2016 at 07:48:12PM +0300, Grygorii Strashko wrote:
>> On 08/11/2016 06:54 PM, Catalin Marinas wrote:
>>> On Thu, Aug 11, 2016 at 05:20:51PM +0530, Vignesh R wrote:
>>>> I see the below message from kmemleak when booting linux-next on AM335x
>>>> GP EVM and DRA7 EVM
>>>
>>> Can you also reproduce it with 4.8-rc1?

Yes, I can reproduce this on 4.8.0-rc1-g4b9eaf33d83d

[...]
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 483197ef613f..7d3361d53ac2 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -723,7 +723,8 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size)
>  		     (unsigned long long)base + size - 1,
>  		     (void *)_RET_IP_);
>  
> -	kmemleak_free_part(__va(base), size);
> +	if (base < __pa(high_memory))
> +		kmemleak_free_part(__va(base), size);
>  	return memblock_remove_range(&memblock.reserved, base, size);
>  }
>  
> @@ -1152,7 +1153,8 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
>  		 * The min_count is set to 0 so that memblock allocations are
>  		 * never reported as leaks.
>  		 */
> -		kmemleak_alloc(__va(found), size, 0, 0);
> +		if (found < __pa(high_memory))
> +			kmemleak_alloc(__va(found), size, 0, 0);
>  		return found;
>  	}
>  	return 0;
> @@ -1399,7 +1401,8 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size)
>  	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
>  		     __func__, (u64)base, (u64)base + size - 1,
>  		     (void *)_RET_IP_);
> -	kmemleak_free_part(__va(base), size);
> +	if (base < __pa(high_memory))
> +		kmemleak_free_part(__va(base), size);
>  	memblock_remove_range(&memblock.reserved, base, size);
>  }
>  
> @@ -1419,7 +1422,8 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size)
>  	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
>  		     __func__, (u64)base, (u64)base + size - 1,
>  		     (void *)_RET_IP_);
> -	kmemleak_free_part(__va(base), size);
> +	if (base < __pa(high_memory))
> +		kmemleak_free_part(__va(base), size);
>  	cursor = PFN_UP(base);
>  	end = PFN_DOWN(base + size);
>  
> 

With above change on 4.8-rc1, I see a different warning from kmemleak:

[    0.002918] kmemleak: Trying to color unknown object at 0xfe800000 as
Black
[    0.002943] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.8.0-rc1-00121-g4b9eaf33d83d-dirty #59
[    0.002955] Hardware name: Generic AM33XX (Flattened Device Tree)
[    0.003000] [<c01100fc>] (unwind_backtrace) from [<c010c264>]
(show_stack+0x10/0x14)
[    0.003027] [<c010c264>] (show_stack) from [<c049040c>]
(dump_stack+0xac/0xe0)
[    0.003052] [<c049040c>] (dump_stack) from [<c02971c0>]
(paint_ptr+0x78/0x9c)
[    0.003074] [<c02971c0>] (paint_ptr) from [<c0b25e20>]
(kmemleak_init+0x1cc/0x284)
[    0.003104] [<c0b25e20>] (kmemleak_init) from [<c0b00bc0>]
(start_kernel+0x2d8/0x3b4)
[    0.003122] [<c0b00bc0>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.003133] kmemleak: Early log backtrace:
[    0.003146]    [<c0b3c9cc>] dma_contiguous_reserve+0x80/0x94
[    0.003170]    [<c0b06810>] arm_memblock_init+0x130/0x184
[    0.003191]    [<c0b04210>] setup_arch+0x58c/0xc00
[    0.003208]    [<c0b00940>] start_kernel+0x58/0x3b4
[    0.003224]    [<8000807c>] 0x8000807c
[    0.003239]    [<ffffffff>] 0xffffffff

Full boot log: http://pastebin.ubuntu.com/23048180/

-- 
Thanks
Vignesh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
  2016-08-12  4:15       ` Vignesh R
@ 2016-08-12 10:04         ` Catalin Marinas
  2016-08-12 10:40           ` Vignesh R
  0 siblings, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2016-08-12 10:04 UTC (permalink / raw)
  To: Vignesh R; +Cc: Strashko, Grygorii, linux-mm, linux-kernel, linux-omap

On Fri, Aug 12, 2016 at 09:45:05AM +0530, Vignesh R wrote:
> On Thursday 11 August 2016 10:38 PM, Catalin Marinas wrote:
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index 483197ef613f..7d3361d53ac2 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -723,7 +723,8 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size)
> >  		     (unsigned long long)base + size - 1,
> >  		     (void *)_RET_IP_);
> >  
> > -	kmemleak_free_part(__va(base), size);
> > +	if (base < __pa(high_memory))
> > +		kmemleak_free_part(__va(base), size);
> >  	return memblock_remove_range(&memblock.reserved, base, size);
> >  }
> >  
> > @@ -1152,7 +1153,8 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
> >  		 * The min_count is set to 0 so that memblock allocations are
> >  		 * never reported as leaks.
> >  		 */
> > -		kmemleak_alloc(__va(found), size, 0, 0);
> > +		if (found < __pa(high_memory))
> > +			kmemleak_alloc(__va(found), size, 0, 0);
> >  		return found;
> >  	}
> >  	return 0;
> > @@ -1399,7 +1401,8 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size)
> >  	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
> >  		     __func__, (u64)base, (u64)base + size - 1,
> >  		     (void *)_RET_IP_);
> > -	kmemleak_free_part(__va(base), size);
> > +	if (base < __pa(high_memory))
> > +		kmemleak_free_part(__va(base), size);
> >  	memblock_remove_range(&memblock.reserved, base, size);
> >  }
> >  
> > @@ -1419,7 +1422,8 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size)
> >  	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
> >  		     __func__, (u64)base, (u64)base + size - 1,
> >  		     (void *)_RET_IP_);
> > -	kmemleak_free_part(__va(base), size);
> > +	if (base < __pa(high_memory))
> > +		kmemleak_free_part(__va(base), size);
> >  	cursor = PFN_UP(base);
> >  	end = PFN_DOWN(base + size);
> 
> With above change on 4.8-rc1, I see a different warning from kmemleak:
> 
> [    0.002918] kmemleak: Trying to color unknown object at 0xfe800000 as
> Black
> [    0.002943] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.8.0-rc1-00121-g4b9eaf33d83d-dirty #59
> [    0.002955] Hardware name: Generic AM33XX (Flattened Device Tree)
> [    0.003000] [<c01100fc>] (unwind_backtrace) from [<c010c264>] (show_stack+0x10/0x14)
> [    0.003027] [<c010c264>] (show_stack) from [<c049040c>] (dump_stack+0xac/0xe0)
> [    0.003052] [<c049040c>] (dump_stack) from [<c02971c0>] (paint_ptr+0x78/0x9c)
> [    0.003074] [<c02971c0>] (paint_ptr) from [<c0b25e20>] (kmemleak_init+0x1cc/0x284)
> [    0.003104] [<c0b25e20>] (kmemleak_init) from [<c0b00bc0>] (start_kernel+0x2d8/0x3b4)
> [    0.003122] [<c0b00bc0>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.003133] kmemleak: Early log backtrace:
> [    0.003146]    [<c0b3c9cc>] dma_contiguous_reserve+0x80/0x94
> [    0.003170]    [<c0b06810>] arm_memblock_init+0x130/0x184
> [    0.003191]    [<c0b04210>] setup_arch+0x58c/0xc00
> [    0.003208]    [<c0b00940>] start_kernel+0x58/0x3b4
> [    0.003224]    [<8000807c>] 0x8000807c
> [    0.003239]    [<ffffffff>] 0xffffffff

That's because I missed the CMA kmemleak call:

diff --git a/mm/cma.c b/mm/cma.c
index bd0e1412475e..7c0ef3037415 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -336,7 +336,8 @@ int __init cma_declare_contiguous(phys_addr_t base,
 		 * kmemleak scans/reads tracked objects for pointers to other
 		 * objects but this address isn't mapped and accessible
 		 */
-		kmemleak_ignore(phys_to_virt(addr));
+		if (addr < __pa(high_memory))
+			kmemleak_ignore(phys_to_virt(addr));
 		base = addr;
 	}
 

Anyway, a better workaround is to add kmemleak_*_phys() static inline
functions and do the __pa(high_memory) check in there:

-----------------8<---------------------------
>From b8b9141fffc1fd3c73583c1fd50a724c4a6452e1 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Fri, 12 Aug 2016 10:43:06 +0100
Subject: [PATCH] mm: kmemleak: Avoid using __va() on addresses that don't have
 a lowmem mapping

Some of the kmemleak_*() callbacks in memblock, bootmem, CMA convert a
physical address to a virtual one using __va(). However, such physical
addresses may sometimes be located in highmem and using __va() is
incorrect, leading to inconsistent object tracking in kmemleak.

The following functions have been added to the kmemleak API and they
take a physical address as the object pointer. They only perform the
corresponding action if the address has a lowmem mapping:

kmemleak_alloc_phys
kmemleak_free_part_phys
kmemleak_not_leak_phys
kmemleak_ignore_phys

The affected calling placess have been updated to use the new kmemleak
API.

Reported-by: Vignesh R <vigneshr@ti.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 Documentation/kmemleak.txt |  9 +++++++++
 include/linux/kmemleak.h   | 26 ++++++++++++++++++++++++++
 mm/bootmem.c               |  6 +++---
 mm/cma.c                   |  2 +-
 mm/memblock.c              |  8 ++++----
 mm/nobootmem.c             |  2 +-
 6 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/Documentation/kmemleak.txt b/Documentation/kmemleak.txt
index 18e24abb3ecf..35e1a8891e3a 100644
--- a/Documentation/kmemleak.txt
+++ b/Documentation/kmemleak.txt
@@ -155,6 +155,15 @@ kmemleak_erase		 - erase an old value in a pointer variable
 kmemleak_alloc_recursive - as kmemleak_alloc but checks the recursiveness
 kmemleak_free_recursive	 - as kmemleak_free but checks the recursiveness
 
+The following functions take a physical address as the object pointer
+and only perform the corresponding action if the address has a lowmem
+mapping:
+
+kmemleak_alloc_phys
+kmemleak_free_part_phys
+kmemleak_not_leak_phys
+kmemleak_ignore_phys
+
 Dealing with false positives/negatives
 --------------------------------------
 
diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
index 4894c6888bc6..380f72bc3657 100644
--- a/include/linux/kmemleak.h
+++ b/include/linux/kmemleak.h
@@ -21,6 +21,7 @@
 #ifndef __KMEMLEAK_H
 #define __KMEMLEAK_H
 
+#include <linux/mm.h>
 #include <linux/slab.h>
 
 #ifdef CONFIG_DEBUG_KMEMLEAK
@@ -109,4 +110,29 @@ static inline void kmemleak_no_scan(const void *ptr)
 
 #endif	/* CONFIG_DEBUG_KMEMLEAK */
 
+static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
+				       int min_count, gfp_t gfp)
+{
+	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
+		kmemleak_alloc(__va(phys), size, min_count, gfp);
+}
+
+static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size)
+{
+	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
+		kmemleak_free_part(__va(phys), size);
+}
+
+static inline void kmemleak_not_leak_phys(phys_addr_t phys)
+{
+	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
+		kmemleak_not_leak(__va(phys));
+}
+
+static inline void kmemleak_ignore_phys(phys_addr_t phys)
+{
+	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
+		kmemleak_ignore(__va(phys));
+}
+
 #endif	/* __KMEMLEAK_H */
diff --git a/mm/bootmem.c b/mm/bootmem.c
index 0aa7dda52402..80f1d70bad2d 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -158,7 +158,7 @@ void __init free_bootmem_late(unsigned long physaddr, unsigned long size)
 {
 	unsigned long cursor, end;
 
-	kmemleak_free_part(__va(physaddr), size);
+	kmemleak_free_part_phys(physaddr, size);
 
 	cursor = PFN_UP(physaddr);
 	end = PFN_DOWN(physaddr + size);
@@ -402,7 +402,7 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
 {
 	unsigned long start, end;
 
-	kmemleak_free_part(__va(physaddr), size);
+	kmemleak_free_part_phys(physaddr, size);
 
 	start = PFN_UP(physaddr);
 	end = PFN_DOWN(physaddr + size);
@@ -423,7 +423,7 @@ void __init free_bootmem(unsigned long physaddr, unsigned long size)
 {
 	unsigned long start, end;
 
-	kmemleak_free_part(__va(physaddr), size);
+	kmemleak_free_part_phys(physaddr, size);
 
 	start = PFN_UP(physaddr);
 	end = PFN_DOWN(physaddr + size);
diff --git a/mm/cma.c b/mm/cma.c
index bd0e1412475e..384c2cb51b56 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -336,7 +336,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
 		 * kmemleak scans/reads tracked objects for pointers to other
 		 * objects but this address isn't mapped and accessible
 		 */
-		kmemleak_ignore(phys_to_virt(addr));
+		kmemleak_ignore_phys(addr);
 		base = addr;
 	}
 
diff --git a/mm/memblock.c b/mm/memblock.c
index 483197ef613f..30ecea7b45d1 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -723,7 +723,7 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size)
 		     (unsigned long long)base + size - 1,
 		     (void *)_RET_IP_);
 
-	kmemleak_free_part(__va(base), size);
+	kmemleak_free_part_phys(base, size);
 	return memblock_remove_range(&memblock.reserved, base, size);
 }
 
@@ -1152,7 +1152,7 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
 		 * The min_count is set to 0 so that memblock allocations are
 		 * never reported as leaks.
 		 */
-		kmemleak_alloc(__va(found), size, 0, 0);
+		kmemleak_alloc_phys(found, size, 0, 0);
 		return found;
 	}
 	return 0;
@@ -1399,7 +1399,7 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size)
 	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
 		     __func__, (u64)base, (u64)base + size - 1,
 		     (void *)_RET_IP_);
-	kmemleak_free_part(__va(base), size);
+	kmemleak_free_part_phys(base, size);
 	memblock_remove_range(&memblock.reserved, base, size);
 }
 
@@ -1419,7 +1419,7 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size)
 	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
 		     __func__, (u64)base, (u64)base + size - 1,
 		     (void *)_RET_IP_);
-	kmemleak_free_part(__va(base), size);
+	kmemleak_free_part_phys(base, size);
 	cursor = PFN_UP(base);
 	end = PFN_DOWN(base + size);
 
diff --git a/mm/nobootmem.c b/mm/nobootmem.c
index bd05a70f44b9..a056d31dff7e 100644
--- a/mm/nobootmem.c
+++ b/mm/nobootmem.c
@@ -81,7 +81,7 @@ void __init free_bootmem_late(unsigned long addr, unsigned long size)
 {
 	unsigned long cursor, end;
 
-	kmemleak_free_part(__va(addr), size);
+	kmemleak_free_part_phys(addr, size);
 
 	cursor = PFN_UP(addr);
 	end = PFN_DOWN(addr + size);

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
  2016-08-12 10:04         ` Catalin Marinas
@ 2016-08-12 10:40           ` Vignesh R
  0 siblings, 0 replies; 8+ messages in thread
From: Vignesh R @ 2016-08-12 10:40 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Strashko, Grygorii, linux-mm, linux-kernel, linux-omap



On Friday 12 August 2016 03:34 PM, Catalin Marinas wrote:
> On Fri, Aug 12, 2016 at 09:45:05AM +0530, Vignesh R wrote:
>> On Thursday 11 August 2016 10:38 PM, Catalin Marinas wrote:
[...]
> From b8b9141fffc1fd3c73583c1fd50a724c4a6452e1 Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Fri, 12 Aug 2016 10:43:06 +0100
> Subject: [PATCH] mm: kmemleak: Avoid using __va() on addresses that don't have
>  a lowmem mapping
> 
> Some of the kmemleak_*() callbacks in memblock, bootmem, CMA convert a
> physical address to a virtual one using __va(). However, such physical
> addresses may sometimes be located in highmem and using __va() is
> incorrect, leading to inconsistent object tracking in kmemleak.
> 
> The following functions have been added to the kmemleak API and they
> take a physical address as the object pointer. They only perform the
> corresponding action if the address has a lowmem mapping:
> 
> kmemleak_alloc_phys
> kmemleak_free_part_phys
> kmemleak_not_leak_phys
> kmemleak_ignore_phys
> 
> The affected calling placess have been updated to use the new kmemleak
> API.
> 
> Reported-by: Vignesh R <vigneshr@ti.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks, this solves the issue!
I tested on AM335x GP EVM and DRA72 EVM on linux-next with below patch
applied. FWIW,
Tested-by: Vignesh R <vigneshr@ti.com>

I can see this issue on stable v4.4.17 kernel as well. So, can this
patch be marked for stable backport?


Regards
Vignesh

> ---
>  Documentation/kmemleak.txt |  9 +++++++++
>  include/linux/kmemleak.h   | 26 ++++++++++++++++++++++++++
>  mm/bootmem.c               |  6 +++---
>  mm/cma.c                   |  2 +-
>  mm/memblock.c              |  8 ++++----
>  mm/nobootmem.c             |  2 +-
>  6 files changed, 44 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/kmemleak.txt b/Documentation/kmemleak.txt
> index 18e24abb3ecf..35e1a8891e3a 100644
> --- a/Documentation/kmemleak.txt
> +++ b/Documentation/kmemleak.txt
> @@ -155,6 +155,15 @@ kmemleak_erase		 - erase an old value in a pointer variable
>  kmemleak_alloc_recursive - as kmemleak_alloc but checks the recursiveness
>  kmemleak_free_recursive	 - as kmemleak_free but checks the recursiveness
>  
> +The following functions take a physical address as the object pointer
> +and only perform the corresponding action if the address has a lowmem
> +mapping:
> +
> +kmemleak_alloc_phys
> +kmemleak_free_part_phys
> +kmemleak_not_leak_phys
> +kmemleak_ignore_phys
> +
>  Dealing with false positives/negatives
>  --------------------------------------
>  
> diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
> index 4894c6888bc6..380f72bc3657 100644
> --- a/include/linux/kmemleak.h
> +++ b/include/linux/kmemleak.h
> @@ -21,6 +21,7 @@
>  #ifndef __KMEMLEAK_H
>  #define __KMEMLEAK_H
>  
> +#include <linux/mm.h>
>  #include <linux/slab.h>
>  
>  #ifdef CONFIG_DEBUG_KMEMLEAK
> @@ -109,4 +110,29 @@ static inline void kmemleak_no_scan(const void *ptr)
>  
>  #endif	/* CONFIG_DEBUG_KMEMLEAK */
>  
> +static inline void kmemleak_alloc_phys(phys_addr_t phys, size_t size,
> +				       int min_count, gfp_t gfp)
> +{
> +	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
> +		kmemleak_alloc(__va(phys), size, min_count, gfp);
> +}
> +
> +static inline void kmemleak_free_part_phys(phys_addr_t phys, size_t size)
> +{
> +	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
> +		kmemleak_free_part(__va(phys), size);
> +}
> +
> +static inline void kmemleak_not_leak_phys(phys_addr_t phys)
> +{
> +	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
> +		kmemleak_not_leak(__va(phys));
> +}
> +
> +static inline void kmemleak_ignore_phys(phys_addr_t phys)
> +{
> +	if (!IS_ENABLED(CONFIG_HIGHMEM) || phys < __pa(high_memory))
> +		kmemleak_ignore(__va(phys));
> +}
> +
>  #endif	/* __KMEMLEAK_H */
> diff --git a/mm/bootmem.c b/mm/bootmem.c
> index 0aa7dda52402..80f1d70bad2d 100644
> --- a/mm/bootmem.c
> +++ b/mm/bootmem.c
> @@ -158,7 +158,7 @@ void __init free_bootmem_late(unsigned long physaddr, unsigned long size)
>  {
>  	unsigned long cursor, end;
>  
> -	kmemleak_free_part(__va(physaddr), size);
> +	kmemleak_free_part_phys(physaddr, size);
>  
>  	cursor = PFN_UP(physaddr);
>  	end = PFN_DOWN(physaddr + size);
> @@ -402,7 +402,7 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
>  {
>  	unsigned long start, end;
>  
> -	kmemleak_free_part(__va(physaddr), size);
> +	kmemleak_free_part_phys(physaddr, size);
>  
>  	start = PFN_UP(physaddr);
>  	end = PFN_DOWN(physaddr + size);
> @@ -423,7 +423,7 @@ void __init free_bootmem(unsigned long physaddr, unsigned long size)
>  {
>  	unsigned long start, end;
>  
> -	kmemleak_free_part(__va(physaddr), size);
> +	kmemleak_free_part_phys(physaddr, size);
>  
>  	start = PFN_UP(physaddr);
>  	end = PFN_DOWN(physaddr + size);
> diff --git a/mm/cma.c b/mm/cma.c
> index bd0e1412475e..384c2cb51b56 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -336,7 +336,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
>  		 * kmemleak scans/reads tracked objects for pointers to other
>  		 * objects but this address isn't mapped and accessible
>  		 */
> -		kmemleak_ignore(phys_to_virt(addr));
> +		kmemleak_ignore_phys(addr);
>  		base = addr;
>  	}
>  
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 483197ef613f..30ecea7b45d1 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -723,7 +723,7 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size)
>  		     (unsigned long long)base + size - 1,
>  		     (void *)_RET_IP_);
>  
> -	kmemleak_free_part(__va(base), size);
> +	kmemleak_free_part_phys(base, size);
>  	return memblock_remove_range(&memblock.reserved, base, size);
>  }
>  
> @@ -1152,7 +1152,7 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
>  		 * The min_count is set to 0 so that memblock allocations are
>  		 * never reported as leaks.
>  		 */
> -		kmemleak_alloc(__va(found), size, 0, 0);
> +		kmemleak_alloc_phys(found, size, 0, 0);
>  		return found;
>  	}
>  	return 0;
> @@ -1399,7 +1399,7 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size)
>  	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
>  		     __func__, (u64)base, (u64)base + size - 1,
>  		     (void *)_RET_IP_);
> -	kmemleak_free_part(__va(base), size);
> +	kmemleak_free_part_phys(base, size);
>  	memblock_remove_range(&memblock.reserved, base, size);
>  }
>  
> @@ -1419,7 +1419,7 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size)
>  	memblock_dbg("%s: [%#016llx-%#016llx] %pF\n",
>  		     __func__, (u64)base, (u64)base + size - 1,
>  		     (void *)_RET_IP_);
> -	kmemleak_free_part(__va(base), size);
> +	kmemleak_free_part_phys(base, size);
>  	cursor = PFN_UP(base);
>  	end = PFN_DOWN(base + size);
>  
> diff --git a/mm/nobootmem.c b/mm/nobootmem.c
> index bd05a70f44b9..a056d31dff7e 100644
> --- a/mm/nobootmem.c
> +++ b/mm/nobootmem.c
> @@ -81,7 +81,7 @@ void __init free_bootmem_late(unsigned long addr, unsigned long size)
>  {
>  	unsigned long cursor, end;
>  
> -	kmemleak_free_part(__va(addr), size);
> +	kmemleak_free_part_phys(addr, size);
>  
>  	cursor = PFN_UP(addr);
>  	end = PFN_DOWN(addr + size);
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
  2016-08-11 17:08     ` Catalin Marinas
  2016-08-12  4:15       ` Vignesh R
@ 2016-08-12 13:50       ` Grygorii Strashko
  1 sibling, 0 replies; 8+ messages in thread
From: Grygorii Strashko @ 2016-08-12 13:50 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Vignesh R, linux-mm, linux-kernel, linux-omap

On 08/11/2016 08:08 PM, Catalin Marinas wrote:
> On Thu, Aug 11, 2016 at 07:48:12PM +0300, Grygorii Strashko wrote:
>> On 08/11/2016 06:54 PM, Catalin Marinas wrote:
>>> On Thu, Aug 11, 2016 at 05:20:51PM +0530, Vignesh R wrote:
>>>> I see the below message from kmemleak when booting linux-next on AM335x
>>>> GP EVM and DRA7 EVM
>>>
>>> Can you also reproduce it with 4.8-rc1?
>>>
>>>> [    0.803934] kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing)
>>>> [    0.803950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1-next-20160809 #497
>>>> [    0.803958] Hardware name: Generic DRA72X (Flattened Device Tree)
>>>> [    0.803979] [<c0110104>] (unwind_backtrace) from [<c010c24c>] (show_stack+0x10/0x14)
>>>> [    0.803994] [<c010c24c>] (show_stack) from [<c0490df0>] (dump_stack+0xac/0xe0)
>>>> [    0.804010] [<c0490df0>] (dump_stack) from [<c0296f88>] (create_object+0x214/0x278)
>>>> [    0.804025] [<c0296f88>] (create_object) from [<c07c770c>] (kmemleak_alloc_percpu+0x54/0xc0)
>>>> [    0.804038] [<c07c770c>] (kmemleak_alloc_percpu) from [<c025fb08>] (pcpu_alloc+0x368/0x5fc)
>>>> [    0.804052] [<c025fb08>] (pcpu_alloc) from [<c0b1bfbc>] (crash_notes_memory_init+0x10/0x40)
>>>> [    0.804064] [<c0b1bfbc>] (crash_notes_memory_init) from [<c010188c>] (do_one_initcall+0x3c/0x178)
>>>> [    0.804075] [<c010188c>] (do_one_initcall) from [<c0b00e98>] (kernel_init_freeable+0x1fc/0x2c8)
>>>> [    0.804086] [<c0b00e98>] (kernel_init_freeable) from [<c07c66b0>] (kernel_init+0x8/0x114)
>>>> [    0.804098] [<c07c66b0>] (kernel_init) from [<c0107910>] (ret_from_fork+0x14/0x24)
>>>
>>> This is the allocation stack trace, going via pcpu_alloc().
>>>
>>>> [    0.804106] kmemleak: Kernel memory leak detector disabled
>>>> [    0.804113] kmemleak: Object 0xfe800000 (size 16777216):
>>>> [    0.804121] kmemleak:   comm "swapper/0", pid 0, jiffies 4294937296
>>>> [    0.804127] kmemleak:   min_count = -1
>>>> [    0.804132] kmemleak:   count = 0
>>>> [    0.804138] kmemleak:   flags = 0x5
>>>> [    0.804143] kmemleak:   checksum = 0
>>>> [    0.804149] kmemleak:   backtrace:
>>>> [    0.804155]      [<c0b26a90>] cma_declare_contiguous+0x16c/0x214
>>>> [    0.804170]      [<c0b3c9c0>] dma_contiguous_reserve_area+0x30/0x64
>>>> [    0.804183]      [<c0b3ca74>] dma_contiguous_reserve+0x80/0x94
>>>> [    0.804195]      [<c0b06810>] arm_memblock_init+0x130/0x184
>>>> [    0.804207]      [<c0b04214>] setup_arch+0x590/0xc08
>>>> [    0.804217]      [<c0b00940>] start_kernel+0x58/0x3b4
>>>> [    0.804227]      [<8000807c>] 0x8000807c
>>>> [    0.804237]      [<ffffffff>] 0xffffffff
>>>
>>> This seems to be the original object that was allocated via
>>> cma_declare_contiguous(): 16MB range from 0xfe800000 to 0xff800000.
>>> Since the pointer returned by pcpu_alloc is 0xff7f1000 falls in the 16MB
>>> CMA range, kmemleak gets confused (it doesn't allow overlapping
>>> objects).
>>>
>>> So what I think goes wrong is that the kmemleak_alloc(__va(found)) call
>>> in memblock_alloc_range_nid() doesn't get the right value for the VA of
>>> the CMA block. The memblock_alloc_range() call in
>>> cma_declare_contiguous() asks for memory above high_memory, hence on a
>>> 32-bit architecture with highmem enabled, __va() use is not really
>>> valid, returning the wrong address. The existing kmemleak object is
>>> bogus, it shouldn't have been created in the first place.
>>>
>>> Now I'm trying to figure out how to differentiate between lowmem
>>> memblocks and highmem ones. Ignoring the kmemleak_alloc() calls
>>> altogether in mm/memblock.c is probably not an option as it would lead
>>> to lots of false positives.
>>
>> But cma_declare_contiguous() calls -
>> 		/*
>> 		 * kmemleak scans/reads tracked objects for pointers to other
>> 		 * objects but this address isn't mapped and accessible
>> 		 */
>> 		kmemleak_ignore(phys_to_virt(addr));
>>
>> Does it means above code is incorrect also?
>
> Yes, as long as the phys_to_virt() use is invalid. You may get away with
> this, depending on the SoC. Also, kmemleak_ignore() here is meant to
> tell kmemleak not to bother with scanning or reporting such memory since
> it is not meant for pointers but it still keeps track of it. The only
> way to remove it from kmemleak is replace this with kmemleak_free(). But
> That's more of a hack since phys_to_virt(addr) is still invalid.
>
>> It's a little bit strange that this can be seen only now, because
>> commit 95b0e655f9 ("ARM: mm: don't limit default CMA region only to low memor")
>> is pretty old.
>
> You might want to double check my scenario above but I guess we've been
> lucky. So either some configuration changed and arm_dma_limit >
> arm_lowmem_limit or the random VA for the CMA memory didn't overlap with
> any other block.
>

Thanks a  lot for explanation.


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-08-12 13:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-11 11:50 kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing) Vignesh R
2016-08-11 15:54 ` Catalin Marinas
2016-08-11 16:48   ` Grygorii Strashko
2016-08-11 17:08     ` Catalin Marinas
2016-08-12  4:15       ` Vignesh R
2016-08-12 10:04         ` Catalin Marinas
2016-08-12 10:40           ` Vignesh R
2016-08-12 13:50       ` Grygorii Strashko

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).