On Tue, 14 Sep 2021 16:12:21 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> Some error paths in read_event_files() 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@xxxxxxxxx>
> ---
> lib/trace-cmd/trace-input.c | 39 ++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 94fd8693..32358ce9 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -645,7 +645,7 @@ out:
> static int read_event_files(struct tracecmd_input *handle, const char *regex)
> {
> unsigned long long size;
> - char *system;
> + char *system = NULL;
> regex_t spreg;
> regex_t epreg;
> regex_t *sreg = NULL;
> @@ -670,13 +670,16 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
> return -1;
> }
>
> - if (read4(handle, &systems) < 0)
> - return -1;
> + ret = read4(handle, &systems);
> + if (ret < 0)
> + goto out;
>
> for (i = 0; i < systems; i++) {
> system = read_string(handle);
> - if (!system)
> - return -1;
> + if (!system) {
> + ret = -1;
> + goto out;
> + }
>
> sys_printed = 0;
> print_all = 0;
> @@ -703,39 +706,35 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
> }
> }
>
> - if (read4(handle, &count) < 0)
> - goto failed;
> + ret = read4(handle, &count);
> + if (ret < 0)
> + goto out;
>
> for (x=0; x < count; x++) {
> - if (read8(handle, &size) < 0)
> - goto failed;
> + ret = read8(handle, &size);
> + if (ret < 0)
> + goto out;
>
> ret = read_event_file(handle, system, size,
> print_all, &sys_printed,
> reg);
> if (ret < 0)
> - goto failed;
> + goto out;
> }
> free(system);
> - }
> -
> - if (sreg) {
> - regfree(sreg);
> - regfree(ereg);
> + system = NULL;
> }
Nit, I would have placed "system = NULL;" outside the loop as it gets set
first thing in the loop, but that's just my preference ;-)
Other than that, the patch looks good. No need to update it.
-- Steve
>
> handle->file_state = TRACECMD_FILE_ALL_EVENTS;
> -
> - return 0;
> -
> - failed:
> + ret = 0;
> + out:
> if (sreg) {
> regfree(sreg);
> regfree(ereg);
> }
>
> free(system);
> - return -1;
> + return ret;
> }
>
> static int read_proc_kallsyms(struct tracecmd_input *handle)
|