linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
@ 2021-11-18  5:44 Kuan-Ying Lee
  2021-11-18  9:20 ` Kuan-Ying Lee
  2021-12-02 18:19 ` Catalin Marinas
  0 siblings, 2 replies; 9+ messages in thread
From: Kuan-Ying Lee @ 2021-11-18  5:44 UTC (permalink / raw)
  To: Catalin Marinas, Andrew Morton, Matthias Brugger
  Cc: chinwen.chang, nicholas.tang, james.hsu, yee.lee, Kuan-Ying Lee,
	linux-mm, linux-kernel, linux-arm-kernel, linux-mediatek

With HW tag-based kasan enable, We will get the warning
when we free object whose address starts with 0xFF.

It is because kmemleak rbtree stores tagged object and
this freeing object's tag does not match with rbtree object.

In the example below, kmemleak rbtree stores the tagged object in
the kmalloc(), and kfree() gets the pointer with 0xFF tag.

Call sequence:
ptr = kmalloc(size, GFP_KERNEL);
page = virt_to_page(ptr);
kfree(page_address(page));
ptr = kmalloc(size, GFP_KERNEL);

Call sequence like that may cause the warning as following:
1) Freeing unknown object:
In kfree(), we will get free unknown object warning in kmemleak_free().
Because object(0xFx) in kmemleak rbtree and pointer(0xFF) in kfree() have
different tag.

2) Overlap existing:
When we allocate that object with the same hw-tag again, we will
find the overlap in the kmemleak rbtree and kmemleak thread will
be killed.

