On 02/28/2014 04:50 AM, Steven Rostedt wrote: > On Thu, 27 Feb 2014 13:18:48 +0800 > Chen Ditang <chendt.fnst@xxxxxxxxxxxxxx> wrote: > >> Reads a wrong trace.dat file, the read_cpu_data() function returns a >> failure, it will cause memory double free. >> >> # ./trace-graph ../../trace.dat >> version = 6 >> File possibly truncated. Need at least 18446744073709551614, but file size is 3564371. >> *** Error in `./trace-graph': double free or corruption (fasttop): 0x000000000262a6e0 *** >> ======= Backtrace: ========= >> /lib64/libc.so.6[0x387b27cef8] >> ./trace-graph(kbuffer_free+0x18)[0x434f78] >> ./trace-graph(tracecmd_close+0xca)[0x432f55] >> ./trace-graph(tracecmd_open_fd+0x5d)[0x432e22] >> ./trace-graph(tracecmd_open+0x3c)[0x432e65] >> ./trace-graph(trace_graph+0x148)[0x40a685] >> ./trace-graph(main+0x20)[0x40adee] >> /lib64/libc.so.6(__libc_start_main+0xf5)[0x387b221b75] >> ./trace-graph[0x40a029] >> >> Signed-off-by: Ditang Chen <chendt.fnst@xxxxxxxxxxxxxx> >> --- >> trace-input.c | 16 ++++------------ >> 1 file changed, 4 insertions(+), 12 deletions(-) >> >> diff --git a/trace-input.c b/trace-input.c >> index 6eef168..8493495 100644 >> --- a/trace-input.c >> +++ b/trace-input.c >> @@ -1841,7 +1841,7 @@ static int read_cpu_data(struct tracecmd_input *handle) >> >> handle->cpu_data[cpu].kbuf = kbuffer_alloc(long_size, endian); >> if (!handle->cpu_data[cpu].kbuf) >> - goto out_free; >> + return -1; >> if (pevent->old_format) >> kbuffer_set_old_format(handle->cpu_data[cpu].kbuf); >> >> @@ -1857,22 +1857,14 @@ static int read_cpu_data(struct tracecmd_input *handle) >> "Need at least %llu, but file size is %zu.\n", >> offset + size, handle->total_file_size); >> errno = EINVAL; >> - goto out_free; >> + return -1; >> } >> >> if (init_cpu(handle, cpu)) >> - goto out_free; >> + return -1; >> } >> >> return 0; >> - >> - out_free: >> - for ( ; cpu >= 0; cpu--) { >> - free_page(handle, cpu); >> - kbuffer_free(handle->cpu_data[cpu].kbuf); > > These frees are still required. But you did uncover a real bug. > Though, the real fix to it is to init kbuf back to NULL: > > kbuffer_free(handle->cpu_data[cpu].kbuf); > handle->cpu_data[cpu].kbuf = NULL; > > Because free() and all the other freeing functions should allow for > NULL to be passed, and it should then be ignored. Thanks,I‘ll send another patch to fix it. > > -- Steve > >> - } >> - return -1; >> - >> } >> >> static int read_data_and_size(struct tracecmd_input *handle, >> @@ -2209,7 +2201,7 @@ void tracecmd_close(struct tracecmd_input *handle) >> /* The tracecmd_peek_data may have cached a record */ >> free_next(handle, cpu); >> free_page(handle, cpu); >> - if (handle->cpu_data) { >> + if (handle->cpu_data && handle->cpu_data[cpu].kbuf) { >> kbuffer_free(handle->cpu_data[cpu].kbuf); >> >> if (!list_empty(&handle->cpu_data[cpu].pages)) > > -- To unsubscribe from this list: send the line "unsubscribe linux-trace-users" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |