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

On Wed, Aug 5, 2026 at 12:45 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> 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 <[email protected]> wrote:
> > >
> > > From: Arnaldo Carvalho de Melo <[email protected]>
> > >
> > > 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 <[email protected]>
> > > Cc: Stephane Eranian <[email protected]>
> > > Assisted-by: Claude:claude-opus-4.6
> > > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> > > ---
> > >  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
> :-)

Yeah, I don't know of a plan for AGENTS.md files but there is always:
https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/perf.md

Perhaps we can add something like:
```
## PATH_MAX avoidance

The PATH_MAX constant isn't a limit to paths in places like
/proc/pid/maps, and reading paths from these locations can lead to
buffer overruns for buffers sized at PATH_MAX - as such prefer dynamic
memory allocation for paths. When referring to the size of PATH_MAX
sized char arrays in kernel system call and ring buffer data
structures, prefer to use sizeof the variable rather than the PATH_MAX
constant to make it clear the size and possible truncation is being
enforced by the kernel which may substitute long paths with the value
"//toolong".
```

Thanks,
Ian

> > Reviewed-by: Ian Rogers <[email protected]>
>
> 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
> > >