All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf data convert json: Fix trace_seq memory leak in process_sample_event()
@ 2026-06-04  6:55 Tanushree Shah
  2026-06-04  7:06 ` sashiko-bot
  2026-06-04 21:29 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 4+ messages in thread
From: Tanushree Shah @ 2026-06-04  6:55 UTC (permalink / raw)
  To: acme, jolsa, adrian.hunter, vmolnaro, mpetlan, tmricht, maddy,
	irogers, namhyung
  Cc: linux-perf-users, linuxppc-dev, atrajeev, hbathini, Tejas.Manhas1,
	Tanushree.Shah, Shivani.Nittor, Tanushree Shah

Unlike the in-kernel trace_seq which uses a statically allocated buffer,
the userspace traceevent library's trace_seq uses a dynamically allocated
one. Therefore, every trace_seq_init() call must be paired with a
trace_seq_destroy(), otherwise it produces a memory leak.

In process_sample_event(), a trace_seq is initialized for each field when
formatting tracepoint raw_data, but the matching trace_seq_destroy() is
never called, leaking memory for every field of every sample processed.

Add the missing trace_seq_destroy() after using the trace_seq buffer to
properly free the allocated memory.

Detected with Valgrind on a perf.data file with 2,729 tracepoint samples:
  Before: definitely lost: 55,537,664 bytes in 13,559 blocks
  After:  definitely lost: 0 bytes in 0 blocks

Fixes: 9d895e468429 ("perf data: Add tracepoint fields when converting to JSON")
Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
---
 tools/perf/util/data-convert-json.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/data-convert-json.c b/tools/perf/util/data-convert-json.c
index d526c91312ed..6dd9349ab10f 100644
--- a/tools/perf/util/data-convert-json.c
+++ b/tools/perf/util/data-convert-json.c
@@ -257,6 +257,7 @@ static int process_sample_event(const struct perf_tool *tool,
 				trace_seq_init(&s);
 				tep_print_field(&s, sample->raw_data, fields[i]);
 				output_json_key_string(out, true, 3, fields[i]->name, s.buffer);
+				trace_seq_destroy(&s);
 
 				i++;
 			}
-- 
2.47.3


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

* Re: [PATCH] perf data convert json: Fix trace_seq memory leak in process_sample_event()
  2026-06-04  6:55 [PATCH] perf data convert json: Fix trace_seq memory leak in process_sample_event() Tanushree Shah
@ 2026-06-04  7:06 ` sashiko-bot
  2026-06-04 16:10   ` Tanushree Shah
  2026-06-04 21:29 ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-06-04  7:06 UTC (permalink / raw)
  To: Tanushree Shah; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `process_sample_event()` function leaks reference counts and memory associated with `struct addr_location` when a sample is skipped due to time filtering.
--

commit de6e4710063f2942dd58ebe436d7d992872132b3
Author: Tanushree Shah <tshah@linux.ibm.com>

perf data convert json: Fix trace_seq memory leak in process_sample_event()

This commit fixes a memory leak by ensuring trace_seq_destroy() is called
to pair with trace_seq_init() when formatting tracepoint raw data.

> diff --git a/tools/perf/util/data-convert-json.c b/tools/perf/util/data-convert-json.c
> index d526c91312ed8..6dd9349ab10f1 100644
> --- a/tools/perf/util/data-convert-json.c
> +++ b/tools/perf/util/data-convert-json.c
> @@ -257,6 +257,7 @@ static int process_sample_event(const struct perf_tool *tool,
>  				trace_seq_init(&s);
>  				tep_print_field(&s, sample->raw_data, fields[i]);
>  				output_json_key_string(out, true, 3, fields[i]->name, s.buffer);
> +				trace_seq_destroy(&s);
>  
>  				i++;
>  			}

[Severity: High]
This is a pre-existing issue, but does the early return path for skipped
samples higher up in process_sample_event() leak the addr_location
reference handles?

tools/perf/util/data-convert-json.c:process_sample_event() {
        addr_location__init(&al);
        if (machine__resolve(machine, &al, sample) < 0) {
                pr_err("Sample resolution failed!\n");
                addr_location__exit(&al);
                return -1;
        }

        if (perf_time__ranges_skip_sample(c->ptime_range, c->range_num, sample->time)) {
                ++c->skipped;
                return 0;
        }
        ...
}

When machine__resolve() populates the addr_location structure, it acquires
reference handles for objects like thread and map. The early return for time
filtering bypasses the addr_location__exit() cleanup.

Could this cause reference counts and memory to leak when dropping skipped
samples?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604065519.112101-1-tshah@linux.ibm.com?part=1

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

* Re: [PATCH] perf data convert json: Fix trace_seq memory leak in process_sample_event()
  2026-06-04  7:06 ` sashiko-bot
