Re: [PATCH 05/12] perf jitdump: Check snprintf return before computing header size

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Wed, Aug 05, 2026 at 12:07:23PM -0700, Ian Rogers wrote:
> On Wed, Aug 5, 2026 at 6:31 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > snprintf() returns the would-have-been length on truncation.  When the
> > jitted filename exceeds PATH_MAX, the unclamped 'size' value inflates
> > sizeof(event->mmap2.filename) - size into a massive underflow, causing
> > the header.size computation to write an oversized header.  The
> > subsequent write to 'id = event + header.size - idr_size' then corrupts
> > the heap.
> >
> > Clamp size to PATH_MAX - 1 after snprintf in both jit_repipe_code_load()
> > and jit_repipe_code_move().
> >
> > Fixes: 9b07e27f88b9 ("perf inject: Add jitdump mmap injection support")
> > Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> > Cc: Stephane Eranian <eranian@google.com>
> > Assisted-by: Claude:claude-opus-4.6
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/jitdump.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> > index 078d3304d2b7ebce..fd11e07bf00b7978 100644
> > --- a/tools/perf/util/jitdump.c
> > +++ b/tools/perf/util/jitdump.c
> > @@ -493,6 +493,9 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
> >                         jd->dir,
> >                         nspid,
> >                         count);
> > +       /* snprintf returns would-be length on truncation, clamp to buffer */
> > +       if (size >= PATH_MAX)
> > +               size = PATH_MAX - 1;
> 
> Given the recent fixes to reading /proc/pid/maps where it was assumed
> the file paths would be limited to PATH_MAX and it turns out that
> PATH_MAX doesn't really do that and we had potential buffer overruns
> during synthesis, I wonder it would be more intention revealing here
> to use "sizeof(event->mmap2.filename)" rather than PATH_MAX. Other
> than the constant used, I agree with the change and using PATH_MAX
> isn't wrong.

We need to go on having our tools/perf/AGENTS.md with all those rules
:-)
 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!

- Arnaldo
 
> Thanks,
> Ian
> 
> >
> >         size++; /* for \0 */
> >
> > @@ -623,6 +626,9 @@ static int jit_repipe_code_move(struct jit_buf_desc *jd, union jr_entry *jr)
> >                  jd->dir,
> >                  nspid,
> >                  jr->move.code_index);
> > +       /* snprintf returns would-be length on truncation, clamp to buffer */
> > +       if (size >= PATH_MAX)
> > +               size = PATH_MAX - 1;
> >
> >         size++; /* for \0 */
> >
> > --
> > 2.55.0
> >