linux-kernel - Re: [PATCH v6 7/8] perf: disasm: prefer debugging files in build-id cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3c2c5e34-2e26-4580-95dc-030504245872@intel.com>
Date: Wed, 11 Sep 2024 11:05:04 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Changbin Du <changbin.du@...wei.com>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Nathan Chancellor <nathan@...nel.org>
Cc: Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 "Liang, Kan" <kan.liang@...ux.intel.com>,
 Nick Desaulniers <ndesaulniers@...gle.com>, Bill Wendling
 <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
 llvm@...ts.linux.dev, Hui Wang <hw.huiwang@...wei.com>
Subject: Re: [PATCH v6 7/8] perf: disasm: prefer debugging files in build-id
 cache

On 25/07/24 05:15, Changbin Du wrote:
> The build-id cache might have both debugging and non-debugging files. Here
> we prefer the debugging version for annotation.

As I pointed out before, disassembling a different file
from the one that actually executed can have pitfalls.

If you want this, it needs to be optional, not the default.

But if you take the approach to remove vdso from the buildid
cache, and add back the debug version, then this patch would
not be needed for vdso.

> 
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Signed-off-by: Changbin Du <changbin.du@...wei.com>
> ---
>  tools/perf/util/disasm.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 6af9fbec3a95..5f66b3632770 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -1162,18 +1162,25 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>  	    !dso__is_kcore(dso))
>  		return SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX;
>  
> -	build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
> -	if (build_id_filename) {
> -		__symbol__join_symfs(filename, filename_size, build_id_filename);
> -		free(build_id_filename);
> -	} else {
> -		if (dso__has_build_id(dso))
> -			return ENOMEM;
> -		return fallback_filename(dso, filename, filename_size);
> -	}
> +	/* Prefer debugging file if exists, otherwise non-debugging one is used. */
> +	for (int i = 0; i < 2; i++) {
> +		build_id_filename = dso__build_id_filename(dso, NULL, 0, !i);
> +		if (build_id_filename) {
> +			__symbol__join_symfs(filename, filename_size, build_id_filename);
> +			free(build_id_filename);
> +		} else {
> +			if (dso__has_build_id(dso))
> +				return ENOMEM;
> +			return fallback_filename(dso, filename, filename_size);
> +		}
>  
> -	if (access(filename, R_OK))
> -		return fallback_filename(dso, filename, filename_size);
> +		if (!access(filename, R_OK))
> +			break;
> +		else if (i != 0) {
> +			/* nor debugging or non-debugging is found */
> +			return fallback_filename(dso, filename, filename_size);
> +		}
> +	}
>  
>  	if (dso__is_kcore(dso) || dso__is_vdso(dso))
>  		goto fallback;