@ 2026-06-04 16:10   ` Tanushree Shah
  0 siblings, 0 replies; 4+ messages in thread
From: Tanushree Shah @ 2026-06-04 16:10 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-perf-users

Hi Sashiko,

Thank you for the review and for spotting this!

Regarding the addr_location reference leak:

yes its correct. The early return path for time-filtered samples does 
leak addr_location references.

When machine__resolve() succeeds, it calls thread__find_map(), which 
acquires reference counts for thread, map, and maps objects and stores 
them in the addr_location structure. The early return bypasses 
addr_location__exit(), preventing these references from being released.

Validation with Valgrind:

I've validated the issue with Valgrind using a test case with 2,385 
total samples (150 converted, 2,235 skipped by time filter):

Without the fix:

definitely lost:  8,336 bytes in 32 blocks
indirectly lost:  1,539,361 bytes in 21,307 blocks
possibly lost:    10,325,465 bytes in 146,283 blocks
Total leaked:     ~11.9 MB

With the fix:

definitely lost:  0 bytes in 0 blocks
indirectly lost:  0 bytes in 0 blocks
possibly lost:    0 bytes in 0 blocks
Total leaked:     0 bytes


I will send a v2 patch series that addresses both memory leaks:

The trace_seq_destroy() fix (original issue)
The addr_location__exit() fix for the early return path

Thanks
Tanushree Shah

On 04/06/26 12:36, sashiko-bot@kernel.org wrote:
> perf data convert json: Fix trace_seq memory leak in process_sample_event()
> 
> This commit fixes a memory leak by ensuring trace_seq_destroy() is called
> to pair with trace_seq_init() when formatting tracepoint raw data.
> 
>> diff --git a/tools/perf/util/data-convert-json.c b/tools/perf/util/data-convert-json.c
>> index d526c91312ed8..6dd9349ab10f1 100644
>> --- a/tools/perf/util/data-convert-json.c
>> +++ b/tools/perf/util/data-convert-json.c
>> @@ -257,6 +257,7 @@ static int process_sample_event(const struct perf_tool *tool,
>>   				trace_seq_init(&s);
>>   				tep_print_field(&s, sample->raw_data, fields[i]);
>>   				output_json_key_string(out, true, 3, fields[i]->name, s.buffer);
>> +				trace_seq_destroy(&s);
>>   
>>   				i++;
>>   			}
> [Severity: High]
> This is a pre-existing issue, but does the early return path for skipped
> samples higher up in process_sample_event() leak the addr_location
> reference handles?
> 
> tools/perf/util/data-convert-json.c:process_sample_event() {
>          addr_location__init(&al);
>          if (machine__resolve(machine, &al, sample) < 0) {
>                  pr_err("Sample resolution failed!\n");
>                  addr_location__exit(&al);
>                  return -1;
>          }
> 
>          if (perf_time__ranges_skip_sample(c->ptime_range, c->range_num, sample->time)) {
>                  ++c->skipped;
>                  return 0;
>          }
>          ...
> }
> 
> When machine__resolve() populates the addr_location structure, it acquires
> reference handles for objects like thread and map. The early return for time
> filtering bypasses the addr_location__exit() cleanup.
> 
> Could this cause reference counts and memory to leak when dropping skipped
> samples?
> 
> -- Sashiko AI review · https://sashiko.dev/#/ 
> patchset/20260604065519.112101-1-tshah@linux.ibm.com?part=1


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

* Re: [PATCH] perf data convert json: Fix trace_seq memory leak in process_sample_event()
  2026-06-04  6:55 [PATCH] perf data convert json: Fix trace_seq memory leak in process_sample_event() Tanushree Shah
  2026-06-04  7:06 ` sashiko-bot
@ 2026-06-04 21:29 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-04 21:29 UTC (permalink / raw)
  To: Tanushree Shah
  Cc: jolsa, adrian.hunter, vmolnaro, mpetlan, tmricht, maddy, irogers,
	namhyung, linux-perf-users, linuxppc-dev, atrajeev, hbathini,
	Tejas.Manhas1, Tanushree.Shah, Shivani.Nittor

On Thu, Jun 04, 2026 at 12:25:19PM +0530, Tanushree Shah wrote:
> Unlike the in-kernel trace_seq which uses a statically allocated buffer,
> the userspace traceevent library's trace_seq uses a dynamically allocated
> one. Therefore, every trace_seq_init() call must be paired with a
> trace_seq_destroy(), otherwise it produces a memory leak.
> 
> In process_sample_event(), a trace_seq is initialized for each field when
> formatting tracepoint raw_data, but the matching trace_seq_destroy() is
> never called, leaking memory for every field of every sample processed.
> 
> Add the missing trace_seq_destroy() after using the trace_seq buffer to
> properly free the allocated memory.
> 
> Detected with Valgrind on a perf.data file with 2,729 tracepoint samples:
>   Before: definitely lost: 55,537,664 bytes in 13,559 blocks
>   After:  definitely lost: 0 bytes in 0 blocks

Thanks, applied to perf-tools-next, for v7.2.

- Arnaldo
 
> Fixes: 9d895e468429 ("perf data: Add tracepoint fields when converting to JSON")
> Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
> ---
>  tools/perf/util/data-convert-json.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/data-convert-json.c b/tools/perf/util/data-convert-json.c
> index d526c91312ed..6dd9349ab10f 100644
> --- a/tools/perf/util/data-convert-json.c
> +++ b/tools/perf/util/data-convert-json.c
> @@ -257,6 +257,7 @@ static int process_sample_event(const struct perf_tool *tool,
>  				trace_seq_init(&s);
>  				tep_print_field(&s, sample->raw_data, fields[i]);
>  				output_json_key_string(out, true, 3, fields[i]->name, s.buffer);
> +				trace_seq_destroy(&s);
>  
>  				i++;
>  			}
> -- 
> 2.47.3

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

end of thread, other threads:[~2026-06-04 21:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04  6:55 [PATCH] perf data convert json: Fix trace_seq memory leak in process_sample_event() Tanushree Shah
2026-06-04  7:06 ` sashiko-bot
2026-06-04 16:10   ` Tanushree Shah
2026-06-04 21:29 ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.