Re: [PATCH v2 14/14] perf test: Remove /usr/bin/cc dependency from Intel PT shell test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Malformed JIT record payload causes an out-of-bounds heap read in `perf inject --jit` on non-x86 architectures.
- [Medium] Unconditional use of `gettid()` causes build failures on older glibc versions.
--

commit cf0e0be72561e89fdfe0a9f78c96ee5fbf656c9b
Author: Ian Rogers <irogers@google.com>

perf test: Remove /usr/bin/cc dependency from Intel PT shell test

This commit replaces the shell test dependency on an external C compiler in
test_intel_pt.sh by providing a built-in jitdump workload. The new workload
is implemented to support multiple architectures by dynamically encoding
instructions without relying on a compiler.

> diff --git a/tools/perf/tests/workloads/jitdump.c b/tools/perf/tests/workloads/jitdump.c
> new file mode 100644
> index 0000000000000..ff164158902e9
> --- /dev/null
> +++ b/tools/perf/tests/workloads/jitdump.c

[ ... ]

> +static int write_jitdump(FILE *f, void *addr, const void *dat, size_t sz, uint64_t *idx)
> +{
> +	struct jr_code_load rec = {
> +		.p.id = JIT_CODE_LOAD,
> +		.p.total_size = sizeof(rec) + sz,

[Severity: High]
Does this calculation need to include space for the mandatory null-terminated
symbol name string?

The JIT dump specification requires a JIT_CODE_LOAD record to contain a
null-terminated symbol name string immediately following the header, before
the native code payload.

> +		.p.timestamp = get_timestamp(),
> +		.pid = getpid(),
> +		.tid = gettid(),

[Severity: Medium]
Will this unconditional use of gettid() cause build failures on older glibc
versions?

While gettid() is provided by <unistd.h> in glibc 2.30 and later, it is absent
in older versions (such as glibc 2.27, which is still widely tested). Because
this file is now compiled unconditionally for all architectures, compiling perf
on systems with older standard libraries could fail with a
-Werror=implicit-function-declaration error.

> +		.vma = (unsigned long)addr,
> +		.code_addr = (unsigned long)addr,
> +		.code_size = sz,
> +		.code_index = ++*idx,
> +	};
> +
> +	if (fwrite(&rec, sizeof(rec), 1, f) != 1 || fwrite(dat, sz, 1, f) != 1)

[Severity: High]
Does this write omit the required symbol string before the machine code?

Because the symbol string is omitted, perf inject --jit will calculate both
the symbol pointer and the code pointer to point to the exact same location
(the start of the machine code) and call strlen() on it.

On non-x86 architectures where the instruction payload lacks null bytes, this
could cause strlen() to read past the end of the heap-allocated record buffer,
crashing the perf inject tool or causing memory corruption.

> +		return -1;
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531052740.796087-1-irogers@google.com?part=14