Re: [PATCH 08/29] perf session: Add validated swap infrastructure with null-termination checks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `prot` and `flags` fields in `PERF_RECORD_MMAP2` are completely ignored during endianness swap.
- [High] The `perf_event__event_update_swap` function fails to byte-swap the variable union fields of `PERF_RECORD_EVENT_UPDATE`.
--

commit bc0a5c7da863b9ea5a7bf13cf8099e8bd03a1abc
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf session: Add validated swap infrastructure with null-termination checks

This commit changes swap callbacks to return an int instead of void to allow
error propagation. It also adds a string null-termination check and includes
several security hardenings for cross-endian swap handlers.

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 37544a3574185..d5864e380c1bd 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
[ ... ]
> -static void perf_event__mmap2_swap(union perf_event *event,
> +static int perf_event__mmap2_swap(union perf_event *event,
>  				  bool sample_id_all)
>  {
>  	event->mmap2.pid   = bswap_32(event->mmap2.pid);

[Severity: High]
This is a pre-existing issue, but I noticed the prot and flags fields of
PERF_RECORD_MMAP2 are skipped during the endianness swap. 

When processing cross-endian perf.data files, these 32-bit values will be 
interpreted with the wrong endianness. Could this result in garbage values 
when the flags field is read directly during processing (such as in 
machine__process_mmap2_event to check for MAP_SHARED), leading to incorrect 
memory permissions or misattribution of samples in memory maps?

[ ... ]
> -static void perf_event__event_update_swap(union perf_event *event,
> -					  bool sample_id_all __maybe_unused)
> +static int perf_event__event_update_swap(union perf_event *event,
> +					 bool sample_id_all __maybe_unused)
>  {
>  	event->event_update.type = bswap_64(event->event_update.type);
>  	event->event_update.id   = bswap_64(event->event_update.id);
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but it looks like the variable union fields
in struct perf_record_event_update are never byte-swapped.

The struct contains a union payload (like cpus, scale, or name) based on
event->event_update.type. For cross-endian captures, multi-byte fields
inside the union remain unswapped. Could this lead to data corruption in
downstream stats processing, or potential memory exhaustion and
out-of-bounds reads when allocating CPU maps using unswapped nr values in
cpu_map__new_data?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526211806.1193848-1-acme@kernel.org?part=8