Re: [RFC] perf subcmd: avoid crash in exclude_cmds when excludes is empty

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Wed, Sep 10, 2025 at 04:27:05PM +0800, hupu wrote:
> Dear Maintainers,
> 
> When cross-compiling perf tool for ARM64 (though the issue is actually
> platform-independent), I encountered a crash when running `perf help`
> if the perf binary name is not exactly "perf" or if multiple "perf-*"
> binaries exist in the same directory.
> 
> This patch is an attempt to fix the issue. I would like to discuss its
> reasonableness with you and see if this is the right direction.
> 
> 
> 1. Problem Reproduction
> =======================
> 
> Case 1: Binary named "perf-master"
> ----------------------------------
>   # /mnt/perf-master help
>   perf-master: help.c:122: exclude_cmds: Assertion `cmds->names[ci] ==
> NULL' failed.
>   Aborted
> 
> Case 2: Renaming to "perf"
> --------------------------
>   # mv perf-master perf
>   # /mnt/perf help
>   -> works as expected
>   usage: perf [--version] [--help] [OPTIONS] COMMAND [ARGS]
> 
>   The most commonly used perf commands are:
>     annotate        Read perf.data (created by perf record) and
> display annotated code
>     archive         Create archive with object files with build-ids
> found in perf.data file
>     bench           General framework for benchmark suites
>     buildid-cache   Manage build-id cache.
>     buildid-list    List the buildids in a perf.data file
>     c2c             Shared Data C2C/HITM Analyzer.
>     config          Get and set variables in a configuration file.
>     daemon          Run record sessions on background
>     data            Data file related processing
>     diff            Read perf.data files and display the differential profile
>     evlist          List the event names in a perf.data file
>     ftrace          simple wrapper for kernel's ftrace functionality
>     inject          Filter to augment the events stream with
> additional information
>     iostat          Show I/O performance metrics
>     kallsyms        Searches running kernel for symbols
>     kvm             Tool to trace/measure kvm guest os
>     list            List all symbolic event types
>     mem             Profile memory accesses
>     record          Run a command and record its profile into perf.data
>     report          Read perf.data (created by perf record) and
> display the profile
>     script          Read perf.data (created by perf record) and
> display trace output
>     stat            Run a command and gather performance counter statistics
>     test            Runs sanity tests.
>     top             System profiling tool.
>     version         display the version of perf binary
>     probe           Define new dynamic tracepoints
>     trace           strace inspired tool
>     kmem            Tool to trace/measure kernel memory properties
>     kwork           Tool to trace/measure kernel work properties (latencies)
>     lock            Analyze lock events
>     sched           Tool to trace/measure scheduler properties (latencies)
>     timechart       Tool to visualize total system behavior during a workload
> 
>   See 'perf help COMMAND' for more information on a specific command.
> 
> Case 3: Copying to "perf-backup"
> --------------------------------
>   # cp perf perf-backup
>   # /mnt/perf help
>   perf: help.c:122: exclude_cmds: Assertion `cmds->names[ci] == NULL' failed.
>   Aborted
> 
>   # /mnt/perf-backup help
>   perf-backup: help.c:122: exclude_cmds: Assertion `cmds->names[ci] ==
> NULL' failed.
>   Aborted
> 
> 
> 
> 2. Root Cause
> =============
> 
> In `exclude_cmds()`, `cmds` represents commands found in the current
> binary directory and PATH, while `excludes` comes from the
> `~/libexec/perf-core/` directory.
> 
> The function attempts to remove commands from `cmds` that already
> exist in `excludes`. However, when `excludes->cnt == 0`, the final
> `assert(cmds->names[ci] == NULL)` may fail, leading to an abort.
> 
> 
> 
> 3. Fix
> ======
> 
> Add a simple guard at the beginning of `exclude_cmds()` to return
> early if `excludes->cnt == 0`.
> 
> diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> index 9ef569492560..ddaeb4eb3e24 100644
> --- a/tools/lib/subcmd/help.c
> +++ b/tools/lib/subcmd/help.c
> @@ -75,6 +75,9 @@ void exclude_cmds(struct cmdnames *cmds, struct
> cmdnames *excludes)
>         size_t ci, cj, ei;
>         int cmp;
> 
> +       if (!excludes->cnt)
> +               return;
> +
>         ci = cj = ei = 0;
>         while (ci < cmds->cnt && ei < excludes->cnt) {
>                 cmp = strcmp(cmds->names[ci]->name, excludes->names[ei]->name);
> 
> 
> 
> 4.Verification
> ===============
> 
> With this patch applied, running perf help works correctly even when
> the binary is named perf, perf-master, or perf-backup:
> 
> /mnt/perf help
> /mnt/perf-master help
> /mnt/perf-backup help
> 
> All of them now display the expected help information without aborting.

Thanks for the detailed report and the fix.  Looks good to me we don't
need to look up the cmdnames when there's nothing to exclude.

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung