* [for-linus][PATCH 1/4] tracing: Fix memory leaks in trace_events_hist.c
2020-04-23 23:18 [for-linus][PATCH 0/4] tracing: A few fixes Steven Rostedt
@ 2020-04-23 23:18 ` Steven Rostedt
2020-04-23 23:18 ` [for-linus][PATCH 2/4] ftrace: Fix memory leak caused by not freeing entry in unregister_ftrace_direct() Steven Rostedt
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-23 23:18 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Vamshi K Sthambamkadi
From: Vamshi K Sthambamkadi <vamshi.k.sthambamkadi@gmail.com>
kmemleak report 1:
[<9092c50b>] kmem_cache_alloc_trace+0x138/0x270
[<05a2c9ed>] create_field_var+0xcf/0x180
[<528a2d68>] action_create+0xe2/0xc80
[<63f50b61>] event_hist_trigger_func+0x15b5/0x1920
[<28ea5d3d>] trigger_process_regex+0x7b/0xc0
[<3138e86f>] event_trigger_write+0x4d/0xb0
[<ffd66c19>] __vfs_write+0x30/0x200
[<4f424a0d>] vfs_write+0x96/0x1b0
[<da59a290>] ksys_write+0x53/0xc0
[<3717101a>] __ia32_sys_write+0x15/0x20
[<c5f23497>] do_fast_syscall_32+0x70/0x250
[<46e2629c>] entry_SYSENTER_32+0xaf/0x102
This is because save_vars[] of struct hist_trigger_data are
not destroyed
kmemleak report 2:
[<9092c50b>] kmem_cache_alloc_trace+0x138/0x270
[<6e5e97c5>] create_var+0x3c/0x110
[<de82f1b9>] create_field_var+0xaf/0x180
[<528a2d68>] action_create+0xe2/0xc80
[<63f50b61>] event_hist_trigger_func+0x15b5/0x1920
[<28ea5d3d>] trigger_process_regex+0x7b/0xc0
[<3138e86f>] event_trigger_write+0x4d/0xb0
[<ffd66c19>] __vfs_write+0x30/0x200
[<4f424a0d>] vfs_write+0x96/0x1b0
[<da59a290>] ksys_write+0x53/0xc0
[<3717101a>] __ia32_sys_write+0x15/0x20
[<c5f23497>] do_fast_syscall_32+0x70/0x250
[<46e2629c>] entry_SYSENTER_32+0xaf/0x102
struct hist_field allocated through create_var() do not initialize
"ref" field to 1. The code in __destroy_hist_field() does not destroy
object if "ref" is initialized to zero, the condition
if (--hist_field->ref > 1) always passes since unsigned int wraps.
kmemleak report 3:
[<f8666fcc>] __kmalloc_track_caller+0x139/0x2b0
[<bb7f80a5>] kstrdup+0x27/0x50
[<39d70006>] init_var_ref+0x58/0xd0
[<8ca76370>] create_var_ref+0x89/0xe0
[<f045fc39>] action_create+0x38f/0xc80
[<7c146821>] event_hist_trigger_func+0x15b5/0x1920
[<07de3f61>] trigger_process_regex+0x7b/0xc0
[<e87daf8f>] event_trigger_write+0x4d/0xb0
[<19bf1512>] __vfs_write+0x30/0x200
[<64ce4d27>] vfs_write+0x96/0x1b0
[<a6f34170>] ksys_write+0x53/0xc0
[<7d4230cd>] __ia32_sys_write+0x15/0x20
[<8eadca00>] do_fast_syscall_32+0x70/0x250
[<235cf985>] entry_SYSENTER_32+0xaf/0x102
hist_fields (system & event_name) are not freed
Link: http://lkml.kernel.org/r/20200422061503.GA5151@cosmos
Signed-off-by: Vamshi K Sthambamkadi <vamshi.k.sthambamkadi@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 5f6834a2bf41..fcab11cc6833 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3320,6 +3320,9 @@ static void __destroy_hist_field(struct hist_field *hist_field)
kfree(hist_field->name);
kfree(hist_field->type);
+ kfree(hist_field->system);
+ kfree(hist_field->event_name);
+
kfree(hist_field);
}
@@ -4382,6 +4385,7 @@ static struct hist_field *create_var(struct hist_trigger_data *hist_data,
goto out;
}
+ var->ref = 1;
var->flags = HIST_FIELD_FL_VAR;
var->var.idx = idx;
var->var.hist_data = var->hist_data = hist_data;
@@ -5011,6 +5015,9 @@ static void destroy_field_vars(struct hist_trigger_data *hist_data)
for (i = 0; i < hist_data->n_field_vars; i++)
destroy_field_var(hist_data->field_vars[i]);
+
+ for (i = 0; i < hist_data->n_save_vars; i++)
+ destroy_field_var(hist_data->save_vars[i]);
}
static void save_field_var(struct hist_trigger_data *hist_data,
--
2.26.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [for-linus][PATCH 2/4] ftrace: Fix memory leak caused by not freeing entry in unregister_ftrace_direct()
2020-04-23 23:18 [for-linus][PATCH 0/4] tracing: A few fixes Steven Rostedt
2020-04-23 23:18 ` [for-linus][PATCH 1/4] tracing: Fix memory leaks in trace_events_hist.c Steven Rostedt
@ 2020-04-23 23:18 ` Steven Rostedt
2020-04-23 23:18 ` [for-linus][PATCH 3/4] tracing: Remove DECLARE_TRACE_NOARGS Steven Rostedt
2020-04-23 23:18 ` [for-linus][PATCH 4/4] tracing: Convert local functions in tracing_map.c to static Steven Rostedt
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-23 23:18 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
kmemleak reported the following:
unreferenced object 0xffff90d47127a920 (size 32):
comm "modprobe", pid 1766, jiffies 4294792031 (age 162.568s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 22 01 00 00 00 00 ad de ........".......
00 78 12 a7 ff ff ff ff 00 00 b6 c0 ff ff ff ff .x..............
backtrace:
[<00000000bb79e72e>] register_ftrace_direct+0xcb/0x3a0
[<00000000295e4f79>] do_one_initcall+0x72/0x340
[<00000000873ead18>] do_init_module+0x5a/0x220
[<00000000974d9de5>] load_module+0x2235/0x2550
[<0000000059c3d6ce>] __do_sys_finit_module+0xc0/0x120
[<000000005a8611b4>] do_syscall_64+0x60/0x230
[<00000000a0cdc49e>] entry_SYSCALL_64_after_hwframe+0x49/0xb3
The entry used to save the direct descriptor needs to be freed
when unregistering.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 041694a1eb74..bd030b1b9514 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5165,6 +5165,7 @@ int unregister_ftrace_direct(unsigned long ip, unsigned long addr)
list_del_rcu(&direct->next);
synchronize_rcu_tasks();
kfree(direct);
+ kfree(entry);
ftrace_direct_func_count--;
}
}
--
2.26.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [for-linus][PATCH 3/4] tracing: Remove DECLARE_TRACE_NOARGS
2020-04-23 23:18 [for-linus][PATCH 0/4] tracing: A few fixes Steven Rostedt
2020-04-23 23:18 ` [for-linus][PATCH 1/4] tracing: Fix memory leaks in trace_events_hist.c Steven Rostedt
2020-04-23 23:18 ` [for-linus][PATCH 2/4] ftrace: Fix memory leak caused by not freeing entry in unregister_ftrace_direct() Steven Rostedt
@ 2020-04-23 23:18 ` Steven Rostedt
2020-04-23 23:18 ` [for-linus][PATCH 4/4] tracing: Convert local functions in tracing_map.c to static Steven Rostedt
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-23 23:18 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Nikolay Borisov
From: Nikolay Borisov <nborisov@suse.com>
This macro was intentionally broken so that the kernel code is not
poluted with such noargs macro used simply as markers. This use case
can be satisfied by using dummy no inline functions. Just remove it.
Link: http://lkml.kernel.org/r/20200413153246.8511-1-nborisov@suse.com
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/linux/tracepoint.h | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 1fb11daa5c53..a1fecf311621 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -156,8 +156,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
* Note, the proto and args passed in includes "__data" as the first parameter.
* The reason for this is to handle the "void" prototype. If a tracepoint
* has a "void" prototype, then it is invalid to declare a function
- * as "(void *, void)". The DECLARE_TRACE_NOARGS() will pass in just
- * "void *data", where as the DECLARE_TRACE() will pass in "void *data, proto".
+ * as "(void *, void)".
*/
#define __DO_TRACE(tp, proto, args, cond, rcuidle) \
do { \
@@ -373,25 +372,6 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
# define __tracepoint_string
#endif
-/*
- * The need for the DECLARE_TRACE_NOARGS() is to handle the prototype
- * (void). "void" is a special value in a function prototype and can
- * not be combined with other arguments. Since the DECLARE_TRACE()
- * macro adds a data element at the beginning of the prototype,
- * we need a way to differentiate "(void *data, proto)" from
- * "(void *data, void)". The second prototype is invalid.
- *
- * DECLARE_TRACE_NOARGS() passes "void" as the tracepoint prototype
- * and "void *__data" as the callback prototype.
- *
- * DECLARE_TRACE() passes "proto" as the tracepoint protoype and
- * "void *__data, proto" as the callback prototype.
- */
-#define DECLARE_TRACE_NOARGS(name) \
- __DECLARE_TRACE(name, void, , \
- cpu_online(raw_smp_processor_id()), \
- void *__data, __data)
-
#define DECLARE_TRACE(name, proto, args) \
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
cpu_online(raw_smp_processor_id()), \
--
2.26.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [for-linus][PATCH 4/4] tracing: Convert local functions in tracing_map.c to static
2020-04-23 23:18 [for-linus][PATCH 0/4] tracing: A few fixes Steven Rostedt
` (2 preceding siblings ...)
2020-04-23 23:18 ` [for-linus][PATCH 3/4] tracing: Remove DECLARE_TRACE_NOARGS Steven Rostedt
@ 2020-04-23 23:18 ` Steven Rostedt
3 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-04-23 23:18 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Hulk Robot, Jason Yan
From: Jason Yan <yanaijie@huawei.com>
Fix the following sparse warning:
kernel/trace/tracing_map.c:286:6: warning: symbol
'tracing_map_array_clear' was not declared. Should it be static?
kernel/trace/tracing_map.c:297:6: warning: symbol
'tracing_map_array_free' was not declared. Should it be static?
kernel/trace/tracing_map.c:319:26: warning: symbol
'tracing_map_array_alloc' was not declared. Should it be static?
Link: http://lkml.kernel.org/r/20200410073312.38855-1-yanaijie@huawei.com
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Jason Yan <yanaijie@huawei.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/tracing_map.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
index 9e31bfc818ff..74738c9856f1 100644
--- a/kernel/trace/tracing_map.c
+++ b/kernel/trace/tracing_map.c
@@ -283,7 +283,7 @@ int tracing_map_add_key_field(struct tracing_map *map,
return idx;
}
-void tracing_map_array_clear(struct tracing_map_array *a)
+static void tracing_map_array_clear(struct tracing_map_array *a)
{
unsigned int i;
@@ -294,7 +294,7 @@ void tracing_map_array_clear(struct tracing_map_array *a)
memset(a->pages[i], 0, PAGE_SIZE);
}
-void tracing_map_array_free(struct tracing_map_array *a)
+static void tracing_map_array_free(struct tracing_map_array *a)
{
unsigned int i;
@@ -316,7 +316,7 @@ void tracing_map_array_free(struct tracing_map_array *a)
kfree(a);
}
-struct tracing_map_array *tracing_map_array_alloc(unsigned int n_elts,
+static struct tracing_map_array *tracing_map_array_alloc(unsigned int n_elts,
unsigned int entry_size)
{
struct tracing_map_array *a;
--
2.26.1
^ permalink raw reply related [flat|nested] 5+ messages in thread