linux-kernel - Re: [PATCH 2/3] btf: move pahole check in scripts/link-vmlinux.sh to lib/Kconfig.debug

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: <56fbf243-8e50-4760-9ed0-8d1f0f7e5ed3@oracle.com>
Date: Wed, 11 Sep 2024 18:25:12 +0100
From: Alan Maguire <alan.maguire@...cle.com>
To: Masahiro Yamada <masahiroy@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>, bpf@...r.kernel.org
Cc: linux-arch@...r.kernel.org, Andrii Nakryiko <andrii@...nel.org>,
        linux-kernel@...r.kernel.org, Nathan Chancellor <nathan@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Nicolas Schier
 <nicolas@...sle.eu>, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH 2/3] btf: move pahole check in scripts/link-vmlinux.sh to
 lib/Kconfig.debug

On 11/09/2024 12:03, Masahiro Yamada wrote:
> When DEBUG_INFO_DWARF5 is selected, pahole 1.21+ is required to enable
> DEBUG_INFO_BTF.
> 
> When DEBUG_INFO_DWARF4 or DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT is selected,
> DEBUG_INFO_BTF can be enabled without pahole installed, but a build error
> will occur in scripts/link-vmlinux.sh:
> 
>     LD      .tmp_vmlinux1
>   BTF: .tmp_vmlinux1: pahole (pahole) is not available
>   Failed to generate BTF for vmlinux
>   Try to disable CONFIG_DEBUG_INFO_BTF
> 
> We did not guard DEBUG_INFO_BTF by PAHOLE_VERSION when previously
> discussed [1].
> 
> However, commit 613fe1692377 ("kbuild: Add CONFIG_PAHOLE_VERSION")
> added CONFIG_PAHOLE_VERSION at all. Now several CONFIG options, as
> well as the combination of DEBUG_INFO_BTF and DEBUG_INFO_DWARF5, are
> guarded by PAHOLE_VERSION.
> 
> The remaining compile-time check in scripts/link-vmlinux.sh now appears
> to be an awkward inconsistency.
> 
> This commit adopts Nathan's original work.
> 
> [1]: https://lore.kernel.org/lkml/20210111180609.713998-1-natechancellor@gmail.com/
> 
> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>

Nice cleanup! For the series

Reviewed-by: Alan Maguire <alan.maguire@...cle.com>

One small thing below..

> ---
> 
>  lib/Kconfig.debug       |  3 ++-
>  scripts/link-vmlinux.sh | 12 ------------
>  2 files changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 5e2f30921cb2..eff408a88dfd 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -379,12 +379,13 @@ config DEBUG_INFO_BTF
>  	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
>  	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
>  	depends on BPF_SYSCALL
> +	depends on PAHOLE_VERSION >= 116
>  	depends on !DEBUG_INFO_DWARF5 || PAHOLE_VERSION >= 121
>  	# pahole uses elfutils, which does not have support for Hexagon relocations
>  	depends on !HEXAGON
>  	help
>  	  Generate deduplicated BTF type information from DWARF debug info.
> -	  Turning this on expects presence of pahole tool, which will convert
> +	  Turning this on requires presence of pahole tool, which will convert
>  	  DWARF type info into equivalent deduplicated BTF type info.
>  

One thing we lose from the change below is an explicit message about the
minimal pahole version required. While it is codified in the
dependendencies, given that we used to loudly warn about this,
would it make sense to note it in the help text here; just a sentence
like "BTF generation requires pahole v1.16 or later."? Thanks!

Alan

>  config PAHOLE_HAS_SPLIT_BTF
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index cfffc41e20ed..53bd4b727e21 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -111,20 +111,8 @@ vmlinux_link()
>  # ${1} - vmlinux image
>  gen_btf()
>  {
> -	local pahole_ver
>  	local btf_data=${1}.btf.o
>  
> -	if ! [ -x "$(command -v ${PAHOLE})" ]; then
> -		echo >&2 "BTF: ${1}: pahole (${PAHOLE}) is not available"
> -		return 1
> -	fi
> -
> -	pahole_ver=$(${PAHOLE} --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
> -	if [ "${pahole_ver}" -lt "116" ]; then
> -		echo >&2 "BTF: ${1}: pahole version $(${PAHOLE} --version) is too old, need at least v1.16"
> -		return 1
> -	fi
> -
>  	info BTF "${btf_data}"
>  	LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1}
>