Re: [PATCH] perf callchain: Handle multiple address spaces

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Bypassing the kernel IP check on architectures with split address spaces (like s390) causes kernel IPs to be duplicated into the user space callchain.
- [High] Unbounded and off-by-one array writes to `inject->raw_callchain->ips` cause heap buffer overflows. This problem wasn't introduced by this patch, but existed previously.
--

commit 26481cc9533f02e4ba0dd0fa26a4b3d91969b29b
Author: Thomas Richter <tmricht@linux.ibm.com>

perf callchain: Handle multiple address spaces

The patch modifies perf_event__convert_sample_callchain to handle
architectures like s390 that use separate address spaces for user and
kernel memory. It prevents incorrectly dropping user-space functions that
happen to share the same instruction pointer values as kernel addresses.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 5b29f42968611..4f07e90431546 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c

[ ... ]

> @@ -439,7 +439,8 @@ static int perf_event__convert_sample_callchain(const struct perf_tool *tool,
>  
>  	node = cursor->first;
>  	for (k = 0; k < cursor->nr && i < PERF_MAX_STACK_DEPTH; k++) {

This wasn't introduced by this patch, but does this loop and the preceding code
write past the end of the inject->raw_callchain->ips array?

The raw_callchain buffer is allocated with
calloc(PERF_MAX_STACK_DEPTH, sizeof(u64)), which allocates 1016 bytes
(127 * 8). The struct ip_callchain begins with an 8-byte nr header,
leaving only 126 elements for the ips flexible array.

The first loop in perf_event__convert_sample_callchain() iterates up to
sample->callchain->nr without any bounds check against PERF_MAX_STACK_DEPTH.
Since sample->callchain->nr comes from the input perf.data file, a
maliciously crafted file might cause a massive unbounded heap buffer overflow
when perf inject processes it.

Additionally, this loop iterates while i < PERF_MAX_STACK_DEPTH
(i.e. up to i = 126), which allows a write to ips[126]. Accessing ips[126]
writes 8 bytes to offset 1016-1023, which is strictly past the 1016-byte
allocation boundary, causing an off-by-one overflow even for well-formed
callchains.

> -		if (machine__kernel_ip(machine, node->ip))
> +		if (machine->single_address_space &&
> +		    machine__kernel_ip(machine, node->ip))

Will this bypass of the kernel IP check on architectures with split address
spaces (like s390) cause kernel IPs to be duplicated into the user space
callchain?

The cursor populated by thread__resolve_callchain() contains BOTH the kernel
IPs (resolved from sample->callchain via thread__resolve_callchain_sample())
and the user IPs (unwound from DWARF via thread__resolve_callchain_unwind()).

The first loop in perf_event__convert_sample_callchain() copies the kernel IPs
and the PERF_CONTEXT_USER marker directly from the raw sample->callchain into
the newly injected callchain:

	/* copy kernel callchain and context entries */
	for (i = 0; i < sample->callchain->nr; i++) {
		inject->raw_callchain->ips[i] = sample->callchain->ips[i];
		if (sample->callchain->ips[i] == PERF_CONTEXT_USER) {
			i++;
			break;
		}
	}

Because the kernel IPs are present at the beginning of the cursor,
the original code relied on machine__kernel_ip() to identify and skip
them so they wouldn't be appended a second time.

By completely bypassing this check when machine->single_address_space is
false, the kernel IPs in the cursor are no longer skipped. Instead, they
might be erroneously appended to the raw callchain after the
PERF_CONTEXT_USER marker.

This duplicates the kernel callchain and incorrectly classifies those kernel
IPs as user-space addresses, corrupting the callchain data in the injected
perf.data file.

>  			/* kernel IPs were added already */;
>  		else if (node->ms.sym && node->ms.sym->inlined)
>  			/* we can't handle inlined callchains */;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260414124241.490185-1-tmricht@linux.ibm.com?part=1