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

[Arnaldo Melo]

On April 2, 2026 12:19:24 PM GMT-03:00, Ian Rogers <irogers@xxxxxxxxxx> 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 :-)

:-)

I think we should just follow it's advice and use strdup in it's return, in a helper, and have it's users free it afterwards. 

- Arnaldo

>
>Thanks,
>Ian
>
>> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>> ---
>>  tools/perf/util/symbol.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
>> index c67814d6d6d6f64a..b37ecc2e90f77efe 100644
>> --- a/tools/perf/util/symbol.h
>> +++ b/tools/perf/util/symbol.h
>> @@ -10,6 +10,7 @@
>>  #include <linux/rbtree.h>
>>  #include <stdio.h>
>>  #include <errno.h>
>> +#include <libgen.h>
>>  #include "addr_location.h"
>>  #include "path.h"
>>  #include "symbol_conf.h"
>> --
>> 2.53.0
>>

- Arnaldo