On Wed, 5 Jun 2024 15:40:21 +0200
"Jerome Marchand" <jmarchan@xxxxxxxxxx> wrote:
> Free buf in the error path.
>
> Fixes a RESOURCE_LEAK error (CWE-772)
>
> Signed-off-by: Jerome Marchand <jmarchan@xxxxxxxxxx>
> ---
> lib/trace-cmd/trace-input.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index ce4ecf43..2cf0d1c1 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -4030,7 +4030,7 @@ static int handle_options(struct tracecmd_input *handle)
> }
> ret = do_read_check(handle, buf, size);
> if (ret)
> - goto out;
> + goto out_free;
>
> switch (option) {
> case TRACECMD_OPTION_DATE:
> @@ -4084,7 +4084,7 @@ static int handle_options(struct tracecmd_input *handle)
> buf + 8, 4);
> ret = tsync_cpu_offsets_load(handle, buf + 12, size - 12);
> if (ret < 0)
> - goto out;
> + goto out_free;
> tracecmd_enable_tsync(handle, true);
> break;
> case TRACECMD_OPTION_CPUSTAT:
> @@ -4093,7 +4093,7 @@ static int handle_options(struct tracecmd_input *handle)
> handle->cpustats_size + size + 1);
> if (!cpustats) {
> ret = -ENOMEM;
> - goto out;
> + goto out_free;
> }
> memcpy(cpustats + handle->cpustats_size, buf, size);
> handle->cpustats_size += size;
> @@ -4104,7 +4104,7 @@ static int handle_options(struct tracecmd_input *handle)
> case TRACECMD_OPTION_BUFFER_TEXT:
> ret = handle_buffer_option(handle, option, buf, size);
> if (ret < 0)
> - goto out;
> + goto out_free;
> break;
> case TRACECMD_OPTION_TRACECLOCK:
> tracecmd_parse_trace_clock(handle, buf, size);
> @@ -4183,6 +4183,8 @@ static int handle_options(struct tracecmd_input *handle)
>
> ret = 0;
>
The for (;;) loop ends with a free(buf) and then in the next iteration it can do:
if (!HAS_SECTIONS(handle) && option == TRACECMD_OPTION_DONE)
break;
> +out_free:
> + free(buf);
Which will cause this to do a double free.
I'm going to not pull this patch.
-- Steve
> out:
> if (compress)
> in_uncompress_reset(handle);
|