[PATCH] perf cs-etm: Avoid truncating AUX buffer sizes to int

[PATCH] perf cs-etm: Avoid truncating AUX buffer sizes to int

[Leo Yan]

cs_etm__get_trace() returns an int, but it used to return etmq->buf_len
on success. That value comes from auxtrace_buffer::size, which is a
size_t. For a large AUX trace block, returning the byte count through an
int can overflow and make a valid buffer look like a negative error.

The callers do not need the actual byte count from cs_etm__get_trace().
The buffer length is already stored in the etmq->buf_len. The callers
only need to distinguish three states:

  < 0: error
  = 0: no more AUX buffers
  > 0: data is available

Make cs_etm__get_trace() return 0 for all non-error cases and use
etmq->buf_len to indicate whether a new buffer was found. Then make
cs_etm__get_data_block() return 1 whenever data is available, instead of
returning the buffer length.

Also refactor cs_etm__get_data_block() to make its return value
semantics clearer.

Reported-by: Suyash Mahar <smahar@meta.com>
Fixes: 8224531cf5a1 ("perf cs-etm: Modularize auxtrace_buffer fetch function")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/cs-etm.c | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 95e3ec1171acc8442d8539a72a26a1a5a53a2f37..114b3cd2da495e54e78df8fbd707898312f95079 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1467,8 +1467,7 @@ cs_etm__get_trace(struct cs_etm_queue *etmq)
 	etmq->buf_used = 0;
 	etmq->buf_len = aux_buffer->size;
 	etmq->buf = aux_buffer->data;
-
-	return etmq->buf_len;
+	return 0;
 }
 
 /*
@@ -2151,26 +2150,33 @@ static int cs_etm__get_data_block(struct cs_etm_queue *etmq)
 {
 	int ret;
 
-	if (!etmq->buf_len) {
-		ret = cs_etm__get_trace(etmq);
-		if (ret <= 0)
-			return ret;
-		/*
-		 * We cannot assume consecutive blocks in the data file
-		 * are contiguous, reset the decoder to force re-sync.
-		 */
-		ret = cs_etm_decoder__reset(etmq->decoder);
-		if (ret)
-			return ret;
+	/* The current block is not finished */
+	if (etmq->buf_len)
+		return 1;
 
-		/*
-		 * Since the decoder is reset, this causes a global trace
-		 * discontinuity. Flush all thread stacks.
-		 */
-		cs_etm__flush_all_stack(etmq);
-	}
+	ret = cs_etm__get_trace(etmq);
+	if (ret < 0)
+		return ret;
+
+	/* No more buffer to read */
+	if (!etmq->buf_len)
+		return 0;
+
+	/*
+	 * We cannot assume consecutive blocks in the data file
+	 * are contiguous, reset the decoder to force re-sync.
+	 */
+	ret = cs_etm_decoder__reset(etmq->decoder);
+	if (ret)
+		return ret;
+
+	/*
+	 * Since the decoder is reset, this causes a global trace
+	 * discontinuity. Flush all thread stacks.
+	 */
+	cs_etm__flush_all_stack(etmq);
 
-	return etmq->buf_len;
+	return 1;
 }
 
 static bool cs_etm__is_svc_instr(struct cs_etm_queue *etmq,

---
base-commit: e01c7bd5b1eece254bcbf282db066b12c4815d21
change-id: 20260720-perf_cs_etm_fix_big_size-ec5608f767df

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

Re: [PATCH] perf cs-etm: Avoid truncating AUX buffer sizes to int

[sashiko-bot]

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

Pre-existing issues:
- [Medium] Integer truncation leading to infinite loop (DoS) in cs-etm decoder for AUX trace blocks >= 4GB.
--

--- Patch [1]: [PATCH] perf cs-etm: Avoid truncating AUX buffer sizes to int ---
commit 1f84d1046e544ee9b629c80e55d031135982e6db
Author: Leo Yan <leo.yan@arm.com>

perf cs-etm: Avoid truncating AUX buffer sizes to int

This commit prevents truncation of large AUX buffer sizes when returning
them from cs_etm__get_trace(). Instead of returning the buffer length as
an int, which can overflow for sizes over 2GB, the function now returns 0
on success and uses etmq->buf_len to track the size. The caller
cs_etm__get_data_block() is updated to return 1 when data is available.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 95e3ec1171acc..114b3cd2da495 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1467,8 +1467,7 @@ cs_etm__get_trace(struct cs_etm_queue *etmq)
>  	etmq->buf_used = 0;
>  	etmq->buf_len = aux_buffer->size;
>  	etmq->buf = aux_buffer->data;
> -
> -	return etmq->buf_len;
> +	return 0;
>  }

[Severity: Medium]
This is a pre-existing issue, but since we are fixing buffer size
truncations for large AUX trace blocks, should we also address the
truncation in cs_etm_decoder__process_data_block()?

When an AUX trace block size is 4GB or larger, etmq->buf_len (a 64-bit
size_t) is eventually passed down to cs_etm_decoder__process_data_block()
as the len parameter. In tools/perf/util/cs-etm-decoder/cs-etm-decoder.c:

		} else if (OCSD_DATA_RESP_IS_CONT(prev_return)) {
			cur = ocsd_dt_process_data(decoder->dcd_tree,
						   OCSD_OP_DATA,
						   indx + processed,
						   len - processed,
						   &buf[processed],
						   &count);
			processed += count;

The OpenCSD library function ocsd_dt_process_data() uses a 32-bit integer
for the buffer size. The expression len - processed is implicitly
truncated to a 32-bit integer.

If len - processed is exactly a multiple of 4GB (or reaches a multiple
of 4GB during processing), the truncated size becomes 0. This instructs
ocsd_dt_process_data() to process 0 bytes, returning count = 0.

Since processed does not increase, could this result in an infinite loop
when processing large or maliciously crafted perf.data files?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-perf_cs_etm_fix_big_size-v1-1-97ded24636b1@arm.com?part=1