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]
Date: Tue, 13 Feb 2024 11:39:54 +0800
From: Leo Yan <leo.yan@...ux.dev>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
	Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
	linux-perf-users@...r.kernel.org, Will Deacon <will@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	John Garry <john.g.garry@...cle.com>,
	Mike Leach <mike.leach@...aro.org>
Subject: Re: [PATCH] perf tools: Fixup module symbol end address properly

On Mon, Feb 12, 2024 at 03:33:22PM -0800, Namhyung Kim wrote:
> I got a strange error on ARM to fail on processing FINISHED_ROUND
> record.  It turned out that it was failing in symbol__alloc_hist()
> because the symbol size is too big.
> 
> When a sample is captured on a specific BPF program, it failed.  I've
> added a debug code and found the end address of the symbol is from
> the next module which is placed far way.
> 
>   ffff800008795778-ffff80000879d6d8: bpf_prog_1bac53b8aac4bc58_netcg_sock    [bpf]
>   ffff80000879d6d8-ffff80000ad656b4: bpf_prog_76867454b5944e15_netcg_getsockopt      [bpf]
>   ffff80000ad656b4-ffffd69b7af74048: bpf_prog_1d50286d2eb1be85_hn_egress     [bpf]   <---------- here
>   ffffd69b7af74048-ffffd69b7af74048: $x.5    [sha3_generic]
>   ffffd69b7af74048-ffffd69b7af740b8: crypto_sha3_init        [sha3_generic]
>   ffffd69b7af740b8-ffffd69b7af741e0: crypto_sha3_update      [sha3_generic]
> 
> The logic in symbols__fixup_end() just uses curr->start to update the
> prev->end.  But in this case, it won't work as it's too different.
> 
> I think ARM has a different kernel memory layout for modules and BPF
> than on x86.  Actually there's a logic to handle kernel and module
> boundary.  Let's do the same for symbols between different modules.

Even Arm32 and Arm64 kernel have different memory layout for modules
and kernel image.

eBPF program (JITed) should be allocated from the vmalloc region, for
Arm64, see bpf_jit_alloc_exec() in arch/arm64/net/bpf_jit_comp.c.

> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/symbol.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 35975189999b..9ebdb8e13c0b 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -248,14 +248,31 @@ void symbols__fixup_end(struct rb_root_cached *symbols, bool is_kallsyms)
>  		 * segment is very big.  Therefore do not fill this gap and do
>  		 * not assign it to the kernel dso map (kallsyms).
>  		 *
> +		 * Also BPF code can be allocated separately from text segments
> +		 * and modules.  So the last entry in a module should not fill
> +		 * the gap too.
> +		 *
>  		 * In kallsyms, it determines module symbols using '[' character
>  		 * like in:
>  		 *   ffffffffc1937000 T hdmi_driver_init  [snd_hda_codec_hdmi]
>  		 */
>  		if (prev->end == prev->start) {
> +			const char *prev_mod;
> +			const char *curr_mod;
> +
> +			if (!is_kallsyms) {
> +				prev->end = curr->start;
> +				continue;
> +			}
> +
> +			prev_mod = strchr(prev->name, '[');
> +			curr_mod = strchr(curr->name, '[');
> +
>  			/* Last kernel/module symbol mapped to end of page */
> -			if (is_kallsyms && (!strchr(prev->name, '[') !=
> -					    !strchr(curr->name, '[')))
> +			if (!prev_mod != !curr_mod)
> +				prev->end = roundup(prev->end + 4096, 4096);
> +			/* Last symbol in the previous module */
> +			else if (prev_mod && strcmp(prev_mod, curr_mod))

Should two consecutive moudles fall into this case? I think we need to assign
'prev->end = curr->start' for two two consecutive moudles.

If so, we should use a specific checking for eBPF program, e.g.:

                        else if (prev_mod && strcmp(prev_mod, curr_mod) &&
                                 (!strcmp(prev->name, "bpf") ||
                                  !strcmp(curr->name, "bpf")))

Thanks,
Leo

>  				prev->end = roundup(prev->end + 4096, 4096);
>  			else
>  				prev->end = curr->start;
> -- 
> 2.43.0.687.g38aa6559b0-goog
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