On Fri, 28 Feb 2014 11:24:19 +0800
Ditang Chen <chendt.fnst@xxxxxxxxxxxxxx> wrote:
> Reads a wrong trace.dat file, the read_cpu_data() function returns a
> failure, it will cause memory double free. we should init kbuf back to
> NULL after free kbuf.
>
> and if kbuffer_alloc() fail, do not need to free any more.
>
>
> Signed-off-by: Ditang Chen <chendt.fnst@xxxxxxxxxxxxxx>
> ---
> trace-input.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/trace-input.c b/trace-input.c
> index 6eef168..7bebbfe 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);
>
> @@ -1870,6 +1870,7 @@ static int read_cpu_data(struct tracecmd_input *handle)
> for ( ; cpu >= 0; cpu--) {
> free_page(handle, cpu);
> kbuffer_free(handle->cpu_data[cpu].kbuf);
> + handle->cpu_data[cpu].kbuf = NULL;
> }
This is actually the only change needed. The above is in a loop and we
may have allocated pages, so it should still do the goto_free.
> return -1;
>
> @@ -2209,7 +2210,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) {
Hmm, this gives a tight coupling between read_cpu_data() and here, as
it depends on kbuf being the first allocated. But I see that that also
denotes that init_cpu() wasn't called to initialize the .pages list.
OK, keep this change too. That is, please send another patch without
the first hunk.
Thanks,
-- Steve
> 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
|