When kernel introduces new function to 'format' file before traceevent
update correspondingly, a double-free corruption occures if the newly
intorduced function resides in following format:
# cat /sys/kernel/debug/tracing/events/bpf/bpf_output_data/format
...
print fmt: "%s", __print_hex(__get_dynamic_array(buf), __get_dynamic_array_len(buf))
...
(where __get_dynamic_array_len is the new function)
And:
# perf record -e bpf:bpf_output_data ls
Warning: [bpf:bpf_output_data] function __get_dynamic_array_len not defined
Warning: Error: expected type 5 but read 0
*** Error in `/path/to/perf': double free or corruption (fasttop): 0x0000000001821210 ***
======= Backtrace: =========
/lib64/libc.so.6(+0x6eeef)[0x7f86a4cf6eef]
/lib64/libc.so.6(+0x78cae)[0x7f86a4d00cae]
/lib64/libc.so.6(+0x79987)[0x7f86a4d01987]
/path/to/perf[0x51b0e2]
/path/to/perf[0x51afc2]
/path/to/perf[0x51fe67]
/path/to/perf[0x5200a5]
/path/to/perf(__pevent_parse_format+0x185)[0x525bc9]
/path/to/perf[0x525d82]
/path/to/perf(pevent_parse_format+0x3b)[0x525e15]
/path/to/perf[0x4ceb61]
/path/to/perf(perf_evsel__newtp_idx+0x9f)[0x48cabf]
/path/to/perf(parse_events_add_tracepoint+0x25b)[0x49782b]
/path/to/perf(parse_events_parse+0x10a3)[0x4c8243]
/path/to/perf(parse_events+0x75)[0x498ce5]
/path/to/perf(parse_events_option+0x41)[0x498df1]
/path/to/perf[0x493e9b]
/path/to/perf(parse_options+0x215)[0x4957c5]
/path/to/perf(cmd_record+0x6a)[0x43c4ba]
/path/to/perf[0x47b2a3]
/path/to/perf(main+0x5f6)[0x42fed6]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x7f86a4ca9bd5]
/path/to/perf[0x430005]
======= Memory map: ========
00400000-006b9000 r-xp 00000000 08:05 23596607 /path/to/perf
008b9000-00903000 rw-p 002b9000 08:05 23596607 /path/to/perf
This is caused by error processing code in process_hex() which frees
arg->hex.field but doesn't set it to NULL. When event_read_print_args()
freeing the hex arg, the dangling pointer will be free again and cause
the above error. process_int_array() has similar problem.
This patch fixes the dangling pointer problem by not setting them until
everything is okay.
Signed-off-by: Wang Nan <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: <[email protected]>
---
tools/lib/traceevent/event-parse.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index cc25f05..8d662b5 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2546,19 +2546,23 @@ out_free:
static enum event_type
process_hex(struct event_format *event, struct print_arg *arg, char **tok)
{
+ struct print_arg *field, *size;
+
memset(arg, 0, sizeof(*arg));
arg->type = PRINT_HEX;
- if (alloc_and_process_delim(event, ",", &arg->hex.field))
+ if (alloc_and_process_delim(event, ",", &field))
goto out;
- if (alloc_and_process_delim(event, ")", &arg->hex.size))
+ if (alloc_and_process_delim(event, ")", &size))
goto free_field;
+ arg->hex.field = field;
+ arg->hex.size = size;
return read_token_item(tok);
free_field:
- free_arg(arg->hex.field);
+ free_arg(field);
out:
*tok = NULL;
return EVENT_ERROR;
@@ -2567,24 +2571,29 @@ out:
static enum event_type
process_int_array(struct event_format *event, struct print_arg *arg, char **tok)
{
+ struct print_arg *field, *count, *el_size;
+
memset(arg, 0, sizeof(*arg));
arg->type = PRINT_INT_ARRAY;
- if (alloc_and_process_delim(event, ",", &arg->int_array.field))
+ if (alloc_and_process_delim(event, ",", &field))
goto out;
- if (alloc_and_process_delim(event, ",", &arg->int_array.count))
+ if (alloc_and_process_delim(event, ",", &count))
goto free_field;
- if (alloc_and_process_delim(event, ")", &arg->int_array.el_size))
+ if (alloc_and_process_delim(event, ")", &el_size))
goto free_size;
+ arg->int_array.field = field;
+ arg->int_array.count = count;
+ arg->int_array.el_size = el_size;
return read_token_item(tok);
free_size:
- free_arg(arg->int_array.count);
+ free_arg(count);
free_field:
- free_arg(arg->int_array.field);
+ free_arg(field);
out:
*tok = NULL;
return EVENT_ERROR;
--
1.8.3.4
On Fri, 17 Jul 2015 07:58:46 +0000
Wang Nan <[email protected]> wrote:
> When kernel introduces new function to 'format' file before traceevent
> update correspondingly, a double-free corruption occures if the newly
> intorduced function resides in following format:
>
> # cat /sys/kernel/debug/tracing/events/bpf/bpf_output_data/format
> ...
> print fmt: "%s", __print_hex(__get_dynamic_array(buf), __get_dynamic_array_len(buf))
> ...
>
> (where __get_dynamic_array_len is the new function)
>
> And:
> # perf record -e bpf:bpf_output_data ls
> Warning: [bpf:bpf_output_data] function __get_dynamic_array_len not defined
> Warning: Error: expected type 5 but read 0
> *** Error in `/path/to/perf': double free or corruption (fasttop): 0x0000000001821210 ***
> ======= Backtrace: =========
> /lib64/libc.so.6(+0x6eeef)[0x7f86a4cf6eef]
> /lib64/libc.so.6(+0x78cae)[0x7f86a4d00cae]
> /lib64/libc.so.6(+0x79987)[0x7f86a4d01987]
> /path/to/perf[0x51b0e2]
> /path/to/perf[0x51afc2]
> /path/to/perf[0x51fe67]
> /path/to/perf[0x5200a5]
> /path/to/perf(__pevent_parse_format+0x185)[0x525bc9]
> /path/to/perf[0x525d82]
> /path/to/perf(pevent_parse_format+0x3b)[0x525e15]
> /path/to/perf[0x4ceb61]
> /path/to/perf(perf_evsel__newtp_idx+0x9f)[0x48cabf]
> /path/to/perf(parse_events_add_tracepoint+0x25b)[0x49782b]
> /path/to/perf(parse_events_parse+0x10a3)[0x4c8243]
> /path/to/perf(parse_events+0x75)[0x498ce5]
> /path/to/perf(parse_events_option+0x41)[0x498df1]
> /path/to/perf[0x493e9b]
> /path/to/perf(parse_options+0x215)[0x4957c5]
> /path/to/perf(cmd_record+0x6a)[0x43c4ba]
> /path/to/perf[0x47b2a3]
> /path/to/perf(main+0x5f6)[0x42fed6]
> /lib64/libc.so.6(__libc_start_main+0xf5)[0x7f86a4ca9bd5]
> /path/to/perf[0x430005]
> ======= Memory map: ========
> 00400000-006b9000 r-xp 00000000 08:05 23596607 /path/to/perf
> 008b9000-00903000 rw-p 002b9000 08:05 23596607 /path/to/perf
>
> This is caused by error processing code in process_hex() which frees
> arg->hex.field but doesn't set it to NULL. When event_read_print_args()
> freeing the hex arg, the dangling pointer will be free again and cause
> the above error. process_int_array() has similar problem.
>
> This patch fixes the dangling pointer problem by not setting them until
> everything is okay.
I actually stumbled over this exact bug in trace-cmd but came up with a
much simpler solution:
-- Steve
>From 1d44d0f3361b70724b63c8aab20992704557e9c0 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <[email protected]>
Date: Fri, 17 Jul 2015 12:07:23 -0400
Subject: [PATCH] tools lib traceevent: Set int_array fields to NULL if freeing from error
Had a bug where on error of parsing __print_array() where the
fields are freed after they were allocated, but since they were not
set to NULL, the freeing of the arg also tried to free the already
freed fields causing a double free.
Fix process_hex() while at it.
Signed-off-by: Steven Rostedt <[email protected]>
---
tools/lib/traceevent/event-parse.c | 3 +++
1 file changed, 3 insertions(+)
Index: linux-trace.git/tools/lib/traceevent/event-parse.c
===================================================================
--- linux-trace.git.orig/tools/lib/traceevent/event-parse.c 2015-07-15 14:49:03.275290348 -0400
+++ linux-trace.git/tools/lib/traceevent/event-parse.c 2015-07-17 12:16:54.535359526 -0400
@@ -2559,6 +2559,7 @@ process_hex(struct event_format *event,
free_field:
free_arg(arg->hex.field);
+ arg->hex.field = NULL;
out:
*tok = NULL;
return EVENT_ERROR;
@@ -2583,8 +2584,10 @@ process_int_array(struct event_format *e
free_size:
free_arg(arg->int_array.count);
+ arg->int_array.count = NULL;
free_field:
free_arg(arg->int_array.field);
+ arg->int_array.field = NULL;
out:
*tok = NULL;
return EVENT_ERROR;