public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scripts/tracepoint-update: fix memory leak in make_trace_array()
@ 2026-01-18 13:02 Weigang He
  2026-01-18 15:54 ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Weigang He @ 2026-01-18 13:02 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel, Weigang He,
	Tuo Li

In make_trace_array(), if add_string() fails after some successful
iterations, the function returns without freeing the 'vals' array that
was allocated by previous add_string() calls.

The add_string() function uses realloc() internally with a local
temporary variable, which means the original pointer is preserved on
allocation failure. When make_trace_array() returns early on error,
the previously allocated memory is leaked.

Fix this by freeing 'vals' before returning on the error path.

This bug is found by my static analysis tool and my code review.

Signed-off-by: Tuo Li <islituo@gmail.com>
---
 scripts/tracepoint-update.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/tracepoint-update.c b/scripts/tracepoint-update.c
index 90046aedc97b9..7bc9d66229ddf 100644
--- a/scripts/tracepoint-update.c
+++ b/scripts/tracepoint-update.c
@@ -93,8 +93,10 @@ static void make_trace_array(struct elf_tracepoint *etrace)
 	for_each_shdr_str(len, ehdr, check_data_sec) {
 		if (!len)
 			continue;
-		if (add_string(str, &vals, &count) < 0)
+		if (add_string(str, &vals, &count) < 0) {
+			free(vals);
 			return;
+		}
 	}
 
 	/* If CONFIG_TRACEPOINT_VERIFY_USED is not set, there's nothing to do */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] scripts/tracepoint-update: fix memory leak in make_trace_array()
  2026-01-18 13:02 [PATCH] scripts/tracepoint-update: fix memory leak in make_trace_array() Weigang He
@ 2026-01-18 15:54 ` Steven Rostedt
  2026-01-21  2:30   ` Masami Hiramatsu
  0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2026-01-18 15:54 UTC (permalink / raw)
  To: Weigang He
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, Tuo Li

On Sun, 18 Jan 2026 13:02:47 +0000
Weigang He <geoffreyhe2@gmail.com> wrote:

> In make_trace_array(), if add_string() fails after some successful
> iterations, the function returns without freeing the 'vals' array that
> was allocated by previous add_string() calls.
> 
> The add_string() function uses realloc() internally with a local
> temporary variable, which means the original pointer is preserved on
> allocation failure. When make_trace_array() returns early on error,
> the previously allocated memory is leaked.
> 
> Fix this by freeing 'vals' before returning on the error path.
> 
> This bug is found by my static analysis tool and my code review.
> 
> Signed-off-by: Tuo Li <islituo@gmail.com>
> ---
>  scripts/tracepoint-update.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/tracepoint-update.c b/scripts/tracepoint-update.c
> index 90046aedc97b9..7bc9d66229ddf 100644
> --- a/scripts/tracepoint-update.c
> +++ b/scripts/tracepoint-update.c
> @@ -93,8 +93,10 @@ static void make_trace_array(struct elf_tracepoint *etrace)
>  	for_each_shdr_str(len, ehdr, check_data_sec) {
>  		if (!len)
>  			continue;
> -		if (add_string(str, &vals, &count) < 0)
> +		if (add_string(str, &vals, &count) < 0) {
> +			free(vals);
>  			return;
> +		}
>  	}

It would make much more sense to have add_string() free vals, and set
vals to NULL on error.

-- Steve


>  
>  	/* If CONFIG_TRACEPOINT_VERIFY_USED is not set, there's nothing to do */


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] scripts/tracepoint-update: fix memory leak in make_trace_array()
  2026-01-18 15:54 ` Steven Rostedt
@ 2026-01-21  2:30   ` Masami Hiramatsu
  2026-01-21 14:41     ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Masami Hiramatsu @ 2026-01-21  2:30 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Weigang He, Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, Tuo Li

On Sun, 18 Jan 2026 10:54:57 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Sun, 18 Jan 2026 13:02:47 +0000
> Weigang He <geoffreyhe2@gmail.com> wrote:
> 
> > In make_trace_array(), if add_string() fails after some successful
> > iterations, the function returns without freeing the 'vals' array that
> > was allocated by previous add_string() calls.
> > 
> > The add_string() function uses realloc() internally with a local
> > temporary variable, which means the original pointer is preserved on
> > allocation failure. When make_trace_array() returns early on error,
> > the previously allocated memory is leaked.
> > 
> > Fix this by freeing 'vals' before returning on the error path.
> > 
> > This bug is found by my static analysis tool and my code review.
> > 
> > Signed-off-by: Tuo Li <islituo@gmail.com>
> > ---
> >  scripts/tracepoint-update.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/tracepoint-update.c b/scripts/tracepoint-update.c
> > index 90046aedc97b9..7bc9d66229ddf 100644
> > --- a/scripts/tracepoint-update.c
> > +++ b/scripts/tracepoint-update.c
> > @@ -93,8 +93,10 @@ static void make_trace_array(struct elf_tracepoint *etrace)
> >  	for_each_shdr_str(len, ehdr, check_data_sec) {
> >  		if (!len)
> >  			continue;
> > -		if (add_string(str, &vals, &count) < 0)
> > +		if (add_string(str, &vals, &count) < 0) {
> > +			free(vals);
> >  			return;
> > +		}
> >  	}
> 
> It would make much more sense to have add_string() free vals, and set
> vals to NULL on error.

I think it should be failed if it fails to add string. Can it
continue checking tracepoints even after the error?

Thank you,

> 
> -- Steve
> 
> 
> >  
> >  	/* If CONFIG_TRACEPOINT_VERIFY_USED is not set, there's nothing to do */
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] scripts/tracepoint-update: fix memory leak in make_trace_array()
  2026-01-21  2:30   ` Masami Hiramatsu
@ 2026-01-21 14:41     ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2026-01-21 14:41 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Weigang He, Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
	Tuo Li

On Wed, 21 Jan 2026 11:30:35 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > It would make much more sense to have add_string() free vals, and set
> > vals to NULL on error.  
> 
> I think it should be failed if it fails to add string. Can it
> continue checking tracepoints even after the error?

This patch is simply fixing a memory leak on failure (which isn't really a
big deal since this is just a user space tool that runs for a short time
during build). Returning a failure here is out of scope of this patch.

Feel free to send an RFC patch that returns a failure built on top of this
patch, and we can discuss if that should be done or not for that change.

-- Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-01-21 14:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-18 13:02 [PATCH] scripts/tracepoint-update: fix memory leak in make_trace_array() Weigang He
2026-01-18 15:54 ` Steven Rostedt
2026-01-21  2:30   ` Masami Hiramatsu
2026-01-21 14:41     ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox