Re: [PATCH v2 02/10] perf tools: Try linked debug files for DSO debug info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf: DSO_BINARY_TYPE__DEBUGLINK bypasses mount namespace fallback
- [Low] perf: Incorrect path formatting in DEBUGLINK resolution for local files
--

commit 8a09d3bb34892b4472de63b4949a0752ebfa1103
Author: Namhyung Kim <namhyung@kernel.org>

perf tools: Try linked debug files for DSO debug info

The dso__find_dbginfo_type() should try DSO_BINARY_TYPE__DEBUGLINK so it
can follow the .gnu_debuglink and find the debug information in the
separate file.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index df03f84c97235..913877b84b2d4 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -2111,6 +2111,7 @@ void dso__find_dbginfo_type(struct dso *dso)
>  		DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
>  		DSO_BINARY_TYPE__BUILDID_DEBUGINFO,
>  		DSO_BINARY_TYPE__SYSTEM_PATH_DSO,
> +		DSO_BINARY_TYPE__DEBUGLINK,
[Severity: Medium]
Will this addition bypass the mount namespace fallback for containerized
workloads?

When dso__find_dbginfo_type() iterates through the types and calls
dso__get_filename() for DSO_BINARY_TYPE__DEBUGLINK, it performs direct file
I/O on the host path:

tools/perf/util/dso.c:dso__read_binary_type_filename() {
    ...
		if (!is_regular_file(filename)) {
			ret = -1;
			break;
		}
    ...
}

If the binary is containerized and not present on the host at that exact
path, this check fails and returns -1. This early error return causes
dso__get_filename() to exit via "goto out" before it can attempt the mount
namespace fallback logic:

tools/perf/util/dso.c:dso__get_filename() {
    ...
	if (dso__read_binary_type_filename(dso, type, root_dir, name, PATH_MAX))
		goto out;

	if (!is_regular_file(name)) {
        ...
		new_name = dso__filename_with_chroot(dso, name);
    ...
}

Can the early file existence check in dso__read_binary_type_filename() be
adjusted so it does not permanently bypass dso__filename_with_chroot()?

[Severity: Low]
Does this code incorrectly format fallback paths for local files?

In dso__read_binary_type_filename() for DSO_BINARY_TYPE__DEBUGLINK, the
directory component is extracted by searching backward for a slash:

tools/perf/util/dso.c:dso__read_binary_type_filename() {
    ...
		last_slash = filename + len;
		while (last_slash != filename && *last_slash != '/')
			last_slash--;

		strncpy(dso_dir, filename, last_slash - filename);
		dso_dir[last_slash-filename] = '\0';
    ...
}

If a DSO is specified without a path separator (e.g., a local file like
"vmlinux" in the current directory), last_slash remains equal to filename,
leaving dso_dir as an empty string.

Later in the function, debuglink_paths[i] evaluates the formats:

tools/perf/util/dso.c:dso__read_binary_type_filename() {
    ...
		for (i = 0; i < ARRAY_SIZE(debuglink_paths); i++) {
			snprintf(filename, size,
					debuglink_paths[i], dso_dir, symfile);
    ...
}

When evaluating formats like "%s/.debug/%s" against an empty dso_dir, doesn't
this incorrectly generate an absolute path starting from the root directory
(e.g., "/.debug/symfile") instead of a relative path from the current
directory?

>  		DSO_BINARY_TYPE__NOT_FOUND,
>  	};
>  	const enum dso_binary_type *type;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006234315.920817-1-namhyung@kernel.org?part=2