scripts/tracepoint-update.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
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
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 */
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>
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
© 2016 - 2026 Red Hat, Inc.