linux-kernel - Re: [PATCH] tools lib traceevent: Fix double free corruption in error processing code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 17 Jul 2015 12:17:59 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Wang Nan <wangnan0@...wei.com>
Cc:	<hekuang@...wei.com>, <ast@...mgrid.com>, <acme@...nel.org>,
	<a.p.zijlstra@...llo.nl>, <mingo@...hat.com>,
	<namhyung@...nel.org>, <jolsa@...nel.org>, <pi3orama@....com>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tools lib traceevent: Fix double free corruption in
 error processing code

On Fri, 17 Jul 2015 07:58:46 +0000
Wang Nan <wangnan0@...wei.com> 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)" <rostedt@...dmis.org>
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 <rostedt@...dmis.org>
---
 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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/