Re: [PATCH 2/5] perf auxtrace: Fix queue grow overflow and old array leak

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] An infinite loop can be triggered in `auxtrace_heap__add()` when `queue_nr >= 0x80000000`. This is reachable via the CoreSight ETM decoder (`cs-etm.c`) which shifts `queue_nr` left by 16. An attacker can supply a malformed `perf.data` file with an AUXTRACE event `idx` of 32768 to trigger this.
--

--- Patch [2]: [PATCH 2/5] perf auxtrace: Fix queue grow overflow and old array leak ---
commit 6cc7c2090ef2457b5027b79136b869818ae856ee
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf auxtrace: Fix queue grow overflow and old array leak

auxtrace_queues__grow() has two bugs:

1. When idx is UINT_MAX, the caller passes new_nr_queues = idx + 1 = 0.
   The function skips growing (since any nr_queues >= 0), returns
   success, and the caller accesses queue_array[UINT_MAX] — an OOB
   heap write.  Fix by rejecting new_nr_queues == 0 up front.

2. The function allocates a new queue_array via calloc and copies
   elements from the old array, but never frees the old array.  Fix
   by saving the old pointer and freeing it after the copy.

Fixes: e502789302a6ece9 ("perf auxtrace: Add helpers for queuing AUX area tracing data")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index 4cd2caf540152..171edddc982fc 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -251,8 +251,12 @@ static int auxtrace_queues__grow(struct auxtrace_queues *queues,
>  {
>  	unsigned int nr_queues = queues->nr_queues;
>  	struct auxtrace_queue *queue_array;
> +	struct auxtrace_queue *old_array = queues->queue_array;
>  	unsigned int i;
>  
> +	if (!new_nr_queues)
> +		return -EINVAL;
> +
>  	if (!nr_queues)
>  		nr_queues = AUXTRACE_INIT_NR_QUEUES;
>  
> @@ -267,16 +271,17 @@ static int auxtrace_queues__grow(struct auxtrace_queues *queues,
>  		return -ENOMEM;
>  
>  	for (i = 0; i < queues->nr_queues; i++) {
> -		list_splice_tail(&queues->queue_array[i].head,
> +		list_splice_tail(&old_array[i].head,
>  				 &queue_array[i].head);
> -		queue_array[i].tid = queues->queue_array[i].tid;
> -		queue_array[i].cpu = queues->queue_array[i].cpu;
> -		queue_array[i].set = queues->queue_array[i].set;
> -		queue_array[i].priv = queues->queue_array[i].priv;
> +		queue_array[i].tid = old_array[i].tid;
> +		queue_array[i].cpu = old_array[i].cpu;
> +		queue_array[i].set = old_array[i].set;
> +		queue_array[i].priv = old_array[i].priv;
>  	}
>  
>  	queues->nr_queues = nr_queues;
>  	queues->queue_array = queue_array;
> +	free(old_array);
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the queue size
checks here, I noticed a similar unbounded shift loop in auxtrace_heap__add().

Could a malformed perf.data file with an AUXTRACE event idx of 32768 trigger
an infinite loop?

The CoreSight ETM decoder shifts the queue_nr left by 16:

tools/perf/util/cs-etm.c:TO_CS_QUEUE_NR() {
#define TO_CS_QUEUE_NR(queue_nr, trace_chan_id)	\
		      (queue_nr << 16 | trace_chan_id)
}

If queue_nr is 32768, the macro evaluates to 0x80000000. When this value is
passed to auxtrace_heap__add(), it triggers this loop:

tools/perf/util/auxtrace.c:auxtrace_heap__add() {
	if (queue_nr >= heap->heap_sz) {
		unsigned int heap_sz = AUXTRACE_INIT_NR_QUEUES;

		while (heap_sz <= queue_nr)
			heap_sz <<= 1;
...
}

When heap_sz reaches 0x80000000, it satisfies 0x80000000 <= 0x80000000,
shifts left by 1 overflowing to 0, and then 0 <= 0x80000000 remains true
indefinitely, causing an infinite loop.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727161705.64807-1-acme@kernel.org?part=2