[PATCH 1/2] tracing: Fix kmemleak in tracing_map_array_free

[PATCH 1/2] tracing: Fix kmemleak in tracing_map_array_free

[Chunyu Hu]

kmemleak reported the below leak when I was doing clear of the hist
trigger. With this patch, the kmeamleak is gone.

unreferenced object 0xffff94322b63d760 (size 32):
  comm "bash", pid 1522, jiffies 4403687962 (age 2442.311s)
  hex dump (first 32 bytes):
    00 01 00 00 04 00 00 00 08 00 00 00 ff 00 00 00  ................
    10 00 00 00 00 00 00 00 80 a8 7a f2 31 94 ff ff  ..........z.1...
  backtrace:
    [<ffffffff9e96c27a>] kmemleak_alloc+0x4a/0xa0
    [<ffffffff9e424cba>] kmem_cache_alloc_trace+0xca/0x1d0
    [<ffffffff9e377736>] tracing_map_array_alloc+0x26/0x140
    [<ffffffff9e261be0>] kretprobe_trampoline+0x0/0x50
    [<ffffffff9e38b935>] create_hist_data+0x535/0x750
    [<ffffffff9e38bd47>] event_hist_trigger_func+0x1f7/0x420
    [<ffffffff9e38893d>] event_trigger_write+0xfd/0x1a0
    [<ffffffff9e44dfc7>] __vfs_write+0x37/0x170
    [<ffffffff9e44f552>] vfs_write+0xb2/0x1b0
    [<ffffffff9e450b85>] SyS_write+0x55/0xc0
    [<ffffffff9e203857>] do_syscall_64+0x67/0x150
    [<ffffffff9e977ce7>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff9431f27aa880 (size 128):
  comm "bash", pid 1522, jiffies 4403687962 (age 2442.311s)
  hex dump (first 32 bytes):
    00 00 8c 2a 32 94 ff ff 00 f0 8b 2a 32 94 ff ff  ...*2......*2...
    00 e0 8b 2a 32 94 ff ff 00 d0 8b 2a 32 94 ff ff  ...*2......*2...
  backtrace:
    [<ffffffff9e96c27a>] kmemleak_alloc+0x4a/0xa0
    [<ffffffff9e425348>] __kmalloc+0xe8/0x220
    [<ffffffff9e3777c1>] tracing_map_array_alloc+0xb1/0x140
    [<ffffffff9e261be0>] kretprobe_trampoline+0x0/0x50
    [<ffffffff9e38b935>] create_hist_data+0x535/0x750
    [<ffffffff9e38bd47>] event_hist_trigger_func+0x1f7/0x420
    [<ffffffff9e38893d>] event_trigger_write+0xfd/0x1a0
    [<ffffffff9e44dfc7>] __vfs_write+0x37/0x170
    [<ffffffff9e44f552>] vfs_write+0xb2/0x1b0
    [<ffffffff9e450b85>] SyS_write+0x55/0xc0
    [<ffffffff9e203857>] do_syscall_64+0x67/0x150
    [<ffffffff9e977ce7>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff

Signed-off-by: Chunyu Hu <chuhu@redhat.com>
---
 kernel/trace/tracing_map.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
index 0a689bb..305039b 100644
--- a/kernel/trace/tracing_map.c
+++ b/kernel/trace/tracing_map.c
@@ -221,16 +221,19 @@ void tracing_map_array_free(struct tracing_map_array *a)
 	if (!a)
 		return;
 
-	if (!a->pages) {
-		kfree(a);
-		return;
-	}
+	if (!a->pages)
+		goto free;
 
 	for (i = 0; i < a->n_pages; i++) {
 		if (!a->pages[i])
 			break;
 		free_page((unsigned long)a->pages[i]);
 	}
+
+	kfree(a->pages);
+
+ free:
+	kfree(a);
 }
 
 struct tracing_map_array *tracing_map_array_alloc(unsigned int n_elts,
-- 
1.8.3.1

[PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Chunyu Hu]

Found this kmemleak in error path when setting event trigger
filter. The steps is as below.

cd /sys/kernel/debug/tracing/events/irq/irq_handler_entry
echo 'enable_event:kmem:kmalloc:3 if nr_rq > 1' > trigger
echo 'enable_event:kmem:kmalloc:3 if irq > 31' > trigger
echo 'enable_event:kmem:kmalloc:3 if irq > ' > trigger

unreferenced object 0xffffa06e570186a0 (size 32):
  comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s)
  hex dump (first 32 bytes):
    00 00 00 00 01 00 00 00 00 a6 86 69 6e a0 ff ff  ...........in...
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
    [<ffffffff97024e0a>] kmem_cache_alloc_trace+0xca/0x1d0
    [<ffffffff96f86552>] create_filter_start.constprop.28+0x42/0x730
    [<ffffffff96f87a0a>] create_filter+0x4a/0xc0
    [<ffffffff96f87bac>] create_event_filter+0xc/0x10
    [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
    [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
    [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
    [<ffffffff9704e117>] __vfs_write+0x37/0x170
    [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
    [<ffffffff97050cd5>] SyS_write+0x55/0xc0
    [<ffffffff96e03857>] do_syscall_64+0x67/0x150
    [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffffa06e6986a600 (size 512):
  comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s)
  hex dump (first 32 bytes):
    b0 59 f8 96 ff ff ff ff 00 00 00 00 00 00 00 00  .Y..............
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
    [<ffffffff97025498>] __kmalloc+0xe8/0x220
    [<ffffffff96f87518>] replace_preds+0x4c8/0x970
    [<ffffffff96f87a51>] create_filter+0x91/0xc0
    [<ffffffff96f87bac>] create_event_filter+0xc/0x10
    [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
    [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
    [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
    [<ffffffff9704e117>] __vfs_write+0x37/0x170
    [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
    [<ffffffff97050cd5>] SyS_write+0x55/0xc0
    [<ffffffff96e03857>] do_syscall_64+0x67/0x150
    [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffffa06e718a7560 (size 32):
  comm "bash", pid 2521, jiffies 4335807465 (age 43861.320s)
  hex dump (first 32 bytes):
    00 00 00 00 01 00 00 00 00 a2 76 5b 6e a0 ff ff  ..........v[n...
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
    [<ffffffff97024e0a>] kmem_cache_alloc_trace+0xca/0x1d0
    [<ffffffff96f86552>] create_filter_start.constprop.28+0x42/0x730
    [<ffffffff96f87a0a>] create_filter+0x4a/0xc0
    [<ffffffff96f87bac>] create_event_filter+0xc/0x10
    [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
    [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
    [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
    [<ffffffff9704e117>] __vfs_write+0x37/0x170
    [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
    [<ffffffff97050cd5>] SyS_write+0x55/0xc0
    [<ffffffff96e03857>] do_syscall_64+0x67/0x150
    [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
    [<ffffffffffffffff>] 0xffffffffffffffff

Signed-off-by: Chunyu Hu <chuhu@redhat.com>
---
 kernel/trace/trace_events_trigger.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index f2ac9d4..955c3eb 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -739,7 +739,7 @@ int set_trigger_filter(char *filter_str,
 	/* The filter is for the 'trigger' event, not the triggered event */
 	ret = create_event_filter(file->event_call, filter_str, false, &filter);
 	if (ret)
-		goto out;
+		goto out_free;
  assign:
 	tmp = rcu_access_pointer(data->filter);
 
@@ -764,6 +764,10 @@ int set_trigger_filter(char *filter_str,
 	}
  out:
 	return ret;
+
+ out_free:
+	free_event_filter(filter);
+	goto out;
 }
 
 static LIST_HEAD(named_triggers);
-- 
1.8.3.1

Re: [PATCH 1/2] tracing: Fix kmemleak in tracing_map_array_free

[Chunyu Hu]

Resend to add linux-kernel cced. The steps I was using was:

echo hist:keys=irq > trigger
echo '!hist:keys=irq:vals=hitcount:sort=hitcount:size=2048' >  trigger

Then scan the kmem.
echo scan > /sys/kernel/debug/kmemleak

On 14 August 2017 at 18:18, Chunyu Hu <chuhu@redhat.com> wrote:
> kmemleak reported the below leak when I was doing clear of the hist
> trigger. With this patch, the kmeamleak is gone.
>
> unreferenced object 0xffff94322b63d760 (size 32):
>   comm "bash", pid 1522, jiffies 4403687962 (age 2442.311s)
>   hex dump (first 32 bytes):
>     00 01 00 00 04 00 00 00 08 00 00 00 ff 00 00 00  ................
>     10 00 00 00 00 00 00 00 80 a8 7a f2 31 94 ff ff  ..........z.1...
>   backtrace:
>     [<ffffffff9e96c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff9e424cba>] kmem_cache_alloc_trace+0xca/0x1d0
>     [<ffffffff9e377736>] tracing_map_array_alloc+0x26/0x140
>     [<ffffffff9e261be0>] kretprobe_trampoline+0x0/0x50
>     [<ffffffff9e38b935>] create_hist_data+0x535/0x750
>     [<ffffffff9e38bd47>] event_hist_trigger_func+0x1f7/0x420
>     [<ffffffff9e38893d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9e44dfc7>] __vfs_write+0x37/0x170
>     [<ffffffff9e44f552>] vfs_write+0xb2/0x1b0
>     [<ffffffff9e450b85>] SyS_write+0x55/0xc0
>     [<ffffffff9e203857>] do_syscall_64+0x67/0x150
>     [<ffffffff9e977ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffff9431f27aa880 (size 128):
>   comm "bash", pid 1522, jiffies 4403687962 (age 2442.311s)
>   hex dump (first 32 bytes):
>     00 00 8c 2a 32 94 ff ff 00 f0 8b 2a 32 94 ff ff  ...*2......*2...
>     00 e0 8b 2a 32 94 ff ff 00 d0 8b 2a 32 94 ff ff  ...*2......*2...
>   backtrace:
>     [<ffffffff9e96c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff9e425348>] __kmalloc+0xe8/0x220
>     [<ffffffff9e3777c1>] tracing_map_array_alloc+0xb1/0x140
>     [<ffffffff9e261be0>] kretprobe_trampoline+0x0/0x50
>     [<ffffffff9e38b935>] create_hist_data+0x535/0x750
>     [<ffffffff9e38bd47>] event_hist_trigger_func+0x1f7/0x420
>     [<ffffffff9e38893d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9e44dfc7>] __vfs_write+0x37/0x170
>     [<ffffffff9e44f552>] vfs_write+0xb2/0x1b0
>     [<ffffffff9e450b85>] SyS_write+0x55/0xc0
>     [<ffffffff9e203857>] do_syscall_64+0x67/0x150
>     [<ffffffff9e977ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
>
> Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> ---
>  kernel/trace/tracing_map.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
> index 0a689bb..305039b 100644
> --- a/kernel/trace/tracing_map.c
> +++ b/kernel/trace/tracing_map.c
> @@ -221,16 +221,19 @@ void tracing_map_array_free(struct tracing_map_array *a)
>         if (!a)
>                 return;
>
> -       if (!a->pages) {
> -               kfree(a);
> -               return;
> -       }
> +       if (!a->pages)
> +               goto free;
>
>         for (i = 0; i < a->n_pages; i++) {
>                 if (!a->pages[i])
>                         break;
>                 free_page((unsigned long)a->pages[i]);
>         }
> +
> +       kfree(a->pages);
> +
> + free:
> +       kfree(a);
>  }
>
>  struct tracing_map_array *tracing_map_array_alloc(unsigned int n_elts,
> --
> 1.8.3.1
>

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Chunyu Hu]

Resend this to add linux-kernel cced.

On 14 August 2017 at 18:18, Chunyu Hu <chuhu@redhat.com> wrote:
> Found this kmemleak in error path when setting event trigger
> filter. The steps is as below.
>
> cd /sys/kernel/debug/tracing/events/irq/irq_handler_entry
> echo 'enable_event:kmem:kmalloc:3 if nr_rq > 1' > trigger
> echo 'enable_event:kmem:kmalloc:3 if irq > 31' > trigger
> echo 'enable_event:kmem:kmalloc:3 if irq > ' > trigger
>
> unreferenced object 0xffffa06e570186a0 (size 32):
>   comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s)
>   hex dump (first 32 bytes):
>     00 00 00 00 01 00 00 00 00 a6 86 69 6e a0 ff ff  ...........in...
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff97024e0a>] kmem_cache_alloc_trace+0xca/0x1d0
>     [<ffffffff96f86552>] create_filter_start.constprop.28+0x42/0x730
>     [<ffffffff96f87a0a>] create_filter+0x4a/0xc0
>     [<ffffffff96f87bac>] create_event_filter+0xc/0x10
>     [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
>     [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
>     [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9704e117>] __vfs_write+0x37/0x170
>     [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
>     [<ffffffff97050cd5>] SyS_write+0x55/0xc0
>     [<ffffffff96e03857>] do_syscall_64+0x67/0x150
>     [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffffa06e6986a600 (size 512):
>   comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s)
>   hex dump (first 32 bytes):
>     b0 59 f8 96 ff ff ff ff 00 00 00 00 00 00 00 00  .Y..............
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff97025498>] __kmalloc+0xe8/0x220
>     [<ffffffff96f87518>] replace_preds+0x4c8/0x970
>     [<ffffffff96f87a51>] create_filter+0x91/0xc0
>     [<ffffffff96f87bac>] create_event_filter+0xc/0x10
>     [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
>     [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
>     [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9704e117>] __vfs_write+0x37/0x170
>     [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
>     [<ffffffff97050cd5>] SyS_write+0x55/0xc0
>     [<ffffffff96e03857>] do_syscall_64+0x67/0x150
>     [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffffa06e718a7560 (size 32):
>   comm "bash", pid 2521, jiffies 4335807465 (age 43861.320s)
>   hex dump (first 32 bytes):
>     00 00 00 00 01 00 00 00 00 a2 76 5b 6e a0 ff ff  ..........v[n...
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff97024e0a>] kmem_cache_alloc_trace+0xca/0x1d0
>     [<ffffffff96f86552>] create_filter_start.constprop.28+0x42/0x730
>     [<ffffffff96f87a0a>] create_filter+0x4a/0xc0
>     [<ffffffff96f87bac>] create_event_filter+0xc/0x10
>     [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
>     [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
>     [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9704e117>] __vfs_write+0x37/0x170
>     [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
>     [<ffffffff97050cd5>] SyS_write+0x55/0xc0
>     [<ffffffff96e03857>] do_syscall_64+0x67/0x150
>     [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
>
> Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> ---
>  kernel/trace/trace_events_trigger.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index f2ac9d4..955c3eb 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -739,7 +739,7 @@ int set_trigger_filter(char *filter_str,
>         /* The filter is for the 'trigger' event, not the triggered event */
>         ret = create_event_filter(file->event_call, filter_str, false, &filter);
>         if (ret)
> -               goto out;
> +               goto out_free;
>   assign:
>         tmp = rcu_access_pointer(data->filter);
>
> @@ -764,6 +764,10 @@ int set_trigger_filter(char *filter_str,
>         }
>   out:
>         return ret;
> +
> + out_free:
> +       free_event_filter(filter);
> +       goto out;
>  }
>
>  static LIST_HEAD(named_triggers);
> --
> 1.8.3.1
>

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Steven Rostedt]

On Mon, 14 Aug 2017 18:18:18 +0800
Chunyu Hu <chuhu@redhat.com> wrote:

> Found this kmemleak in error path when setting event trigger
> filter. The steps is as below.
> 
> cd /sys/kernel/debug/tracing/events/irq/irq_handler_entry
> echo 'enable_event:kmem:kmalloc:3 if nr_rq > 1' > trigger
> echo 'enable_event:kmem:kmalloc:3 if irq > 31' > trigger
> echo 'enable_event:kmem:kmalloc:3 if irq > ' > trigger
> 
> unreferenced object 0xffffa06e570186a0 (size 32):
>   comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s)
>   hex dump (first 32 bytes):
>     00 00 00 00 01 00 00 00 00 a6 86 69 6e a0 ff ff  ...........in...
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff97024e0a>] kmem_cache_alloc_trace+0xca/0x1d0
>     [<ffffffff96f86552>] create_filter_start.constprop.28+0x42/0x730
>     [<ffffffff96f87a0a>] create_filter+0x4a/0xc0
>     [<ffffffff96f87bac>] create_event_filter+0xc/0x10
>     [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
>     [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
>     [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9704e117>] __vfs_write+0x37/0x170
>     [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
>     [<ffffffff97050cd5>] SyS_write+0x55/0xc0
>     [<ffffffff96e03857>] do_syscall_64+0x67/0x150
>     [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffffa06e6986a600 (size 512):
>   comm "bash", pid 2521, jiffies 4335796661 (age 43872.095s)
>   hex dump (first 32 bytes):
>     b0 59 f8 96 ff ff ff ff 00 00 00 00 00 00 00 00  .Y..............
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff97025498>] __kmalloc+0xe8/0x220
>     [<ffffffff96f87518>] replace_preds+0x4c8/0x970
>     [<ffffffff96f87a51>] create_filter+0x91/0xc0
>     [<ffffffff96f87bac>] create_event_filter+0xc/0x10
>     [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
>     [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
>     [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9704e117>] __vfs_write+0x37/0x170
>     [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
>     [<ffffffff97050cd5>] SyS_write+0x55/0xc0
>     [<ffffffff96e03857>] do_syscall_64+0x67/0x150
>     [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffffa06e718a7560 (size 32):
>   comm "bash", pid 2521, jiffies 4335807465 (age 43861.320s)
>   hex dump (first 32 bytes):
>     00 00 00 00 01 00 00 00 00 a2 76 5b 6e a0 ff ff  ..........v[n...
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff9756c27a>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff97024e0a>] kmem_cache_alloc_trace+0xca/0x1d0
>     [<ffffffff96f86552>] create_filter_start.constprop.28+0x42/0x730
>     [<ffffffff96f87a0a>] create_filter+0x4a/0xc0
>     [<ffffffff96f87bac>] create_event_filter+0xc/0x10
>     [<ffffffff96f88b24>] set_trigger_filter+0x84/0x130
>     [<ffffffff96f8934f>] event_enable_trigger_func+0x25f/0x340
>     [<ffffffff96f8894d>] event_trigger_write+0xfd/0x1a0
>     [<ffffffff9704e117>] __vfs_write+0x37/0x170
>     [<ffffffff9704f6a2>] vfs_write+0xb2/0x1b0
>     [<ffffffff97050cd5>] SyS_write+0x55/0xc0
>     [<ffffffff96e03857>] do_syscall_64+0x67/0x150
>     [<ffffffff97577ce7>] return_from_SYSCALL_64+0x0/0x6a
>     [<ffffffffffffffff>] 0xffffffffffffffff
> 
> Signed-off-by: Chunyu Hu <chuhu@redhat.com>
> ---
>  kernel/trace/trace_events_trigger.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index f2ac9d4..955c3eb 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -739,7 +739,7 @@ int set_trigger_filter(char *filter_str,
>  	/* The filter is for the 'trigger' event, not the triggered event */
>  	ret = create_event_filter(file->event_call, filter_str, false, &filter);

The filter is allocated by create_event_filter. If that returns a
failure, then that should be the one to free it. It is bad taste to
require the calling function to require it.

-- Steve

>  	if (ret)
> -		goto out;
> +		goto out_free;
>   assign:
>  	tmp = rcu_access_pointer(data->filter);
>  
> @@ -764,6 +764,10 @@ int set_trigger_filter(char *filter_str,
>  	}
>   out:
>  	return ret;
> +
> + out_free:
> +	free_event_filter(filter);
> +	goto out;
>  }
>  
>  static LIST_HEAD(named_triggers);

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Steven Rostedt]

On Wed, 23 Aug 2017 10:38:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> > +++ b/kernel/trace/trace_events_trigger.c
> > @@ -739,7 +739,7 @@ int set_trigger_filter(char *filter_str,
> >  	/* The filter is for the 'trigger' event, not the triggered event */
> >  	ret = create_event_filter(file->event_call, filter_str, false, &filter);  
> 
> The filter is allocated by create_event_filter. If that returns a
> failure, then that should be the one to free it. It is bad taste to
> require the calling function to require it.

I take that back. I just read the comment above create_event_filter():

 * On success, returns 0 and *@filterp points to the new filter.  On
 * failure, returns -errno and *@filterp may point to %NULL or to a new
 * filter.  In the latter case, the returned filter contains error
 * information if @set_str is %true and the caller is responsible for
 * freeing it.

So filter contains an error string when it fails. It seems that we
should somehow propagate that up the chain to display. I'll look more
into this.

-- Steve

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Steven Rostedt]

On Wed, 23 Aug 2017 10:41:55 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

>  * On success, returns 0 and *@filterp points to the new filter.  On
>  * failure, returns -errno and *@filterp may point to %NULL or to a new
>  * filter.  In the latter case, the returned filter contains error
>  * information if @set_str is %true and the caller is responsible for
>  * freeing it.
> 
> So filter contains an error string when it fails. It seems that we
> should somehow propagate that up the chain to display. I'll look more
> into this.

The bug is in create_filter(), because "set_str" is set to false, and
the filter should not be passed back allocated on error.

The correct fix is below.

Thanks!

-- Steve


>From 9975be0b2dccaea8ec3574d69a3504e11659a6ea Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Date: Wed, 23 Aug 2017 12:46:27 -0400
Subject: [PATCH] tracing: Fix freeing of filter in create_filter() when
 set_str is false

Performing the following task with kmemleak enabled:

 # cd /sys/kernel/tracing/events/irq/irq_handler_entry/
 # echo 'enable_event:kmem:kmalloc:3 if irq >' > trigger
 # echo 'enable_event:kmem:kmalloc:3 if irq > 31' > trigger
 # echo scan > /sys/kernel/debug/kmemleak
 # cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff8800b9290308 (size 32):
  comm "bash", pid 1114, jiffies 4294848451 (age 141.139s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff81cef5aa>] kmemleak_alloc+0x4a/0xa0
    [<ffffffff81357938>] kmem_cache_alloc_trace+0x158/0x290
    [<ffffffff81261c09>] create_filter_start.constprop.28+0x99/0x940
    [<ffffffff812639c9>] create_filter+0xa9/0x160
    [<ffffffff81263bdc>] create_event_filter+0xc/0x10
    [<ffffffff812655e5>] set_trigger_filter+0xe5/0x210
    [<ffffffff812660c4>] event_enable_trigger_func+0x324/0x490
    [<ffffffff812652e2>] event_trigger_write+0x1a2/0x260
    [<ffffffff8138cf87>] __vfs_write+0xd7/0x380
    [<ffffffff8138f421>] vfs_write+0x101/0x260
    [<ffffffff8139187b>] SyS_write+0xab/0x130
    [<ffffffff81cfd501>] entry_SYSCALL_64_fastpath+0x1f/0xbe
    [<ffffffffffffffff>] 0xffffffffffffffff

The function create_filter() is passed a 'filterp' pointer that gets
allocated, and if "set_str" is true, it is up to the caller to free it, even
on error. The problem is that the pointer is not freed by create_filter()
when set_str is false. This is a bug, and it is not up to the caller to free
the filter on error if it doesn't care about the string.

Link: http://lkml.kernel.org/r/1502705898-27571-2-git-send-email-chuhu@redhat.com

Reported-by: Chunyu Hu <chuhu@redhat.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_filter.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 59a411f..181e139 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1959,6 +1959,10 @@ static int create_filter(struct trace_event_call *call,
 		if (err && set_str)
 			append_filter_err(ps, filter);
 	}
+	if (err && !set_str) {
+		free_event_filter(filter);
+		filter = NULL;
+	}
 	create_filter_finish(ps);
 
 	*filterp = filter;
-- 
2.9.5

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Chunyu Hu]

----- Original Message -----
> From: "Steven Rostedt" <rostedt@goodmis.org>
> To: "Chunyu Hu" <chuhu@redhat.com>
> Cc: mingo@kernel.org, linux-kernel@vger.kernel.org
> Sent: Wednesday, August 23, 2017 12:52:49 PM
> Subject: Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter
> 
> On Wed, 23 Aug 2017 10:41:55 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> >  * On success, returns 0 and *@filterp points to the new filter.  On
> >  * failure, returns -errno and *@filterp may point to %NULL or to a new
> >  * filter.  In the latter case, the returned filter contains error
> >  * information if @set_str is %true and the caller is responsible for
> >  * freeing it.
> > 
> > So filter contains an error string when it fails. It seems that we
> > should somehow propagate that up the chain to display. I'll look more
> > into this.
> 
> The bug is in create_filter(), because "set_str" is set to false, and
> the filter should not be passed back allocated on error.

Thanks for all the analysis. I think you are right. I'll try to have a test on it
in case we miss something. But please don't block on my test.

> 
> The correct fix is below.
> 
> Thanks!
> 
> -- Steve
> 
> 
> From 9975be0b2dccaea8ec3574d69a3504e11659a6ea Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> Date: Wed, 23 Aug 2017 12:46:27 -0400
> Subject: [PATCH] tracing: Fix freeing of filter in create_filter() when
>  set_str is false
> 
> Performing the following task with kmemleak enabled:
> 
>  # cd /sys/kernel/tracing/events/irq/irq_handler_entry/
>  # echo 'enable_event:kmem:kmalloc:3 if irq >' > trigger
>  # echo 'enable_event:kmem:kmalloc:3 if irq > 31' > trigger
>  # echo scan > /sys/kernel/debug/kmemleak
>  # cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffff8800b9290308 (size 32):
>   comm "bash", pid 1114, jiffies 4294848451 (age 141.139s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffff81cef5aa>] kmemleak_alloc+0x4a/0xa0
>     [<ffffffff81357938>] kmem_cache_alloc_trace+0x158/0x290
>     [<ffffffff81261c09>] create_filter_start.constprop.28+0x99/0x940
>     [<ffffffff812639c9>] create_filter+0xa9/0x160
>     [<ffffffff81263bdc>] create_event_filter+0xc/0x10
>     [<ffffffff812655e5>] set_trigger_filter+0xe5/0x210
>     [<ffffffff812660c4>] event_enable_trigger_func+0x324/0x490
>     [<ffffffff812652e2>] event_trigger_write+0x1a2/0x260
>     [<ffffffff8138cf87>] __vfs_write+0xd7/0x380
>     [<ffffffff8138f421>] vfs_write+0x101/0x260
>     [<ffffffff8139187b>] SyS_write+0xab/0x130
>     [<ffffffff81cfd501>] entry_SYSCALL_64_fastpath+0x1f/0xbe
>     [<ffffffffffffffff>] 0xffffffffffffffff
> 
> The function create_filter() is passed a 'filterp' pointer that gets
> allocated, and if "set_str" is true, it is up to the caller to free it, even
> on error. The problem is that the pointer is not freed by create_filter()
> when set_str is false. This is a bug, and it is not up to the caller to free
> the filter on error if it doesn't care about the string.
> 
> Link:
> http://lkml.kernel.org/r/1502705898-27571-2-git-send-email-chuhu@redhat.com
> 
> Reported-by: Chunyu Hu <chuhu@redhat.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_events_filter.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/trace/trace_events_filter.c
> b/kernel/trace/trace_events_filter.c
> index 59a411f..181e139 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1959,6 +1959,10 @@ static int create_filter(struct trace_event_call
> *call,
>  		if (err && set_str)
>  			append_filter_err(ps, filter);
>  	}
> +	if (err && !set_str) {
> +		free_event_filter(filter);
> +		filter = NULL;
> +	}
>  	create_filter_finish(ps);
>  
>  	*filterp = filter;
> --
> 2.9.5
> 
> 

-- 
Regards,
Chunyu Hu

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Steven Rostedt]

On Wed, 23 Aug 2017 18:58:03 -0400 (EDT)
Chunyu Hu <chuhu@redhat.com> wrote:

> ----- Original Message -----
> > From: "Steven Rostedt" <rostedt@goodmis.org>
> > To: "Chunyu Hu" <chuhu@redhat.com>
> > Cc: mingo@kernel.org, linux-kernel@vger.kernel.org
> > Sent: Wednesday, August 23, 2017 12:52:49 PM
> > Subject: Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter
> > 
> > On Wed, 23 Aug 2017 10:41:55 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >   
> > >  * On success, returns 0 and *@filterp points to the new filter.  On
> > >  * failure, returns -errno and *@filterp may point to %NULL or to a new
> > >  * filter.  In the latter case, the returned filter contains error
> > >  * information if @set_str is %true and the caller is responsible for
> > >  * freeing it.
> > > 
> > > So filter contains an error string when it fails. It seems that we
> > > should somehow propagate that up the chain to display. I'll look more
> > > into this.  
> > 
> > The bug is in create_filter(), because "set_str" is set to false, and
> > the filter should not be passed back allocated on error.  
> 
> Thanks for all the analysis. I think you are right. I'll try to have a test on it
> in case we miss something. But please don't block on my test.
> 

My tests are almost done, but I wont send anything till tomorrow. I can
wait a day to post. There's a few other changes I need to send to Linus
as well.

-- Steve

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Chunyu Hu]

----- Original Message -----
> From: "Steven Rostedt" <rostedt@goodmis.org>
> To: "Chunyu Hu" <chuhu@redhat.com>
> Cc: mingo@kernel.org, linux-kernel@vger.kernel.org
> Sent: Thursday, August 24, 2017 10:15:41 AM
> Subject: Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter
> 
> On Wed, 23 Aug 2017 18:58:03 -0400 (EDT)
> Chunyu Hu <chuhu@redhat.com> wrote:
> 
> > ----- Original Message -----
> > > From: "Steven Rostedt" <rostedt@goodmis.org>
> > > To: "Chunyu Hu" <chuhu@redhat.com>
> > > Cc: mingo@kernel.org, linux-kernel@vger.kernel.org
> > > Sent: Wednesday, August 23, 2017 12:52:49 PM
> > > Subject: Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter
> > > 
> > > On Wed, 23 Aug 2017 10:41:55 -0400
> > > Steven Rostedt <rostedt@goodmis.org> wrote:
> > >   
> > > >  * On success, returns 0 and *@filterp points to the new filter.  On
> > > >  * failure, returns -errno and *@filterp may point to %NULL or to a new
> > > >  * filter.  In the latter case, the returned filter contains error
> > > >  * information if @set_str is %true and the caller is responsible for
> > > >  * freeing it.
> > > > 
> > > > So filter contains an error string when it fails. It seems that we
> > > > should somehow propagate that up the chain to display. I'll look more
> > > > into this.
> > > 
> > > The bug is in create_filter(), because "set_str" is set to false, and
> > > the filter should not be passed back allocated on error.
> > 
> > Thanks for all the analysis. I think you are right. I'll try to have a test
> > on it
> > in case we miss something. But please don't block on my test.
> > 
> 
> My tests are almost done, but I wont send anything till tomorrow. I can
> wait a day to post. There's a few other changes I need to send to Linus
> as well.

Tested with your patch, I did not hit the leak issue and other kmemleak. 
Thanks.

> 
> -- Steve
> 

-- 
Regards,
Chunyu Hu

Re: [PATCH 2/2] tracing: Fix kmemleak in set_trigger_filter

[Steven Rostedt]

On Thu, 24 Aug 2017 00:24:50 -0400 (EDT)
Chunyu Hu <chuhu@redhat.com> wrote:

> Tested with your patch, I did not hit the leak issue and other kmemleak. 
> Thanks.

I'll also add a "Tested-by" from you then.

Thanks!

-- Steve