Re: [PATCH 10/12] perf jitdump: Validate debug entries on native (non-swap) path
Ian Rogers <[email protected]> Wed, 5 Aug 2026 12:19:13 -0700
| Newsgroups | org.kernel.vger.linux-perf-users,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAP-5=fUZU6um4s4z38DVuschE1_BFaGqp09kCC_Kj=ibM-bGfg@mail.gmail.com> |
On Wed, Aug 5, 2026 at 6:32 AM Arnaldo Carvalho de Melo <[email protected]> wrote: > > From: Arnaldo Carvalho de Melo <[email protected]> > > The bounds-checking and nr_entry clamping added for the byte-swap path > only runs when jd->needs_bswap is true. On native-endian files, nr_entry > passes through unvalidated to jit_repipe_debug_info(), which stores it > as jd->nr_debug_entries. Downstream, jit_process_debug_info() in > genelf_debug.c iterates nr_debug_entries times via debug_entry_next(), > which calls strlen() on each entry's name field — a crafted nr_entry > causes OOB reads and writes. > > Add bounds-checked iteration in jit_repipe_debug_info() that validates > each debug_entry fits in the payload and its name is NUL-terminated > before calling debug_entry_next(). Clamp nr_debug_entries to the count > of valid entries. > > Fixes: 598b7c6919c7bbcc ("perf jit: add source line info 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 | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c > index 02840dbf8a1fc16c..87612ef3e232598e 100644 > --- a/tools/perf/util/jitdump.c > +++ b/tools/perf/util/jitdump.c > @@ -694,8 +694,10 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr) > > static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr) > { > - void *data; > - size_t sz; > + struct debug_entry *ent; > + void *data, *end; > + size_t sz, valid; > + uint64_t i; > > if (!(jd && jr)) > return -1; > @@ -715,10 +717,25 @@ static int jit_repipe_debug_info(struct jit_buf_desc *jd, union jr_entry *jr) > jd->debug_data = data; > > /* > - * we must use nr_entry instead of size here because > - * we cannot distinguish actual entry from padding otherwise > + * Clamp nr_debug_entries to entries that actually fit in the > + * payload. The byte-swap path already does this for cross-endian > + * files; validate on the native path too, since downstream > + * jit_process_debug_info() iterates via debug_entry_next() which > + * calls strlen() on each entry's name field. > */ > - jd->nr_debug_entries = jr->info.nr_entry; > + end = data + sz; > + ent = data; > + valid = 0; > + for (i = 0; i < jr->info.nr_entry; i++) { > + if ((void *)ent + sizeof(*ent) > end) > + break; > + /* name must be NUL-terminated within the payload */ > + if (!memchr(ent->name, '\0', (char *)end - ent->name)) > + break; > + ent = debug_entry_next(ent); > + valid++; > + } > + jd->nr_debug_entries = valid; > > return 0; > } > -- > 2.55.0 >