Re: [PATCH v5 13/14] perf tools: Initial support for gtrace decoder

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing explicit header inclusions for free() and zalloc(), leading to musl compatibility failures and pointer truncation.
- [Medium] Unconditionally overwriting `session->auxtrace` causes a memory leak on multiple `PERF_RECORD_AUXTRACE_INFO` events.
--

--- Patch [13]: [PATCH v5 13/14] perf tools: Initial support for gtrace decoder ---
commit 7c1bf67a1a25514ab10564a3e48db1965dbac1a8
Author: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>

perf tools: Initial support for gtrace decoder

Add bare bones support for gtrace decoder so that the data received
from the hardware by the gtrace perf driver can be written to the
perf record output file.

> diff --git a/tools/perf/util/gtrace-decoder.c b/tools/perf/util/gtrace-decoder.c
> new file mode 100644
> index 0000000000000..4a1571431edb3
> --- /dev/null
> +++ b/tools/perf/util/gtrace-decoder.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Generic hardware trace (gtrace) decoder
> + * Copyright (c) 2026 Qualcomm Technologies, Inc.
> + */
> +
> +#include <errno.h>
> +#include <inttypes.h>
> +#include <internal/lib.h>
> +#include "evlist.h"
> +#include "session.h"
> +#include "gtrace.h"

[Severity: Medium]
Are we missing some explicit header inclusions here?

The code uses free() and zalloc() further down in the file, but does not
include <stdlib.h> or <linux/zalloc.h>.

Missing <stdlib.h> can cause compilation failures under musl.

Missing <linux/zalloc.h> can cause zalloc() to be implicitly declared as
returning an int. On 64-bit systems, this truncates the upper 32 bits of
the pointer, which can lead to a segmentation fault when dereferenced.

[ ... ]

> +int gtrace__process_auxtrace_info(union perf_event *event,
> +				   struct perf_session *session)
> +{
> +	struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
> +	struct gtrace_decoder *ptr;
> +
> +	if (auxtrace_info->header.size < GTRACE_AUXTRACE_PRIV_SIZE +
> +	    sizeof(struct perf_record_auxtrace_info))
> +		return -EINVAL;
> +
> +	ptr = zalloc(sizeof(*ptr));
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	ptr->session = session;
> +	ptr->machine = &session->machines.host;
> +	ptr->auxtrace_type = auxtrace_info->type;
> +	ptr->pmu_type = auxtrace_info->priv[0];
> +
> +	ptr->auxtrace.process_event = gtrace_process_event;
> +	ptr->auxtrace.process_auxtrace_event = gtrace_process_auxtrace_event;
> +	ptr->auxtrace.flush_events = gtrace_flush;
> +	ptr->auxtrace.free_events = gtrace_free_events;
> +	ptr->auxtrace.free = gtrace_free;
> +	ptr->auxtrace.evsel_is_auxtrace = gtrace_evsel_is_auxtrace;
> +	session->auxtrace = &ptr->auxtrace;

[Severity: Medium]
Could this leak the previously allocated decoder context if multiple
PERF_RECORD_AUXTRACE_INFO events are processed?

It looks like session->auxtrace is unconditionally overwritten here without
checking or freeing any structure that was already allocated.

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810152223.3946743-1-mayuresh.chitale@oss.qualcomm.com?part=13