[PATCH 1/1] libtracecmd: fix memory leak on partial reverse iteration

[PATCH 1/1] libtracecmd: fix memory leak on partial reverse iteration

[Felix Moessbauer]

When calling tracecmd_iterate_events_reverse with a callback that does
not always return 0, the trace is only partially iterated. By that, the
non-iterated records are leaked, resulting in the error:

 1 pages still allocated on cpu <cpu>

We fix this by always iterating the remaining events on all selected
CPUs. In the full iteration case, this stops on the first record as this
is already zero. In the partial iteration case, all remaining records
are freed, which is - by construction of the records list - at max a
page size.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
Note, that this bug has been reported in [1].

[1] https://lore.kernel.org/linux-trace-devel/0b4fb4b774a8754ed86a9212f0a253304a41245c.camel@siemens.com/

Best regards,
Felix Moessbauer
Siemens AG

 lib/trace-cmd/trace-input.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index f2471c92..afdbd2aa 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -2995,6 +2995,25 @@ static struct tep_record *next_last_event(struct tracecmd_input *handle,
 	return record;
 }
 
+static void free_last_events(struct tracecmd_input *handle,
+			     struct tep_record **last_records,
+			     cpu_set_t *cpu_set, int cpu_size,
+			     int cpus)
+{
+	struct tep_record *record;
+	int cpu;
+
+	for (cpu = 0; cpu < cpus; cpu++) {
+		if (cpus && !CPU_ISSET_S(cpu, cpu_size, cpu_set))
+			continue;
+
+		do {
+			record = next_last_event(handle, last_records, cpu);
+			tracecmd_free_record(record);
+		} while (record);
+	}
+}
+
 /**
  * tracecmd_iterate_events_reverse - iterate events over a given handle backwards
  * @handle: The handle to iterate over
@@ -3057,6 +3076,7 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
 		}
 	} while (next_cpu >= 0 && ret == 0);
 
+	free_last_events(handle, records, cpus, cpu_size, max_cpus);
 	free(records);
 
 	return ret;
-- 
2.51.0

Re: [PATCH 1/1] libtracecmd: fix memory leak on partial reverse iteration

[Steven Rostedt]

On Fri, 21 Nov 2025 14:47:49 +0100
Felix Moessbauer <felix.moessbauer@siemens.com> wrote:

> When calling tracecmd_iterate_events_reverse with a callback that does
> not always return 0, the trace is only partially iterated. By that, the
> non-iterated records are leaked, resulting in the error:
> 
>  1 pages still allocated on cpu <cpu>
> 
> We fix this by always iterating the remaining events on all selected
> CPUs. In the full iteration case, this stops on the first record as this
> is already zero. In the partial iteration case, all remaining records
> are freed, which is - by construction of the records list - at max a
> page size.
> 
> Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> ---
> Note, that this bug has been reported in [1].
> 
> [1] https://lore.kernel.org/linux-trace-devel/0b4fb4b774a8754ed86a9212f0a253304a41245c.camel@siemens.com/
> 

Thanks for the report. I actually found two bugs here.

> Best regards,
> Felix Moessbauer
> Siemens AG
> 
>  lib/trace-cmd/trace-input.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index f2471c92..afdbd2aa 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -2995,6 +2995,25 @@ static struct tep_record *next_last_event(struct tracecmd_input *handle,
>  	return record;
>  }
>  
> +static void free_last_events(struct tracecmd_input *handle,
> +			     struct tep_record **last_records,
> +			     cpu_set_t *cpu_set, int cpu_size,
> +			     int cpus)
> +{
> +	struct tep_record *record;
> +	int cpu;
> +
> +	for (cpu = 0; cpu < cpus; cpu++) {
> +		if (cpus && !CPU_ISSET_S(cpu, cpu_size, cpu_set))
> +			continue;
> +
> +		do {
> +			record = next_last_event(handle, last_records, cpu);
> +			tracecmd_free_record(record);
> +		} while (record);

I just fixed it slightly different than what you proposed here. But I think
we can take your patch instead.

> +	}
> +}
> +
>  /**
>   * tracecmd_iterate_events_reverse - iterate events over a given handle backwards
>   * @handle: The handle to iterate over
> @@ -3057,6 +3076,7 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
>  		}
>  	} while (next_cpu >= 0 && ret == 0);
>  
> +	free_last_events(handle, records, cpus, cpu_size, max_cpus);
>  	free(records);
>  
>  	return ret;


The second bug I found was that calling tracecmd_iterate_events_reverse()
again after a callback returned a ret value, and this time with cont = true,
it can skip a lot of records because the cursor of the last record read is
not saved. This now makes calling this function again with cont start with
the last record that was read (the one that the caller exited from).

The below patch fixes that too:

-- Steve

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index afdbd2aa98b6..8dfeff458485 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -3036,6 +3036,7 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
 				    void *callback_data, bool cont)
 {
 	unsigned long long last_timestamp = 0;
+	unsigned long long page_offset = 0;
 	struct tep_record **records;
 	struct tep_record *record;
 	int next_cpu;
@@ -3072,6 +3073,8 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
 			record = next_last_event(handle, records, next_cpu);;
 			ret = call_callbacks(handle, record, next_cpu,
 					     callback, callback_data);
+			if (ret)
+				page_offset = record->offset;
 			tracecmd_free_record(record);
 		}
 	} while (next_cpu >= 0 && ret == 0);
@@ -3079,6 +3082,17 @@ int tracecmd_iterate_events_reverse(struct tracecmd_input *handle,
 	free_last_events(handle, records, cpus, cpu_size, max_cpus);
 	free(records);
 
+	/*
+	 * If the callback exited out early, then set the cursor back
+	 * to the location of that record so that if this gets called
+	 * again with cont = true, it will continue where it left off.
+	 */
+	if (page_offset) {
+		/* Set the record to the previous record that was read */
+		record = tracecmd_read_at(handle, page_offset - 4, NULL);
+		tracecmd_free_record(record);
+	}
+
 	return ret;
 }
 

