Re: [PATCH 05/12] perf jitdump: Bounds-check debug entry byte-swap loop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unaligned memory accesses leading to SIGBUS crashes on strict-alignment architectures.
--

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

perf jitdump: Bounds-check debug entry byte-swap loop
    
This commit replaces fixed-size array indexing with pointer arithmetic using
debug_entry_next to iterate variable-length debug entries in
JIT_CODE_DEBUG_INFO records. It also adds bounds checking against the total
record size to prevent out-of-bounds reads.

> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index f79e9420c6bd7..7efbaa07f1ba7 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c

[ ... ]

> @@ -320,14 +320,32 @@ jit_get_next_entry(struct jit_buf_desc *jd)
>  	switch(id) {
>  	case JIT_CODE_DEBUG_INFO:
>  		if (jd->needs_bswap) {
> +			void *end = (void *)jr + jr->prefix.total_size;
> +			struct debug_entry *ent;
>  			uint64_t n;
> +
>  			jr->info.code_addr = bswap_64(jr->info.code_addr);
>  			jr->info.nr_entry  = bswap_64(jr->info.nr_entry);
> -			for (n = 0 ; n < jr->info.nr_entry; n++) {
> -				jr->info.entries[n].addr    = bswap_64(jr->info.entries[n].addr);
> -				jr->info.entries[n].lineno  = bswap_32(jr->info.entries[n].lineno);
> -				jr->info.entries[n].discrim = bswap_32(jr->info.entries[n].discrim);
> +
> +			/*
> +			 * debug_entry has a variable-length name[], so array
> +			 * indexing would compute wrong offsets — use
> +			 * debug_entry_next() and bounds-check each entry.
> +			 */
> +			ent = &jr->info.entries[0];
> +			for (n = 0; n < jr->info.nr_entry; n++) {
> +				if ((void *)ent + sizeof(*ent) > end)
> +					break;
> +				/* name must be NUL-terminated within the record */
> +				if (!memchr(ent->name, '\0', (char *)end - ent->name))
> +					break;
> +				ent->addr    = bswap_64(ent->addr);
> +				ent->lineno  = bswap_32(ent->lineno);
> +				ent->discrim = bswap_32(ent->discrim);

[Severity: Medium]
Does this code risk triggering unaligned memory accesses and SIGBUS crashes
on strict-alignment architectures?

In jitdump files, the debug_entry structure is packed with a
variable-length name string without padding. The pointer arithmetic in
debug_entry_next advances the ent pointer by the exact string length
(including the NUL byte).

If the string has an odd length, subsequent ent pointers will no longer
be aligned to an 8-byte boundary. Because the struct lacks the __packed
attribute, the compiler assumes natural alignment.

When it emits standard aligned load and store instructions for the 64-bit
addr and 32-bit lineno fields, they will trap when receiving an unaligned
address while parsing cross-endian jitdump files in jit_get_next_entry.

> +				ent = debug_entry_next(ent);
>  			}
> +			/* clamp so downstream consumers don't overrun */
> +			jr->info.nr_entry = n;
>  		}

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