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

On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <[email protected]> wrote:
>
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> The byte-swap loop for JIT_CODE_DEBUG_INFO uses array indexing
> (jr->info.entries[n]) to iterate debug entries.  struct debug_entry has
> a flexible array member name[], so each entry has a different size.
> Array indexing computes offsets assuming fixed-size elements, landing
> inside variable-length name strings after the first entry and
> byte-swapping garbage.
>
> Additionally, nr_entry is read from untrusted jitdump input without
> validation against total_size, so a crafted value causes OOB reads.
>
> Replace the array indexing with debug_entry_next() pointer arithmetic
> (which correctly accounts for the variable-length name) and
> bounds-check each entry against the record's total_size before
> byte-swapping.
>
> Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> Reported-by: sashiko-bot <[email protected]>
> Cc: Stephane Eranian <[email protected]>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

Reviewed-by: Ian Rogers <[email protected]>

Thanks!
Ian

> ---
>  tools/perf/util/jitdump.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index 787f8a03dae87908..078d3304d2b7ebce 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -318,14 +318,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);
> +                               ent = debug_entry_next(ent);
>                         }
> +                       /* clamp so downstream consumers don't overrun */
> +                       jr->info.nr_entry = n;
>                 }
>                 break;
>         case JIT_CODE_UNWINDING_INFO:
> --
> 2.55.0
>