Re: [PATCH v2 3/4] perf scripts: Add perf-stuck, to tell where a running perf is stuck

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] perf-stuck: gdb script can hang indefinitely via until command
- [Medium] perf-stuck: potential deadlock in gdb script via inferior calls
- [Medium] perf-stuck: gdb script crashes if map_symbol.sym is NULL
- [Low] perf-stuck: shell option injection on process name or log path
--

commit 2f3e215696579c8fa2b53359bc4b83edfc0dd017
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf scripts: Add perf-stuck, to tell where a running perf is stuck

This commit adds a prototype perf-stuck script and a gdb command file to
identify whether a running perf process is stuck in a loop or runaway recursion.
It samples the process via /proc and can optionally attach gdb to print DWARF
type chasing DIE chains.

> diff --git a/tools/perf/scripts/perf-stuck.gdb b/tools/perf/scripts/perf-stuck.gdb
> new file mode 100644
> index 0000000000000..ad595392f6beb
> --- /dev/null
> +++ b/tools/perf/scripts/perf-stuck.gdb
[ ... ]
> +define perf-die-chain
> +  if $argc < 2
> +    printf "usage: perf-die-chain <function> <die variable> [iterations]\n"
> +  else
> +    frame function $arg0
> +    if $argc == 3
> +      set $perf_die_chain_n = $arg2
> +    else
> +      set $perf_die_chain_n = 10
> +    end
> +    set $perf_die_chain_head = $pc
> +    set $perf_die_chain_i = 0
> +    while $perf_die_chain_i < $perf_die_chain_n
> +      # Pointer type DIEs have no DW_AT_name, so dwarf_diename() can
> +      # return NULL: printf %s of it would error out and abort this
> +      # batch script, handle it.
> +      set $perf_die_chain_name = (char *) dwarf_diename($arg1)
> +      printf "chain[%d] die=%p addr=%p cu=%p off=0x%lx tag=%d name=", $perf_die_chain_i, $arg1, $arg1->addr, $arg1->cu, ((Dwarf_Off) dwarf_dieoffset($arg1)), ((int) dwarf_tag($arg1))

[Severity: Medium]
Does evaluating dwarf_diename(), dwarf_dieoffset(), and dwarf_tag() risk
deadlocking the debugger? If the target process is already blocked holding
an internal library lock (such as a malloc or libdw lock), and the called
functions attempt to acquire the same lock via GDB's dummy stack frame, the
inferior thread will deadlock. In batch mode, this freezes the debugging
session permanently.

> +      if $perf_die_chain_name == 0
> +	printf "(null)\n"
> +      else
> +	printf "%s\n", $perf_die_chain_name
> +      end
> +      until *$perf_die_chain_head

[Severity: Medium]
Can this command cause the batch script to hang indefinitely? Since GDB
yields execution to the inferior expecting it to hit the saved program
counter, if the target process breaks out of its loop, returns from the
function, or blocks elsewhere, the address will never be reached. Because
GDB runs without a timeout in batch mode, this causes the debugger and
parent script to hang.

> +      set $perf_die_chain_i = $perf_die_chain_i + 1
> +    end
> +  end
> +end
[ ... ]
> +define perf-dso
> +  if $_any_caller_is("find_data_type", 20)
> +    frame function find_data_type
> +    printf "dso=%s ip=0x%lx sym=%s\n", dloc->ms->map->dso->name, dloc->ip, dloc->ms->sym->name

[Severity: Medium]
Will this crash the GDB script if ms->sym is NULL? It is common for an
address in perf tools to not resolve to a symbol, leaving ms->sym empty.
Dereferencing NULL here causes the batch script execution to abort
immediately with a memory access error, preventing further output.

> +  else
> +    printf "not in find_data_type()\n"
> +  end
> +end
> diff --git a/tools/perf/scripts/perf-stuck.sh b/tools/perf/scripts/perf-stuck.sh
> new file mode 100755
> index 0000000000000..3b9b22124dbda
> --- /dev/null
> +++ b/tools/perf/scripts/perf-stuck.sh
[ ... ]
> +if [[ "$1" =~ ^[0-9]+$ ]]; then
> +	pid=$1
> +else
> +	# Resolve the name against the caller's own processes: as root,
> +	# unscoped pgrep picks the first match of any user, e.g. one planted
> +	# to get gdb attached to it, use an explicit pid to look at a perf of
> +	# another user.
> +	pid=$(pgrep -x -u "$(id -u)" "$1" | head -1)

[Severity: Low]
Could this command fail or behave unexpectedly if the user provides a
process name starting with a hyphen? The $1 argument will be interpreted as
a command-line option by pgrep instead of a positional argument. A double
dash end-of-options separator might prevent this issue.

> +	[ -n "$pid" ] || { echo "no process named '$1' owned by $(id -un)"; exit 1; }
> +fi
[ ... ]
> +	progress=
> +	[ -n "$progress_log" ] && [ -s "$progress_log" ] && progress=$(tail -1 "$progress_log")

[Severity: Low]
Can this command also fail due to option injection if the progress log path
starts with a hyphen? Like pgrep above, tail will interpret the path as an
option if it is not separated from the arguments.

> +
> +	if [ -n "$prev_cpu" ]; then

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928171244.2437283-1-acme@kernel.org?part=3