Re: [PATCH 1/1] libtracecmd: fix memory leak on partial reverse iteration

[MOESSBAUER, Felix]

On Fri, 2025-11-21 at 12:01 -0500, Steven Rostedt wrote:
> On Fri, 21 Nov 2025 14:47:49 +0100
> Felix Moessbauer <felix.moessbauer@siemens.com> wrote:
> 
> > When calling tracecmd_iterate_events_reverse with a callback that does
> > not always return 0, the trace is only partially iterated. By that, the
> > non-iterated records are leaked, resulting in the error:
> > 
> >  1 pages still allocated on cpu <cpu>
> > 
> > We fix this by always iterating the remaining events on all selected
> > CPUs. In the full iteration case, this stops on the first record as this
> > is already zero. In the partial iteration case, all remaining records
> > are freed, which is - by construction of the records list - at max a
> > page size.
> > 
> > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > ---
> > Note, that this bug has been reported in [1].
> > 
> > [1] https://lore.kernel.org/linux-trace-devel/0b4fb4b774a8754ed86a9212f0a253304a41245c.camel@siemens.com/
> > 
> 
> Thanks for the report. I actually found two bugs here.

Hi,

while thinking about it again, I noticed that the callback based
iteration interface relies on some assumptions which are not
documented. While probably obvious, it should be documented what is NOT
allowed to be done inside the callback (i.e. calling any 
tracecmd_iterate_* function, but also calling tracecmd_read_* and
alike?).

Best regards,
Felix

-- 
Siemens AG
Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany

Re: [PATCH 1/1] libtracecmd: fix memory leak on partial reverse iteration

[Steven Rostedt]

Nit, when sending patches, please capitalize the first word in the subject:

   [PATCH] libtracecmd: Fix memory leak on partial reverse iteration

(I updated this when pulling in your patch).

And you don't need the 1/1 for a single patch.

Thanks!

-- Steve