Re: [PATCH v3 13/21] trace-cmd library: Fix possible memory leak in read_and_parse_cmdlines()

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 14 Sep 2021 16:12:24 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Some error paths in read_and_parse_cmdlines() may lead to a memory leak.
> Improved the error handling of this internal function to avoid it.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-input.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 60d75d47..9fd7e6e8 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -2999,20 +2999,26 @@ static int read_and_parse_cmdlines(struct tracecmd_input *handle)
>  {
>  	struct tep_handle *pevent = handle->pevent;
>  	unsigned long long size;
> -	char *cmdlines;
> +	char *cmdlines = NULL;
> +	int ret;
>  
>  	if (handle->file_state >= TRACECMD_FILE_CMD_LINES)
>  		return 0;
>  
> -	if (read_data_and_size(handle, &cmdlines, &size) < 0)
> -		return -1;
> +	ret = read_data_and_size(handle, &cmdlines, &size);

This function only returns an allocated cmdline on success. That was the
point of this function, was that you didn't have to check and free the
return value.

-- Steve


> +	if (ret < 0)
> +		goto out;
> +	if (!size) {
> +		handle->file_state = TRACECMD_FILE_CMD_LINES;
> +		goto out;
> +	}
>  	cmdlines[size] = 0;
>  	tep_parse_saved_cmdlines(pevent, cmdlines);
> -	free(cmdlines);
> -
>  	handle->file_state = TRACECMD_FILE_CMD_LINES;
> -
> -	return 0;
> +	ret = 0;
> +out:
> +	free(cmdlines);
> +	return ret;
>  }
>  
>  static void extract_trace_clock(struct tracecmd_input *handle, char *line)