diff mbox series

[v2,38/87] trace-cmd library: Fix possible memory leak in read_ftrace_files()

Message ID 20210729050959.12263-39-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series Trace file version 7 | expand

Commit Message

Tzvetomir Stoyanov (VMware) July 29, 2021, 5:09 a.m. UTC
Some error paths in read_ftrace_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@gmail.com>
---
 lib/trace-cmd/trace-input.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Comments

Steven Rostedt Aug. 19, 2021, 6:07 p.m. UTC | #1
On Thu, 29 Jul 2021 08:09:10 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Some error paths in read_ftrace_files() may lead to a memory leak.
> Improved the error handling of this internal function to avoid it.

This looks more like a normal fix. Can it be moved to the front of the
queue? As it can be applied outside this patch set.

-- Steve

> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-input.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 0ced15a8..b36df98b 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -720,25 +720,28 @@ static int read_ftrace_files(struct tracecmd_input *handle, const char *regex)
>  		}
>  	}
>  
> -	if (read4(handle, &count) < 0)
> -		return -1;
> +	ret = read4(handle, &count);
> +	if (ret < 0)
> +		goto out;
>  
>  	for (i = 0; i < count; i++) {
> -		if (read8(handle, &size) < 0)
> -			return -1;
> +		ret = read8(handle, &size);
> +		if (ret < 0)
> +			goto out;
>  		ret = read_ftrace_file(handle, size, print_all, ereg);
>  		if (ret < 0)
> -			return -1;
> +			goto out;
>  	}
>  
> +	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
> +	ret = 0;
> +out:
>  	if (sreg) {
>  		regfree(sreg);
>  		regfree(ereg);
>  	}
>  
> -	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
> -
> -	return 0;
> +	return ret;
>  }
>  
>  static int read_event_files(struct tracecmd_input *handle, const char *regex)
Steven Rostedt Aug. 19, 2021, 6:08 p.m. UTC | #2
On Thu, 29 Jul 2021 08:09:10 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Some error paths in read_ftrace_files() may lead to a memory leak.
> Improved the error handling of this internal function to avoid it.
> 
>

In fact, all theses memory leak fixes should not even be part of this
patch set, and should be its own patchset to fix the current code.

If any of these is fixing code in this patch set, then simply fix the
patch it fixes.

-- Steve
diff mbox series

Patch

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index 0ced15a8..b36df98b 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -720,25 +720,28 @@  static int read_ftrace_files(struct tracecmd_input *handle, const char *regex)
 		}
 	}
 
-	if (read4(handle, &count) < 0)
-		return -1;
+	ret = read4(handle, &count);
+	if (ret < 0)
+		goto out;
 
 	for (i = 0; i < count; i++) {
-		if (read8(handle, &size) < 0)
-			return -1;
+		ret = read8(handle, &size);
+		if (ret < 0)
+			goto out;
 		ret = read_ftrace_file(handle, size, print_all, ereg);
 		if (ret < 0)
-			return -1;
+			goto out;
 	}
 
+	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
+	ret = 0;
+out:
 	if (sreg) {
 		regfree(sreg);
 		regfree(ereg);
 	}
 
-	handle->file_state = TRACECMD_FILE_FTRACE_EVENTS;
-
-	return 0;
+	return ret;
 }
 
 static int read_event_files(struct tracecmd_input *handle, const char *regex)