[  116.685312] kmemleak: Freeing unknown object at 0xffff000003f88000
[  116.686422] CPU: 5 PID: 177 Comm: cat Not tainted 5.16.0-rc1-dirty #21
[  116.687067] Hardware name: linux,dummy-virt (DT)
[  116.687496] Call trace:
[  116.687792]  dump_backtrace+0x0/0x1ac
[  116.688255]  show_stack+0x1c/0x30
[  116.688663]  dump_stack_lvl+0x68/0x84
[  116.689096]  dump_stack+0x1c/0x38
[  116.689499]  kmemleak_free+0x6c/0x70
[  116.689919]  slab_free_freelist_hook+0x104/0x200
[  116.690420]  kmem_cache_free+0xa8/0x3d4
[  116.690845]  test_version_show+0x270/0x3a0
[  116.691344]  module_attr_show+0x28/0x40
[  116.691789]  sysfs_kf_seq_show+0xb0/0x130
[  116.692245]  kernfs_seq_show+0x30/0x40
[  116.692678]  seq_read_iter+0x1bc/0x4b0
[  116.692678]  seq_read_iter+0x1bc/0x4b0
[  116.693114]  kernfs_fop_read_iter+0x144/0x1c0
[  116.693586]  generic_file_splice_read+0xd0/0x184
[  116.694078]  do_splice_to+0x90/0xe0
[  116.694498]  splice_direct_to_actor+0xb8/0x250
[  116.694975]  do_splice_direct+0x88/0xd4
[  116.695409]  do_sendfile+0x2b0/0x344
[  116.695829]  __arm64_sys_sendfile64+0x164/0x16c
[  116.696306]  invoke_syscall+0x48/0x114
[  116.696735]  el0_svc_common.constprop.0+0x44/0xec
[  116.697263]  do_el0_svc+0x74/0x90
[  116.697665]  el0_svc+0x20/0x80
[  116.698261]  el0t_64_sync_handler+0x1a8/0x1b0
[  116.698695]  el0t_64_sync+0x1ac/0x1b0
...
[  117.520301] kmemleak: Cannot insert 0xf2ff000003f88000 into the object search tree (overlaps existing)
[  117.521118] CPU: 5 PID: 178 Comm: cat Not tainted 5.16.0-rc1-dirty #21
[  117.521827] Hardware name: linux,dummy-virt (DT)
[  117.522287] Call trace:
[  117.522586]  dump_backtrace+0x0/0x1ac
[  117.523053]  show_stack+0x1c/0x30
[  117.523578]  dump_stack_lvl+0x68/0x84
[  117.524039]  dump_stack+0x1c/0x38
[  117.524472]  create_object.isra.0+0x2d8/0x2fc
[  117.524975]  kmemleak_alloc+0x34/0x40
[  117.525416]  kmem_cache_alloc+0x23c/0x2f0
[  117.525914]  test_version_show+0x1fc/0x3a0
[  117.526379]  module_attr_show+0x28/0x40
[  117.526827]  sysfs_kf_seq_show+0xb0/0x130
[  117.527363]  kernfs_seq_show+0x30/0x40
[  117.527848]  seq_read_iter+0x1bc/0x4b0
[  117.528320]  kernfs_fop_read_iter+0x144/0x1c0
[  117.528809]  generic_file_splice_read+0xd0/0x184
[  117.529316]  do_splice_to+0x90/0xe0
[  117.529734]  splice_direct_to_actor+0xb8/0x250
[  117.530227]  do_splice_direct+0x88/0xd4
[  117.530686]  do_sendfile+0x2b0/0x344
[  117.531154]  __arm64_sys_sendfile64+0x164/0x16c
[  117.531673]  invoke_syscall+0x48/0x114
[  117.532111]  el0_svc_common.constprop.0+0x44/0xec
[  117.532621]  do_el0_svc+0x74/0x90
[  117.533048]  el0_svc+0x20/0x80
[  117.533461]  el0t_64_sync_handler+0x1a8/0x1b0
[  117.533950]  el0t_64_sync+0x1ac/0x1b0
[  117.534625] kmemleak: Kernel memory leak detector disabled
[  117.535201] kmemleak: Object 0xf2ff000003f88000 (size 128):
[  117.535761] kmemleak:   comm "cat", pid 177, jiffies 4294921177
[  117.536339] kmemleak:   min_count = 1
[  117.536718] kmemleak:   count = 0
[  117.537068] kmemleak:   flags = 0x1
[  117.537429] kmemleak:   checksum = 0
[  117.537806] kmemleak:   backtrace:
[  117.538211]      kmem_cache_alloc+0x23c/0x2f0
[  117.538924]      test_version_show+0x1fc/0x3a0
[  117.539393]      module_attr_show+0x28/0x40
[  117.539844]      sysfs_kf_seq_show+0xb0/0x130
[  117.540304]      kernfs_seq_show+0x30/0x40
[  117.540750]      seq_read_iter+0x1bc/0x4b0
[  117.541206]      kernfs_fop_read_iter+0x144/0x1c0
[  117.541687]      generic_file_splice_read+0xd0/0x184
[  117.542182]      do_splice_to+0x90/0xe0
[  117.542611]      splice_direct_to_actor+0xb8/0x250
[  117.543097]      do_splice_direct+0x88/0xd4
[  117.543544]      do_sendfile+0x2b0/0x344
[  117.543983]      __arm64_sys_sendfile64+0x164/0x16c
[  117.544471]      invoke_syscall+0x48/0x114
[  117.544917]      el0_svc_common.constprop.0+0x44/0xec
[  117.545416]      do_el0_svc+0x74/0x90
[  117.554100] kmemleak: Automatic memory scanning thread ended

Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com>
---
 mm/kmemleak.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index b57383c17cf6..fa12e2e08cdc 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -381,15 +381,20 @@ static void dump_object_info(struct kmemleak_object *object)
 static struct kmemleak_object *lookup_object(unsigned long ptr, int alias)
 {
 	struct rb_node *rb = object_tree_root.rb_node;
+	unsigned long untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
 
 	while (rb) {
 		struct kmemleak_object *object =
 			rb_entry(rb, struct kmemleak_object, rb_node);
-		if (ptr < object->pointer)
+		unsigned long untagged_objp;
+
+		untagged_objp = (unsigned long)kasan_reset_tag((void *)object->pointer);
+
+		if (untagged_ptr < untagged_objp)
 			rb = object->rb_node.rb_left;
-		else if (object->pointer + object->size <= ptr)
+		else if (untagged_objp + object->size <= untagged_ptr)
 			rb = object->rb_node.rb_right;
-		else if (object->pointer == ptr || alias)
+		else if (untagged_objp == untagged_ptr || alias)
 			return object;
 		else {
 			kmemleak_warn("Found object by alias at 0x%08lx\n",
@@ -576,6 +581,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
 	struct kmemleak_object *object, *parent;
 	struct rb_node **link, *rb_parent;
 	unsigned long untagged_ptr;
+	unsigned long untagged_objp;
 
 	object = mem_pool_alloc(gfp);
 	if (!object) {
@@ -629,9 +635,10 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
 	while (*link) {
 		rb_parent = *link;
 		parent = rb_entry(rb_parent, struct kmemleak_object, rb_node);
-		if (ptr + size <= parent->pointer)
+		untagged_objp = (unsigned long)kasan_reset_tag((void *)parent->pointer);
+		if (untagged_ptr + size <= untagged_objp)
 			link = &parent->rb_node.rb_left;
-		else if (parent->pointer + parent->size <= ptr)
+		else if (untagged_objp + parent->size <= untagged_ptr)
 			link = &parent->rb_node.rb_right;
 		else {
 			kmemleak_stop("Cannot insert 0x%lx into the object search tree (overlaps existing)\n",
-- 
2.18.0


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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-18  5:44 [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable Kuan-Ying Lee
@ 2021-11-18  9:20 ` Kuan-Ying Lee
  2021-11-19 14:15   ` Andrey Konovalov
  2021-11-25 16:13   ` Andrey Konovalov
  2021-12-02 18:19 ` Catalin Marinas
  1 sibling, 2 replies; 9+ messages in thread
From: Kuan-Ying Lee @ 2021-11-18  9:20 UTC (permalink / raw)
  To: Catalin Marinas, Andrew Morton, Matthias Brugger
  Cc: kuan-ying.lee, Chinwen Chang (張錦文),
	Nicholas Tang (鄭秦輝),
	James Hsu (徐慶薰),
	Yee Lee (李建誼),
	linux-mm, linux-kernel, linux-arm-kernel, linux-mediatek,
	kasan-dev

+Cc kasan group

On Thu, 2021-11-18 at 13:44 +0800, Kuan-Ying Lee wrote:
> With HW tag-based kasan enable, We will get the warning
> when we free object whose address starts with 0xFF.
> 
> It is because kmemleak rbtree stores tagged object and
> this freeing object's tag does not match with rbtree object.
> 
> In the example below, kmemleak rbtree stores the tagged object in
> the kmalloc(), and kfree() gets the pointer with 0xFF tag.
> 
> Call sequence:
> ptr = kmalloc(size, GFP_KERNEL);
> page = virt_to_page(ptr);
> kfree(page_address(page));
> ptr = kmalloc(size, GFP_KERNEL);
> 
> Call sequence like that may cause the warning as following:
> 1) Freeing unknown object:
> In kfree(), we will get free unknown object warning in
> kmemleak_free().
> Because object(0xFx) in kmemleak rbtree and pointer(0xFF) in kfree()
> have
> different tag.
> 
> 2) Overlap existing:
> When we allocate that object with the same hw-tag again, we will
> find the overlap in the kmemleak rbtree and kmemleak thread will
> be killed.
> 
> [  116.685312] kmemleak: Freeing unknown object at 0xffff000003f88000
> [  116.686422] CPU: 5 PID: 177 Comm: cat Not tainted 5.16.0-rc1-dirty 
> #21
> [  116.687067] Hardware name: linux,dummy-virt (DT)
> [  116.687496] Call trace:
> [  116.687792]  dump_backtrace+0x0/0x1ac
> [  116.688255]  show_stack+0x1c/0x30
> [  116.688663]  dump_stack_lvl+0x68/0x84
> [  116.689096]  dump_stack+0x1c/0x38
> [  116.689499]  kmemleak_free+0x6c/0x70
> [  116.689919]  slab_free_freelist_hook+0x104/0x200
> [  116.690420]  kmem_cache_free+0xa8/0x3d4
> [  116.690845]  test_version_show+0x270/0x3a0
> [  116.691344]  module_attr_show+0x28/0x40
> [  116.691789]  sysfs_kf_seq_show+0xb0/0x130
> [  116.692245]  kernfs_seq_show+0x30/0x40
> [  116.692678]  seq_read_iter+0x1bc/0x4b0
> [  116.692678]  seq_read_iter+0x1bc/0x4b0
> [  116.693114]  kernfs_fop_read_iter+0x144/0x1c0
> [  116.693586]  generic_file_splice_read+0xd0/0x184
> [  116.694078]  do_splice_to+0x90/0xe0
> [  116.694498]  splice_direct_to_actor+0xb8/0x250
> [  116.694975]  do_splice_direct+0x88/0xd4
> [  116.695409]  do_sendfile+0x2b0/0x344
> [  116.695829]  __arm64_sys_sendfile64+0x164/0x16c
> [  116.696306]  invoke_syscall+0x48/0x114
> [  116.696735]  el0_svc_common.constprop.0+0x44/0xec
> [  116.697263]  do_el0_svc+0x74/0x90
> [  116.697665]  el0_svc+0x20/0x80
> [  116.698261]  el0t_64_sync_handler+0x1a8/0x1b0
> [  116.698695]  el0t_64_sync+0x1ac/0x1b0
> ...
> [  117.520301] kmemleak: Cannot insert 0xf2ff000003f88000 into the
> object search tree (overlaps existing)
> [  117.521118] CPU: 5 PID: 178 Comm: cat Not tainted 5.16.0-rc1-dirty 
> #21
> [  117.521827] Hardware name: linux,dummy-virt (DT)
> [  117.522287] Call trace:
> [  117.522586]  dump_backtrace+0x0/0x1ac
> [  117.523053]  show_stack+0x1c/0x30
> [  117.523578]  dump_stack_lvl+0x68/0x84
> [  117.524039]  dump_stack+0x1c/0x38
> [  117.524472]  create_object.isra.0+0x2d8/0x2fc
> [  117.524975]  kmemleak_alloc+0x34/0x40
> [  117.525416]  kmem_cache_alloc+0x23c/0x2f0
> [  117.525914]  test_version_show+0x1fc/0x3a0
> [  117.526379]  module_attr_show+0x28/0x40
> [  117.526827]  sysfs_kf_seq_show+0xb0/0x130
> [  117.527363]  kernfs_seq_show+0x30/0x40
> [  117.527848]  seq_read_iter+0x1bc/0x4b0
> [  117.528320]  kernfs_fop_read_iter+0x144/0x1c0
> [  117.528809]  generic_file_splice_read+0xd0/0x184
> [  117.529316]  do_splice_to+0x90/0xe0
> [  117.529734]  splice_direct_to_actor+0xb8/0x250
> [  117.530227]  do_splice_direct+0x88/0xd4
> [  117.530686]  do_sendfile+0x2b0/0x344
> [  117.531154]  __arm64_sys_sendfile64+0x164/0x16c
> [  117.531673]  invoke_syscall+0x48/0x114
> [  117.532111]  el0_svc_common.constprop.0+0x44/0xec
> [  117.532621]  do_el0_svc+0x74/0x90
> [  117.533048]  el0_svc+0x20/0x80
> [  117.533461]  el0t_64_sync_handler+0x1a8/0x1b0
> [  117.533950]  el0t_64_sync+0x1ac/0x1b0
> [  117.534625] kmemleak: Kernel memory leak detector disabled
> [  117.535201] kmemleak: Object 0xf2ff000003f88000 (size 128):
> [  117.535761] kmemleak:   comm "cat", pid 177, jiffies 4294921177
> [  117.536339] kmemleak:   min_count = 1
> [  117.536718] kmemleak:   count = 0
> [  117.537068] kmemleak:   flags = 0x1
> [  117.537429] kmemleak:   checksum = 0
> [  117.537806] kmemleak:   backtrace:
> [  117.538211]      kmem_cache_alloc+0x23c/0x2f0
> [  117.538924]      test_version_show+0x1fc/0x3a0
> [  117.539393]      module_attr_show+0x28/0x40
> [  117.539844]      sysfs_kf_seq_show+0xb0/0x130
> [  117.540304]      kernfs_seq_show+0x30/0x40
> [  117.540750]      seq_read_iter+0x1bc/0x4b0
> [  117.541206]      kernfs_fop_read_iter+0x144/0x1c0
> [  117.541687]      generic_file_splice_read+0xd0/0x184
> [  117.542182]      do_splice_to+0x90/0xe0
> [  117.542611]      splice_direct_to_actor+0xb8/0x250
> [  117.543097]      do_splice_direct+0x88/0xd4
> [  117.543544]      do_sendfile+0x2b0/0x344
> [  117.543983]      __arm64_sys_sendfile64+0x164/0x16c
> [  117.544471]      invoke_syscall+0x48/0x114
> [  117.544917]      el0_svc_common.constprop.0+0x44/0xec
> [  117.545416]      do_el0_svc+0x74/0x90
> [  117.554100] kmemleak: Automatic memory scanning thread ended
> 
> Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com>
> ---
>  mm/kmemleak.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index b57383c17cf6..fa12e2e08cdc 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -381,15 +381,20 @@ static void dump_object_info(struct
> kmemleak_object *object)
>  static struct kmemleak_object *lookup_object(unsigned long ptr, int
> alias)
>  {
>  	struct rb_node *rb = object_tree_root.rb_node;
> +	unsigned long untagged_ptr = (unsigned
> long)kasan_reset_tag((void *)ptr);
>  
>  	while (rb) {
>  		struct kmemleak_object *object =
>  			rb_entry(rb, struct kmemleak_object, rb_node);
> -		if (ptr < object->pointer)
> +		unsigned long untagged_objp;
> +
> +		untagged_objp = (unsigned long)kasan_reset_tag((void
> *)object->pointer);
> +
> +		if (untagged_ptr < untagged_objp)
>  			rb = object->rb_node.rb_left;
> -		else if (object->pointer + object->size <= ptr)
> +		else if (untagged_objp + object->size <= untagged_ptr)
>  			rb = object->rb_node.rb_right;
> -		else if (object->pointer == ptr || alias)
> +		else if (untagged_objp == untagged_ptr || alias)
>  			return object;
>  		else {
>  			kmemleak_warn("Found object by alias at
> 0x%08lx\n",
> @@ -576,6 +581,7 @@ static struct kmemleak_object
> *create_object(unsigned long ptr, size_t size,
>  	struct kmemleak_object *object, *parent;
>  	struct rb_node **link, *rb_parent;
>  	unsigned long untagged_ptr;
> +	unsigned long untagged_objp;
>  
>  	object = mem_pool_alloc(gfp);
>  	if (!object) {
> @@ -629,9 +635,10 @@ static struct kmemleak_object
> *create_object(unsigned long ptr, size_t size,
>  	while (*link) {
>  		rb_parent = *link;
>  		parent = rb_entry(rb_parent, struct kmemleak_object,
> rb_node);
> -		if (ptr + size <= parent->pointer)
> +		untagged_objp = (unsigned long)kasan_reset_tag((void
> *)parent->pointer);
> +		if (untagged_ptr + size <= untagged_objp)
>  			link = &parent->rb_node.rb_left;
> -		else if (parent->pointer + parent->size <= ptr)
> +		else if (untagged_objp + parent->size <= untagged_ptr)
>  			link = &parent->rb_node.rb_right;
>  		else {
>  			kmemleak_stop("Cannot insert 0x%lx into the
> object search tree (overlaps existing)\n",
> -- 
> 2.18.0
> 


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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-18  9:20 ` Kuan-Ying Lee
@ 2021-11-19 14:15   ` Andrey Konovalov
  2021-11-19 15:12     ` Kuan-Ying Lee
  2021-11-25 16:13   ` Andrey Konovalov
  1 sibling, 1 reply; 9+ messages in thread
From: Andrey Konovalov @ 2021-11-19 14:15 UTC (permalink / raw)
  To: Kuan-Ying Lee
  Cc: Catalin Marinas, Andrew Morton, Matthias Brugger,
	Chinwen Chang (張錦文),
	Nicholas Tang (鄭秦輝),
	James Hsu (徐慶薰),
	Yee Lee (李建誼),
	linux-mm, linux-kernel, linux-arm-kernel, linux-mediatek,
	kasan-dev

On Thu, Nov 18, 2021 at 10:20 AM Kuan-Ying Lee
<Kuan-Ying.Lee@mediatek.com> wrote:
>
> +Cc kasan group
>
> On Thu, 2021-11-18 at 13:44 +0800, Kuan-Ying Lee wrote:
> > With HW tag-based kasan enable, We will get the warning
> > when we free object whose address starts with 0xFF.
> >
> > It is because kmemleak rbtree stores tagged object and
> > this freeing object's tag does not match with rbtree object.
> >
> > In the example below, kmemleak rbtree stores the tagged object in
> > the kmalloc(), and kfree() gets the pointer with 0xFF tag.
> >
> > Call sequence:
> > ptr = kmalloc(size, GFP_KERNEL);
> > page = virt_to_page(ptr);
> > kfree(page_address(page));
> > ptr = kmalloc(size, GFP_KERNEL);

How is this call sequence valid? page_address returns the address of
the start of the page, while kmalloced object could have been located
in the middle of it.

> >
> > Call sequence like that may cause the warning as following:
> > 1) Freeing unknown object:
> > In kfree(), we will get free unknown object warning in
> > kmemleak_free().
> > Because object(0xFx) in kmemleak rbtree and pointer(0xFF) in kfree()
> > have
> > different tag.
> >
> > 2) Overlap existing:
> > When we allocate that object with the same hw-tag again, we will
> > find the overlap in the kmemleak rbtree and kmemleak thread will
> > be killed.
> >
> > [  116.685312] kmemleak: Freeing unknown object at 0xffff000003f88000
> > [  116.686422] CPU: 5 PID: 177 Comm: cat Not tainted 5.16.0-rc1-dirty
> > #21
> > [  116.687067] Hardware name: linux,dummy-virt (DT)
> > [  116.687496] Call trace:
> > [  116.687792]  dump_backtrace+0x0/0x1ac
> > [  116.688255]  show_stack+0x1c/0x30
> > [  116.688663]  dump_stack_lvl+0x68/0x84
> > [  116.689096]  dump_stack+0x1c/0x38
> > [  116.689499]  kmemleak_free+0x6c/0x70
> > [  116.689919]  slab_free_freelist_hook+0x104/0x200
> > [  116.690420]  kmem_cache_free+0xa8/0x3d4
> > [  116.690845]  test_version_show+0x270/0x3a0
> > [  116.691344]  module_attr_show+0x28/0x40
> > [  116.691789]  sysfs_kf_seq_show+0xb0/0x130
> > [  116.692245]  kernfs_seq_show+0x30/0x40
> > [  116.692678]  seq_read_iter+0x1bc/0x4b0
> > [  116.692678]  seq_read_iter+0x1bc/0x4b0
> > [  116.693114]  kernfs_fop_read_iter+0x144/0x1c0
> > [  116.693586]  generic_file_splice_read+0xd0/0x184
> > [  116.694078]  do_splice_to+0x90/0xe0
> > [  116.694498]  splice_direct_to_actor+0xb8/0x250
> > [  116.694975]  do_splice_direct+0x88/0xd4
> > [  116.695409]  do_sendfile+0x2b0/0x344
> > [  116.695829]  __arm64_sys_sendfile64+0x164/0x16c
> > [  116.696306]  invoke_syscall+0x48/0x114
> > [  116.696735]  el0_svc_common.constprop.0+0x44/0xec
> > [  116.697263]  do_el0_svc+0x74/0x90
> > [  116.697665]  el0_svc+0x20/0x80
> > [  116.698261]  el0t_64_sync_handler+0x1a8/0x1b0
> > [  116.698695]  el0t_64_sync+0x1ac/0x1b0
> > ...
> > [  117.520301] kmemleak: Cannot insert 0xf2ff000003f88000 into the
> > object search tree (overlaps existing)
> > [  117.521118] CPU: 5 PID: 178 Comm: cat Not tainted 5.16.0-rc1-dirty
> > #21
> > [  117.521827] Hardware name: linux,dummy-virt (DT)
> > [  117.522287] Call trace:
> > [  117.522586]  dump_backtrace+0x0/0x1ac
> > [  117.523053]  show_stack+0x1c/0x30
> > [  117.523578]  dump_stack_lvl+0x68/0x84
> > [  117.524039]  dump_stack+0x1c/0x38
> > [  117.524472]  create_object.isra.0+0x2d8/0x2fc
> > [  117.524975]  kmemleak_alloc+0x34/0x40
> > [  117.525416]  kmem_cache_alloc+0x23c/0x2f0
> > [  117.525914]  test_version_show+0x1fc/0x3a0
> > [  117.526379]  module_attr_show+0x28/0x40
> > [  117.526827]  sysfs_kf_seq_show+0xb0/0x130
> > [  117.527363]  kernfs_seq_show+0x30/0x40
> > [  117.527848]  seq_read_iter+0x1bc/0x4b0
> > [  117.528320]  kernfs_fop_read_iter+0x144/0x1c0
> > [  117.528809]  generic_file_splice_read+0xd0/0x184
> > [  117.529316]  do_splice_to+0x90/0xe0
> > [  117.529734]  splice_direct_to_actor+0xb8/0x250
> > [  117.530227]  do_splice_direct+0x88/0xd4
> > [  117.530686]  do_sendfile+0x2b0/0x344
> > [  117.531154]  __arm64_sys_sendfile64+0x164/0x16c
> > [  117.531673]  invoke_syscall+0x48/0x114
> > [  117.532111]  el0_svc_common.constprop.0+0x44/0xec
> > [  117.532621]  do_el0_svc+0x74/0x90
> > [  117.533048]  el0_svc+0x20/0x80
> > [  117.533461]  el0t_64_sync_handler+0x1a8/0x1b0
> > [  117.533950]  el0t_64_sync+0x1ac/0x1b0
> > [  117.534625] kmemleak: Kernel memory leak detector disabled
> > [  117.535201] kmemleak: Object 0xf2ff000003f88000 (size 128):
> > [  117.535761] kmemleak:   comm "cat", pid 177, jiffies 4294921177
> > [  117.536339] kmemleak:   min_count = 1
> > [  117.536718] kmemleak:   count = 0
> > [  117.537068] kmemleak:   flags = 0x1
> > [  117.537429] kmemleak:   checksum = 0
> > [  117.537806] kmemleak:   backtrace:
> > [  117.538211]      kmem_cache_alloc+0x23c/0x2f0
> > [  117.538924]      test_version_show+0x1fc/0x3a0
> > [  117.539393]      module_attr_show+0x28/0x40
> > [  117.539844]      sysfs_kf_seq_show+0xb0/0x130
> > [  117.540304]      kernfs_seq_show+0x30/0x40
> > [  117.540750]      seq_read_iter+0x1bc/0x4b0
> > [  117.541206]      kernfs_fop_read_iter+0x144/0x1c0
> > [  117.541687]      generic_file_splice_read+0xd0/0x184
> > [  117.542182]      do_splice_to+0x90/0xe0
> > [  117.542611]      splice_direct_to_actor+0xb8/0x250
> > [  117.543097]      do_splice_direct+0x88/0xd4
> > [  117.543544]      do_sendfile+0x2b0/0x344
> > [  117.543983]      __arm64_sys_sendfile64+0x164/0x16c
> > [  117.544471]      invoke_syscall+0x48/0x114
> > [  117.544917]      el0_svc_common.constprop.0+0x44/0xec
> > [  117.545416]      do_el0_svc+0x74/0x90
> > [  117.554100] kmemleak: Automatic memory scanning thread ended
> >
> > Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com>
> > ---
> >  mm/kmemleak.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index b57383c17cf6..fa12e2e08cdc 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -381,15 +381,20 @@ static void dump_object_info(struct
> > kmemleak_object *object)
> >  static struct kmemleak_object *lookup_object(unsigned long ptr, int
> > alias)
> >  {
> >       struct rb_node *rb = object_tree_root.rb_node;
> > +     unsigned long untagged_ptr = (unsigned
> > long)kasan_reset_tag((void *)ptr);
> >
> >       while (rb) {
> >               struct kmemleak_object *object =
> >                       rb_entry(rb, struct kmemleak_object, rb_node);
> > -             if (ptr < object->pointer)
> > +             unsigned long untagged_objp;
> > +
> > +             untagged_objp = (unsigned long)kasan_reset_tag((void
> > *)object->pointer);
> > +
> > +             if (untagged_ptr < untagged_objp)
> >                       rb = object->rb_node.rb_left;
> > -             else if (object->pointer + object->size <= ptr)
> > +             else if (untagged_objp + object->size <= untagged_ptr)
> >                       rb = object->rb_node.rb_right;
> > -             else if (object->pointer == ptr || alias)
> > +             else if (untagged_objp == untagged_ptr || alias)
> >                       return object;
> >               else {
> >                       kmemleak_warn("Found object by alias at
> > 0x%08lx\n",
> > @@ -576,6 +581,7 @@ static struct kmemleak_object
> > *create_object(unsigned long ptr, size_t size,
> >       struct kmemleak_object *object, *parent;
> >       struct rb_node **link, *rb_parent;
> >       unsigned long untagged_ptr;
> > +     unsigned long untagged_objp;
> >
> >       object = mem_pool_alloc(gfp);
> >       if (!object) {
> > @@ -629,9 +635,10 @@ static struct kmemleak_object
> > *create_object(unsigned long ptr, size_t size,
> >       while (*link) {
> >               rb_parent = *link;
> >               parent = rb_entry(rb_parent, struct kmemleak_object,
> > rb_node);
> > -             if (ptr + size <= parent->pointer)
> > +             untagged_objp = (unsigned long)kasan_reset_tag((void
> > *)parent->pointer);
> > +             if (untagged_ptr + size <= untagged_objp)
> >                       link = &parent->rb_node.rb_left;
> > -             else if (parent->pointer + parent->size <= ptr)
> > +             else if (untagged_objp + parent->size <= untagged_ptr)
> >                       link = &parent->rb_node.rb_right;
> >               else {
> >                       kmemleak_stop("Cannot insert 0x%lx into the
> > object search tree (overlaps existing)\n",
> > --
> > 2.18.0
> >
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/754511d9a0368065768cc3ad8037184d62c3fbd1.camel%40mediatek.com.

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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-19 14:15   ` Andrey Konovalov
@ 2021-11-19 15:12     ` Kuan-Ying Lee
  2021-11-19 22:43       ` Andrew Morton
  0 siblings, 1 reply; 9+ messages in thread
From: Kuan-Ying Lee @ 2021-11-19 15:12 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Catalin Marinas, Andrew Morton, Matthias Brugger,
	Chinwen Chang (張錦文),
	Nicholas Tang (鄭秦輝),
	James Hsu (徐慶薰),
	Yee Lee (李建誼),
	linux-mm, linux-kernel, linux-arm-kernel, linux-mediatek,
	kasan-dev, kuan-ying.lee

On Fri, 2021-11-19 at 22:15 +0800, Andrey Konovalov wrote:
> On Thu, Nov 18, 2021 at 10:20 AM Kuan-Ying Lee
> <Kuan-Ying.Lee@mediatek.com> wrote:
> > 
> > +Cc kasan group
> > 
> > On Thu, 2021-11-18 at 13:44 +0800, Kuan-Ying Lee wrote:
> > > With HW tag-based kasan enable, We will get the warning
> > > when we free object whose address starts with 0xFF.
> > > 
> > > It is because kmemleak rbtree stores tagged object and
> > > this freeing object's tag does not match with rbtree object.
> > > 
> > > In the example below, kmemleak rbtree stores the tagged object in
> > > the kmalloc(), and kfree() gets the pointer with 0xFF tag.
> > > 
> > > Call sequence:
> > > ptr = kmalloc(size, GFP_KERNEL);
> > > page = virt_to_page(ptr);
> > > kfree(page_address(page));
> > > ptr = kmalloc(size, GFP_KERNEL);
> 
> How is this call sequence valid? page_address returns the address of
> the start of the page, while kmalloced object could have been located
> in the middle of it.

Thanks for pointing out. I miss the offset.

It should be listed as below.

ptr = kmalloc(size, GFP_KERNEL);
page = virt_to_page(ptr);
offset = offset_in_page(ptr);
kfree(page_address(page) + offset);
ptr = kmalloc(size, GFP_KERNEL);

> 
> > > 
> > > Call sequence like that may cause the warning as following:
> > > 1) Freeing unknown object:
> > > In kfree(), we will get free unknown object warning in
> > > kmemleak_free().
> > > Because object(0xFx) in kmemleak rbtree and pointer(0xFF) in
> > > kfree()
> > > have
> > > different tag.
> > > 
> > > 2) Overlap existing:
> > > When we allocate that object with the same hw-tag again, we will
> > > find the overlap in the kmemleak rbtree and kmemleak thread will
> > > be killed.
> > > 
> > > [  116.685312] kmemleak: Freeing unknown object at
> > > 0xffff000003f88000
> > > [  116.686422] CPU: 5 PID: 177 Comm: cat Not tainted 5.16.0-rc1-
> > > dirty
> > > #21
> > > [  116.687067] Hardware name: linux,dummy-virt (DT)
> > > [  116.687496] Call trace:
> > > [  116.687792]  dump_backtrace+0x0/0x1ac
> > > [  116.688255]  show_stack+0x1c/0x30
> > > [  116.688663]  dump_stack_lvl+0x68/0x84
> > > [  116.689096]  dump_stack+0x1c/0x38
> > > [  116.689499]  kmemleak_free+0x6c/0x70
> > > [  116.689919]  slab_free_freelist_hook+0x104/0x200
> > > [  116.690420]  kmem_cache_free+0xa8/0x3d4
> > > [  116.690845]  test_version_show+0x270/0x3a0
> > > [  116.691344]  module_attr_show+0x28/0x40
> > > [  116.691789]  sysfs_kf_seq_show+0xb0/0x130
> > > [  116.692245]  kernfs_seq_show+0x30/0x40
> > > [  116.692678]  seq_read_iter+0x1bc/0x4b0
> > > [  116.692678]  seq_read_iter+0x1bc/0x4b0
> > > [  116.693114]  kernfs_fop_read_iter+0x144/0x1c0
> > > [  116.693586]  generic_file_splice_read+0xd0/0x184
> > > [  116.694078]  do_splice_to+0x90/0xe0
> > > [  116.694498]  splice_direct_to_actor+0xb8/0x250
> > > [  116.694975]  do_splice_direct+0x88/0xd4
> > > [  116.695409]  do_sendfile+0x2b0/0x344
> > > [  116.695829]  __arm64_sys_sendfile64+0x164/0x16c
> > > [  116.696306]  invoke_syscall+0x48/0x114
> > > [  116.696735]  el0_svc_common.constprop.0+0x44/0xec
> > > [  116.697263]  do_el0_svc+0x74/0x90
> > > [  116.697665]  el0_svc+0x20/0x80
> > > [  116.698261]  el0t_64_sync_handler+0x1a8/0x1b0
> > > [  116.698695]  el0t_64_sync+0x1ac/0x1b0
> > > ...
> > > [  117.520301] kmemleak: Cannot insert 0xf2ff000003f88000 into
> > > the
> > > object search tree (overlaps existing)
> > > [  117.521118] CPU: 5 PID: 178 Comm: cat Not tainted 5.16.0-rc1-
> > > dirty
> > > #21
> > > [  117.521827] Hardware name: linux,dummy-virt (DT)
> > > [  117.522287] Call trace:
> > > [  117.522586]  dump_backtrace+0x0/0x1ac
> > > [  117.523053]  show_stack+0x1c/0x30
> > > [  117.523578]  dump_stack_lvl+0x68/0x84
> > > [  117.524039]  dump_stack+0x1c/0x38
> > > [  117.524472]  create_object.isra.0+0x2d8/0x2fc
> > > [  117.524975]  kmemleak_alloc+0x34/0x40
> > > [  117.525416]  kmem_cache_alloc+0x23c/0x2f0
> > > [  117.525914]  test_version_show+0x1fc/0x3a0
> > > [  117.526379]  module_attr_show+0x28/0x40
> > > [  117.526827]  sysfs_kf_seq_show+0xb0/0x130
> > > [  117.527363]  kernfs_seq_show+0x30/0x40
> > > [  117.527848]  seq_read_iter+0x1bc/0x4b0
> > > [  117.528320]  kernfs_fop_read_iter+0x144/0x1c0
> > > [  117.528809]  generic_file_splice_read+0xd0/0x184
> > > [  117.529316]  do_splice_to+0x90/0xe0
> > > [  117.529734]  splice_direct_to_actor+0xb8/0x250
> > > [  117.530227]  do_splice_direct+0x88/0xd4
> > > [  117.530686]  do_sendfile+0x2b0/0x344
> > > [  117.531154]  __arm64_sys_sendfile64+0x164/0x16c
> > > [  117.531673]  invoke_syscall+0x48/0x114
> > > [  117.532111]  el0_svc_common.constprop.0+0x44/0xec
> > > [  117.532621]  do_el0_svc+0x74/0x90
> > > [  117.533048]  el0_svc+0x20/0x80
> > > [  117.533461]  el0t_64_sync_handler+0x1a8/0x1b0
> > > [  117.533950]  el0t_64_sync+0x1ac/0x1b0
> > > [  117.534625] kmemleak: Kernel memory leak detector disabled
> > > [  117.535201] kmemleak: Object 0xf2ff000003f88000 (size 128):
> > > [  117.535761] kmemleak:   comm "cat", pid 177, jiffies
> > > 4294921177
> > > [  117.536339] kmemleak:   min_count = 1
> > > [  117.536718] kmemleak:   count = 0
> > > [  117.537068] kmemleak:   flags = 0x1
> > > [  117.537429] kmemleak:   checksum = 0
> > > [  117.537806] kmemleak:   backtrace:
> > > [  117.538211]      kmem_cache_alloc+0x23c/0x2f0
> > > [  117.538924]      test_version_show+0x1fc/0x3a0
> > > [  117.539393]      module_attr_show+0x28/0x40
> > > [  117.539844]      sysfs_kf_seq_show+0xb0/0x130
> > > [  117.540304]      kernfs_seq_show+0x30/0x40
> > > [  117.540750]      seq_read_iter+0x1bc/0x4b0
> > > [  117.541206]      kernfs_fop_read_iter+0x144/0x1c0
> > > [  117.541687]      generic_file_splice_read+0xd0/0x184
> > > [  117.542182]      do_splice_to+0x90/0xe0
> > > [  117.542611]      splice_direct_to_actor+0xb8/0x250
> > > [  117.543097]      do_splice_direct+0x88/0xd4
> > > [  117.543544]      do_sendfile+0x2b0/0x344
> > > [  117.543983]      __arm64_sys_sendfile64+0x164/0x16c
> > > [  117.544471]      invoke_syscall+0x48/0x114
> > > [  117.544917]      el0_svc_common.constprop.0+0x44/0xec
> > > [  117.545416]      do_el0_svc+0x74/0x90
> > > [  117.554100] kmemleak: Automatic memory scanning thread ended
> > > 
> > > Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com>
> > > ---
> > >  mm/kmemleak.c | 17 ++++++++++++-----
> > >  1 file changed, 12 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > > index b57383c17cf6..fa12e2e08cdc 100644
> > > --- a/mm/kmemleak.c
> > > +++ b/mm/kmemleak.c
> > > @@ -381,15 +381,20 @@ static void dump_object_info(struct
> > > kmemleak_object *object)
> > >  static struct kmemleak_object *lookup_object(unsigned long ptr,
> > > int
> > > alias)
> > >  {
> > >       struct rb_node *rb = object_tree_root.rb_node;
> > > +     unsigned long untagged_ptr = (unsigned
> > > long)kasan_reset_tag((void *)ptr);
> > > 
> > >       while (rb) {
> > >               struct kmemleak_object *object =
> > >                       rb_entry(rb, struct kmemleak_object,
> > > rb_node);
> > > -             if (ptr < object->pointer)
> > > +             unsigned long untagged_objp;
> > > +
> > > +             untagged_objp = (unsigned
> > > long)kasan_reset_tag((void
> > > *)object->pointer);
> > > +
> > > +             if (untagged_ptr < untagged_objp)
> > >                       rb = object->rb_node.rb_left;
> > > -             else if (object->pointer + object->size <= ptr)
> > > +             else if (untagged_objp + object->size <=
> > > untagged_ptr)
> > >                       rb = object->rb_node.rb_right;
> > > -             else if (object->pointer == ptr || alias)
> > > +             else if (untagged_objp == untagged_ptr || alias)
> > >                       return object;
> > >               else {
> > >                       kmemleak_warn("Found object by alias at
> > > 0x%08lx\n",
> > > @@ -576,6 +581,7 @@ static struct kmemleak_object
> > > *create_object(unsigned long ptr, size_t size,
> > >       struct kmemleak_object *object, *parent;
> > >       struct rb_node **link, *rb_parent;
> > >       unsigned long untagged_ptr;
> > > +     unsigned long untagged_objp;
> > > 
> > >       object = mem_pool_alloc(gfp);
> > >       if (!object) {
> > > @@ -629,9 +635,10 @@ static struct kmemleak_object
> > > *create_object(unsigned long ptr, size_t size,
> > >       while (*link) {
> > >               rb_parent = *link;
> > >               parent = rb_entry(rb_parent, struct
> > > kmemleak_object,
> > > rb_node);
> > > -             if (ptr + size <= parent->pointer)
> > > +             untagged_objp = (unsigned
> > > long)kasan_reset_tag((void
> > > *)parent->pointer);
> > > +             if (untagged_ptr + size <= untagged_objp)
> > >                       link = &parent->rb_node.rb_left;
> > > -             else if (parent->pointer + parent->size <= ptr)
> > > +             else if (untagged_objp + parent->size <=
> > > untagged_ptr)
> > >                       link = &parent->rb_node.rb_right;
> > >               else {
> > >                       kmemleak_stop("Cannot insert 0x%lx into the
> > > object search tree (overlaps existing)\n",
> > > --
> > > 2.18.0
> > > 
> > 
> > --
> > You received this message because you are subscribed to the Google
> > Groups "kasan-dev" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to kasan-dev+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit 
> > https://urldefense.com/v3/__https://groups.google.com/d/msgid/kasan-dev/754511d9a0368065768cc3ad8037184d62c3fbd1.camel*40mediatek.com__;JQ!!CTRNKA9wMg0ARbw!y7gGU0PsiMId4XiGTZzBUUL_WtWQ24nnTQtGbFrZ46wqfwk8ZeYCVP2pYmNFHrAUGOKr1g$
> >  .


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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-19 15:12     ` Kuan-Ying Lee
@ 2021-11-19 22:43       ` Andrew Morton
  2021-11-24  2:00         ` Kuan-Ying Lee
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2021-11-19 22:43 UTC (permalink / raw)
  To: Kuan-Ying Lee
  Cc: Andrey Konovalov, Catalin Marinas, Matthias Brugger,
	Chinwen Chang, Nicholas Tang, James Hsu, Yee Lee, linux-mm,
	linux-kernel, linux-arm-kernel, linux-mediatek, kasan-dev,
	kuan-ying.lee

On Fri, 19 Nov 2021 23:12:55 +0800 Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com> wrote:

> > > > Call sequence:
> > > > ptr = kmalloc(size, GFP_KERNEL);
> > > > page = virt_to_page(ptr);
> > > > kfree(page_address(page));
> > > > ptr = kmalloc(size, GFP_KERNEL);
> > 
> > How is this call sequence valid? page_address returns the address of
> > the start of the page, while kmalloced object could have been located
> > in the middle of it.
> 
> Thanks for pointing out. I miss the offset.
> 
> It should be listed as below.
> 
> ptr = kmalloc(size, GFP_KERNEL);
> page = virt_to_page(ptr);
> offset = offset_in_page(ptr);
> kfree(page_address(page) + offset);
> ptr = kmalloc(size, GFP_KERNEL);

I updated the changelog to reflect this.

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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-19 22:43       ` Andrew Morton
@ 2021-11-24  2:00         ` Kuan-Ying Lee
  0 siblings, 0 replies; 9+ messages in thread
From: Kuan-Ying Lee @ 2021-11-24  2:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Catalin Marinas, Matthias Brugger,
	Chinwen Chang (張錦文),
	Nicholas Tang (鄭秦輝),
	Yee Lee (李建誼),
	linux-mm, linux-kernel, linux-arm-kernel, linux-mediatek,
	kasan-dev, james.hsu, kuan-ying.lee

On Sat, 2021-11-20 at 06:43 +0800, Andrew Morton wrote:
> On Fri, 19 Nov 2021 23:12:55 +0800 Kuan-Ying Lee <
> Kuan-Ying.Lee@mediatek.com> wrote:
> 
> > > > > Call sequence:
> > > > > ptr = kmalloc(size, GFP_KERNEL);
> > > > > page = virt_to_page(ptr);
> > > > > kfree(page_address(page));
> > > > > ptr = kmalloc(size, GFP_KERNEL);
> > > 
> > > How is this call sequence valid? page_address returns the address
> > > of
> > > the start of the page, while kmalloced object could have been
> > > located
> > > in the middle of it.
> > 
> > Thanks for pointing out. I miss the offset.
> > 
> > It should be listed as below.
> > 
> > ptr = kmalloc(size, GFP_KERNEL);
> > page = virt_to_page(ptr);
> > offset = offset_in_page(ptr);
> > kfree(page_address(page) + offset);
> > ptr = kmalloc(size, GFP_KERNEL);
> 
> I updated the changelog to reflect this.

Thanks for updating changelog. :)


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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-18  9:20 ` Kuan-Ying Lee
  2021-11-19 14:15   ` Andrey Konovalov
@ 2021-11-25 16:13   ` Andrey Konovalov
  2021-11-28  0:20     ` Andrew Morton
  1 sibling, 1 reply; 9+ messages in thread
From: Andrey Konovalov @ 2021-11-25 16:13 UTC (permalink / raw)
  To: Kuan-Ying Lee
  Cc: Catalin Marinas, Andrew Morton, Matthias Brugger,
	Chinwen Chang (張錦文),
	Nicholas Tang (鄭秦輝),
	James Hsu (徐慶薰),
	Yee Lee (李建誼),
	linux-mm, linux-kernel, linux-arm-kernel, linux-mediatek,
	kasan-dev

On Thu, Nov 18, 2021 at 10:20 AM Kuan-Ying Lee
<Kuan-Ying.Lee@mediatek.com> wrote:
>
> +Cc kasan group
>
> On Thu, 2021-11-18 at 13:44 +0800, Kuan-Ying Lee wrote:
> > With HW tag-based kasan enable, We will get the warning
> > when we free object whose address starts with 0xFF.
> >
> > It is because kmemleak rbtree stores tagged object and
> > this freeing object's tag does not match with rbtree object.
> >
> > In the example below, kmemleak rbtree stores the tagged object in
> > the kmalloc(), and kfree() gets the pointer with 0xFF tag.
> >
> > Call sequence:
> > ptr = kmalloc(size, GFP_KERNEL);
> > page = virt_to_page(ptr);
> > kfree(page_address(page));
> > ptr = kmalloc(size, GFP_KERNEL);
> >
> > Call sequence like that may cause the warning as following:
> > 1) Freeing unknown object:
> > In kfree(), we will get free unknown object warning in
> > kmemleak_free().
> > Because object(0xFx) in kmemleak rbtree and pointer(0xFF) in kfree()
> > have
> > different tag.
> >
> > 2) Overlap existing:
> > When we allocate that object with the same hw-tag again, we will
> > find the overlap in the kmemleak rbtree and kmemleak thread will
> > be killed.
> >
> > [  116.685312] kmemleak: Freeing unknown object at 0xffff000003f88000
> > [  116.686422] CPU: 5 PID: 177 Comm: cat Not tainted 5.16.0-rc1-dirty
> > #21
> > [  116.687067] Hardware name: linux,dummy-virt (DT)
> > [  116.687496] Call trace:
> > [  116.687792]  dump_backtrace+0x0/0x1ac
> > [  116.688255]  show_stack+0x1c/0x30
> > [  116.688663]  dump_stack_lvl+0x68/0x84
> > [  116.689096]  dump_stack+0x1c/0x38
> > [  116.689499]  kmemleak_free+0x6c/0x70
> > [  116.689919]  slab_free_freelist_hook+0x104/0x200
> > [  116.690420]  kmem_cache_free+0xa8/0x3d4
> > [  116.690845]  test_version_show+0x270/0x3a0
> > [  116.691344]  module_attr_show+0x28/0x40
> > [  116.691789]  sysfs_kf_seq_show+0xb0/0x130
> > [  116.692245]  kernfs_seq_show+0x30/0x40
> > [  116.692678]  seq_read_iter+0x1bc/0x4b0
> > [  116.692678]  seq_read_iter+0x1bc/0x4b0
> > [  116.693114]  kernfs_fop_read_iter+0x144/0x1c0
> > [  116.693586]  generic_file_splice_read+0xd0/0x184
> > [  116.694078]  do_splice_to+0x90/0xe0
> > [  116.694498]  splice_direct_to_actor+0xb8/0x250
> > [  116.694975]  do_splice_direct+0x88/0xd4
> > [  116.695409]  do_sendfile+0x2b0/0x344
> > [  116.695829]  __arm64_sys_sendfile64+0x164/0x16c
> > [  116.696306]  invoke_syscall+0x48/0x114
> > [  116.696735]  el0_svc_common.constprop.0+0x44/0xec
> > [  116.697263]  do_el0_svc+0x74/0x90
> > [  116.697665]  el0_svc+0x20/0x80
> > [  116.698261]  el0t_64_sync_handler+0x1a8/0x1b0
> > [  116.698695]  el0t_64_sync+0x1ac/0x1b0
> > ...
> > [  117.520301] kmemleak: Cannot insert 0xf2ff000003f88000 into the
> > object search tree (overlaps existing)
> > [  117.521118] CPU: 5 PID: 178 Comm: cat Not tainted 5.16.0-rc1-dirty
> > #21
> > [  117.521827] Hardware name: linux,dummy-virt (DT)
> > [  117.522287] Call trace:
> > [  117.522586]  dump_backtrace+0x0/0x1ac
> > [  117.523053]  show_stack+0x1c/0x30
> > [  117.523578]  dump_stack_lvl+0x68/0x84
> > [  117.524039]  dump_stack+0x1c/0x38
> > [  117.524472]  create_object.isra.0+0x2d8/0x2fc
> > [  117.524975]  kmemleak_alloc+0x34/0x40
> > [  117.525416]  kmem_cache_alloc+0x23c/0x2f0
> > [  117.525914]  test_version_show+0x1fc/0x3a0
> > [  117.526379]  module_attr_show+0x28/0x40
> > [  117.526827]  sysfs_kf_seq_show+0xb0/0x130
> > [  117.527363]  kernfs_seq_show+0x30/0x40
> > [  117.527848]  seq_read_iter+0x1bc/0x4b0
> > [  117.528320]  kernfs_fop_read_iter+0x144/0x1c0
> > [  117.528809]  generic_file_splice_read+0xd0/0x184
> > [  117.529316]  do_splice_to+0x90/0xe0
> > [  117.529734]  splice_direct_to_actor+0xb8/0x250
> > [  117.530227]  do_splice_direct+0x88/0xd4
> > [  117.530686]  do_sendfile+0x2b0/0x344
> > [  117.531154]  __arm64_sys_sendfile64+0x164/0x16c
> > [  117.531673]  invoke_syscall+0x48/0x114
> > [  117.532111]  el0_svc_common.constprop.0+0x44/0xec
> > [  117.532621]  do_el0_svc+0x74/0x90
> > [  117.533048]  el0_svc+0x20/0x80
> > [  117.533461]  el0t_64_sync_handler+0x1a8/0x1b0
> > [  117.533950]  el0t_64_sync+0x1ac/0x1b0
> > [  117.534625] kmemleak: Kernel memory leak detector disabled
> > [  117.535201] kmemleak: Object 0xf2ff000003f88000 (size 128):
> > [  117.535761] kmemleak:   comm "cat", pid 177, jiffies 4294921177
> > [  117.536339] kmemleak:   min_count = 1
> > [  117.536718] kmemleak:   count = 0
> > [  117.537068] kmemleak:   flags = 0x1
> > [  117.537429] kmemleak:   checksum = 0
> > [  117.537806] kmemleak:   backtrace:
> > [  117.538211]      kmem_cache_alloc+0x23c/0x2f0
> > [  117.538924]      test_version_show+0x1fc/0x3a0
> > [  117.539393]      module_attr_show+0x28/0x40
> > [  117.539844]      sysfs_kf_seq_show+0xb0/0x130
> > [  117.540304]      kernfs_seq_show+0x30/0x40
> > [  117.540750]      seq_read_iter+0x1bc/0x4b0
> > [  117.541206]      kernfs_fop_read_iter+0x144/0x1c0
> > [  117.541687]      generic_file_splice_read+0xd0/0x184
> > [  117.542182]      do_splice_to+0x90/0xe0
> > [  117.542611]      splice_direct_to_actor+0xb8/0x250
> > [  117.543097]      do_splice_direct+0x88/0xd4
> > [  117.543544]      do_sendfile+0x2b0/0x344
> > [  117.543983]      __arm64_sys_sendfile64+0x164/0x16c
> > [  117.544471]      invoke_syscall+0x48/0x114
> > [  117.544917]      el0_svc_common.constprop.0+0x44/0xec
> > [  117.545416]      do_el0_svc+0x74/0x90
> > [  117.554100] kmemleak: Automatic memory scanning thread ended
> >
> > Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com>
> > ---
> >  mm/kmemleak.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> > index b57383c17cf6..fa12e2e08cdc 100644
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -381,15 +381,20 @@ static void dump_object_info(struct
> > kmemleak_object *object)
> >  static struct kmemleak_object *lookup_object(unsigned long ptr, int
> > alias)
> >  {
> >       struct rb_node *rb = object_tree_root.rb_node;
> > +     unsigned long untagged_ptr = (unsigned
> > long)kasan_reset_tag((void *)ptr);
> >
> >       while (rb) {
> >               struct kmemleak_object *object =
> >                       rb_entry(rb, struct kmemleak_object, rb_node);
> > -             if (ptr < object->pointer)
> > +             unsigned long untagged_objp;
> > +
> > +             untagged_objp = (unsigned long)kasan_reset_tag((void
> > *)object->pointer);

The two lines above can be squashed together.

> > +
> > +             if (untagged_ptr < untagged_objp)
> >                       rb = object->rb_node.rb_left;
> > -             else if (object->pointer + object->size <= ptr)
> > +             else if (untagged_objp + object->size <= untagged_ptr)
> >                       rb = object->rb_node.rb_right;
> > -             else if (object->pointer == ptr || alias)
> > +             else if (untagged_objp == untagged_ptr || alias)
> >                       return object;
> >               else {
> >                       kmemleak_warn("Found object by alias at
> > 0x%08lx\n",
> > @@ -576,6 +581,7 @@ static struct kmemleak_object
> > *create_object(unsigned long ptr, size_t size,
> >       struct kmemleak_object *object, *parent;
> >       struct rb_node **link, *rb_parent;
> >       unsigned long untagged_ptr;
> > +     unsigned long untagged_objp;
> >
> >       object = mem_pool_alloc(gfp);
> >       if (!object) {
> > @@ -629,9 +635,10 @@ static struct kmemleak_object
> > *create_object(unsigned long ptr, size_t size,
> >       while (*link) {
> >               rb_parent = *link;
> >               parent = rb_entry(rb_parent, struct kmemleak_object,
> > rb_node);
> > -             if (ptr + size <= parent->pointer)
> > +             untagged_objp = (unsigned long)kasan_reset_tag((void
> > *)parent->pointer);
> > +             if (untagged_ptr + size <= untagged_objp)
> >                       link = &parent->rb_node.rb_left;
> > -             else if (parent->pointer + parent->size <= ptr)
> > +             else if (untagged_objp + parent->size <= untagged_ptr)
> >                       link = &parent->rb_node.rb_right;
> >               else {
> >                       kmemleak_stop("Cannot insert 0x%lx into the
> > object search tree (overlaps existing)\n",
> > --
> > 2.18.0
> >
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/754511d9a0368065768cc3ad8037184d62c3fbd1.camel%40mediatek.com.

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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-25 16:13   ` Andrey Konovalov
@ 2021-11-28  0:20     ` Andrew Morton
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2021-11-28  0:20 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Kuan-Ying Lee, Catalin Marinas, Matthias Brugger, Chinwen Chang,
	Nicholas Tang, James Hsu, Yee Lee, linux-mm, linux-kernel,
	linux-arm-kernel, linux-mediatek, kasan-dev

On Thu, 25 Nov 2021 17:13:36 +0100 Andrey Konovalov <andreyknvl@gmail.com> wrote:

> > > kmemleak_object *object)
> > >  static struct kmemleak_object *lookup_object(unsigned long ptr, int
> > > alias)
> > >  {
> > >       struct rb_node *rb = object_tree_root.rb_node;
> > > +     unsigned long untagged_ptr = (unsigned
> > > long)kasan_reset_tag((void *)ptr);
> > >
> > >       while (rb) {
> > >               struct kmemleak_object *object =
> > >                       rb_entry(rb, struct kmemleak_object, rb_node);
> > > -             if (ptr < object->pointer)
> > > +             unsigned long untagged_objp;
> > > +
> > > +             untagged_objp = (unsigned long)kasan_reset_tag((void
> > > *)object->pointer);
> 
> The two lines above can be squashed together.

That would make a too-long line even longer.  In fact I think it's
better to go the other way:

--- a/mm/kmemleak.c~kmemleak-fix-kmemleak-false-positive-report-with-hw-tag-based-kasan-enable-fix
+++ a/mm/kmemleak.c
@@ -384,10 +384,10 @@ static struct kmemleak_object *lookup_ob
 	unsigned long untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
 
 	while (rb) {
-		struct kmemleak_object *object =
-			rb_entry(rb, struct kmemleak_object, rb_node);
+		struct kmemleak_object *object;
 		unsigned long untagged_objp;
 
+		object = rb_entry(rb, struct kmemleak_object, rb_node);
 		untagged_objp = (unsigned long)kasan_reset_tag((void *)object->pointer);
 
 		if (untagged_ptr < untagged_objp)
_


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

* Re: [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable
  2021-11-18  5:44 [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable Kuan-Ying Lee
  2021-11-18  9:20 ` Kuan-Ying Lee
@ 2021-12-02 18:19 ` Catalin Marinas
  1 sibling, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2021-12-02 18:19 UTC (permalink / raw)
  To: Kuan-Ying Lee
  Cc: Andrew Morton, Matthias Brugger, chinwen.chang, nicholas.tang,
	james.hsu, yee.lee, linux-mm, linux-kernel, linux-arm-kernel,
	linux-mediatek

On Thu, Nov 18, 2021 at 01:44:19PM +0800, Kuan-Ying Lee wrote:
> With HW tag-based kasan enable, We will get the warning
> when we free object whose address starts with 0xFF.
> 
> It is because kmemleak rbtree stores tagged object and
> this freeing object's tag does not match with rbtree object.
> 
> In the example below, kmemleak rbtree stores the tagged object in
> the kmalloc(), and kfree() gets the pointer with 0xFF tag.
> 
> Call sequence:
> ptr = kmalloc(size, GFP_KERNEL);
> page = virt_to_page(ptr);
> kfree(page_address(page));
> ptr = kmalloc(size, GFP_KERNEL);
> 
> Call sequence like that may cause the warning as following:
> 1) Freeing unknown object:
> In kfree(), we will get free unknown object warning in kmemleak_free().
> Because object(0xFx) in kmemleak rbtree and pointer(0xFF) in kfree() have
> different tag.
> 
> 2) Overlap existing:
> When we allocate that object with the same hw-tag again, we will
> find the overlap in the kmemleak rbtree and kmemleak thread will
> be killed.
> 
> [  116.685312] kmemleak: Freeing unknown object at 0xffff000003f88000
> [  116.686422] CPU: 5 PID: 177 Comm: cat Not tainted 5.16.0-rc1-dirty #21
> [  116.687067] Hardware name: linux,dummy-virt (DT)
> [  116.687496] Call trace:
> [  116.687792]  dump_backtrace+0x0/0x1ac
> [  116.688255]  show_stack+0x1c/0x30
> [  116.688663]  dump_stack_lvl+0x68/0x84
> [  116.689096]  dump_stack+0x1c/0x38
> [  116.689499]  kmemleak_free+0x6c/0x70
> [  116.689919]  slab_free_freelist_hook+0x104/0x200
> [  116.690420]  kmem_cache_free+0xa8/0x3d4
> [  116.690845]  test_version_show+0x270/0x3a0
> [  116.691344]  module_attr_show+0x28/0x40
> [  116.691789]  sysfs_kf_seq_show+0xb0/0x130
> [  116.692245]  kernfs_seq_show+0x30/0x40
> [  116.692678]  seq_read_iter+0x1bc/0x4b0
> [  116.692678]  seq_read_iter+0x1bc/0x4b0
> [  116.693114]  kernfs_fop_read_iter+0x144/0x1c0
> [  116.693586]  generic_file_splice_read+0xd0/0x184
> [  116.694078]  do_splice_to+0x90/0xe0
> [  116.694498]  splice_direct_to_actor+0xb8/0x250
> [  116.694975]  do_splice_direct+0x88/0xd4
> [  116.695409]  do_sendfile+0x2b0/0x344
> [  116.695829]  __arm64_sys_sendfile64+0x164/0x16c
> [  116.696306]  invoke_syscall+0x48/0x114
> [  116.696735]  el0_svc_common.constprop.0+0x44/0xec
> [  116.697263]  do_el0_svc+0x74/0x90
> [  116.697665]  el0_svc+0x20/0x80
> [  116.698261]  el0t_64_sync_handler+0x1a8/0x1b0
> [  116.698695]  el0t_64_sync+0x1ac/0x1b0
> ...
> [  117.520301] kmemleak: Cannot insert 0xf2ff000003f88000 into the object search tree (overlaps existing)
> [  117.521118] CPU: 5 PID: 178 Comm: cat Not tainted 5.16.0-rc1-dirty #21
> [  117.521827] Hardware name: linux,dummy-virt (DT)
> [  117.522287] Call trace:
> [  117.522586]  dump_backtrace+0x0/0x1ac
> [  117.523053]  show_stack+0x1c/0x30
> [  117.523578]  dump_stack_lvl+0x68/0x84
> [  117.524039]  dump_stack+0x1c/0x38
> [  117.524472]  create_object.isra.0+0x2d8/0x2fc
> [  117.524975]  kmemleak_alloc+0x34/0x40
> [  117.525416]  kmem_cache_alloc+0x23c/0x2f0
> [  117.525914]  test_version_show+0x1fc/0x3a0
> [  117.526379]  module_attr_show+0x28/0x40
> [  117.526827]  sysfs_kf_seq_show+0xb0/0x130
> [  117.527363]  kernfs_seq_show+0x30/0x40
> [  117.527848]  seq_read_iter+0x1bc/0x4b0
> [  117.528320]  kernfs_fop_read_iter+0x144/0x1c0
> [  117.528809]  generic_file_splice_read+0xd0/0x184
> [  117.529316]  do_splice_to+0x90/0xe0
> [  117.529734]  splice_direct_to_actor+0xb8/0x250
> [  117.530227]  do_splice_direct+0x88/0xd4
> [  117.530686]  do_sendfile+0x2b0/0x344
> [  117.531154]  __arm64_sys_sendfile64+0x164/0x16c
> [  117.531673]  invoke_syscall+0x48/0x114
> [  117.532111]  el0_svc_common.constprop.0+0x44/0xec
> [  117.532621]  do_el0_svc+0x74/0x90
> [  117.533048]  el0_svc+0x20/0x80
> [  117.533461]  el0t_64_sync_handler+0x1a8/0x1b0
> [  117.533950]  el0t_64_sync+0x1ac/0x1b0
> [  117.534625] kmemleak: Kernel memory leak detector disabled
> [  117.535201] kmemleak: Object 0xf2ff000003f88000 (size 128):
> [  117.535761] kmemleak:   comm "cat", pid 177, jiffies 4294921177
> [  117.536339] kmemleak:   min_count = 1
> [  117.536718] kmemleak:   count = 0
> [  117.537068] kmemleak:   flags = 0x1
> [  117.537429] kmemleak:   checksum = 0
> [  117.537806] kmemleak:   backtrace:
> [  117.538211]      kmem_cache_alloc+0x23c/0x2f0
> [  117.538924]      test_version_show+0x1fc/0x3a0
> [  117.539393]      module_attr_show+0x28/0x40
> [  117.539844]      sysfs_kf_seq_show+0xb0/0x130
> [  117.540304]      kernfs_seq_show+0x30/0x40
> [  117.540750]      seq_read_iter+0x1bc/0x4b0
> [  117.541206]      kernfs_fop_read_iter+0x144/0x1c0
> [  117.541687]      generic_file_splice_read+0xd0/0x184
> [  117.542182]      do_splice_to+0x90/0xe0
> [  117.542611]      splice_direct_to_actor+0xb8/0x250
> [  117.543097]      do_splice_direct+0x88/0xd4
> [  117.543544]      do_sendfile+0x2b0/0x344
> [  117.543983]      __arm64_sys_sendfile64+0x164/0x16c
> [  117.544471]      invoke_syscall+0x48/0x114
> [  117.544917]      el0_svc_common.constprop.0+0x44/0xec
> [  117.545416]      do_el0_svc+0x74/0x90
> [  117.554100] kmemleak: Automatic memory scanning thread ended
> 
> Signed-off-by: Kuan-Ying Lee <Kuan-Ying.Lee@mediatek.com>

I was wondering whether we should just give up the tag when storing the
object->pointer and avoid any later untagging when searching the rb
tree. But, if we want to keep that for debugging, fine by me as well.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

-- 
Catalin

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

end of thread, other threads:[~2021-12-02 18:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18  5:44 [PATCH] kmemleak: fix kmemleak false positive report with HW tag-based kasan enable Kuan-Ying Lee
2021-11-18  9:20 ` Kuan-Ying Lee
2021-11-19 14:15   ` Andrey Konovalov
2021-11-19 15:12     ` Kuan-Ying Lee
2021-11-19 22:43       ` Andrew Morton
2021-11-24  2:00         ` Kuan-Ying Lee
2021-11-25 16:13   ` Andrey Konovalov
2021-11-28  0:20     ` Andrew Morton
2021-12-02 18:19 ` Catalin Marinas

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