Re: [PATCH v6 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] perf: memory leak of gtrace_decoder on duplicate auxtrace info
- [Low] perf: missing explicit <stdbool.h> inclusion for musl compatibility
--

commit fdb0fe6a9d0684b5f29d2d2a91c37ec24db2cc06
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. Actual decoding of the trace payload is not yet
implemented.

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

[ ... ]

> +static bool gtrace_evsel_is_auxtrace(struct perf_session *session,
> +				      struct evsel *evsel)
> +{

[Severity: Low]
Does this file need an explicit inclusion of <stdbool.h>?

The gtrace_evsel_is_auxtrace() function signature requires the bool type,
but the file fails to explicitly include <stdbool.h>. This might lead to
build failures on musl libc systems where implicit inclusions differ from
glibc.

> +	struct gtrace_decoder *ptr = container_of(session->auxtrace,
> +						   struct gtrace_decoder, auxtrace);
> +
> +	return evsel->core.attr.type == ptr->pmu_type;
> +}

[ ... ]

> +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]
Can this leak the struct gtrace_decoder pointer?

If perf_event__process_auxtrace_info() processes multiple
PERF_RECORD_AUXTRACE_INFO events for GTRACE (e.g. in a malformed or
concatenated perf.data file), gtrace__process_auxtrace_info() will be
called repeatedly.

Each call allocates a new gtrace_decoder via zalloc() and unconditionally
overwrites session->auxtrace here, without verifying if one was already
present or freeing the previous pointer.

> +
> +	return 0;
> +}

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