Re: [PATCH 4/4] perf symbol: Add missing libgen.h include to get basename() prototype

[Arnaldo Carvalho de Melo]

On Thu, Apr 02, 2026 at 08:19:24AM -0700, Ian Rogers wrote:
> On Wed, Apr 1, 2026 at 5:20 PM Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
> >
> > From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> >
> > This works in some places because there are tricks to prefer a GNU
> > version instead of the XPG version, but started breaking in some musl
> > libc systems due to some unrelated change that made libgen.h stop being
> > included by chance.
> >
> > Since symbol.h is using basename(), make it include libgen.h.
> 
> Huh, this is being flagged by Sashiko:
> """
> Could this include cause segmentation faults by exposing the POSIX version
> of basename() to the rest of the tools?
> 
> According to the POSIX specification, if the input is an empty string
> or slashes, basename() can return a pointer to a statically allocated,
> read-only string literal (such as "." or "/").
> 
> In tools/perf/util/probe-event.c:convert_exec_to_group(), the code takes
> the return value of basename() and modifies it in-place:
>     ptr = basename(exec);
>     ...
>     for (ptr2 = ptr; *ptr2 != '\0'; ptr2++) {
>             if (!isalnum(*ptr2) && *ptr2 != '_')
>                     *ptr2 = '_';
>     }
> If a user provides an empty target or "/", the POSIX basename() would
> return a read-only string. When the loop attempts to write a null or
> underscore into it, wouldn't it trigger an immediate segmentation fault?
> 
> Is there a way we can safely duplicate the returned string before modifying it,
> or ensure the inputs never resolve to these static strings?
> """
> 
> The story of the two basename versions and the issues appears in the
> man pages. The whole thing makes me think we should avoid basename :-)

What sashiko pointed out is a problem in some other part of perf that is
unrelated to this patch and requires a separate fix, no?

I.e. tools/perf/util/probe-event.c already includes libgen.h since
581037151910126a ("perf probe: Add missing libgen.h header needed for
using basename()") from March, 2024.

In the scope of this patch the potential problem isn't possible as
__symbol__join_symfs() isn't changing the return of basename(), right?

⬢ [acme@toolbx perf-tools-next]$ git grep '\<basename(' tools/perf
tools/perf/builtin-daemon.c:    base = basename(basen);
tools/perf/scripts/python/exported-sql-viewer.py:               if short_name[0:7] == "[kernel" and os.path.basename(long_name) == "kcore":
tools/perf/util/annotate.c:             d_filename = basename(filename);
tools/perf/util/dsos.c:        * basename() may modify path buffer, so we must pass
tools/perf/util/dsos.c:          * basename() may return a pointer to internal
tools/perf/util/dsos.c:         base = strdup(basename(lname));
tools/perf/util/probe-event.c:  ptr1 = basename(exec_copy);
tools/perf/util/symbol.h:               base = basename(path_copy);
⬢ [acme@toolbx perf-tools-next]$

It seems just the probe-event.c case needs fixing.

And we also have this in tools/perf/util/srcline.c:

/* basename version that takes a const input string */
static const char *gnu_basename(const char *path)
{
        const char *base = strrchr(path, '/');

        return base ? base + 1 : path;
}

That probably could be reused to avoid these basename() oddities?

- Arnaldo