netdev - Re: [PATCH] bpf: Hoise pahole version checks into Kconfig

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: <20210111193400.GA1343746@ubuntu-m3-large-x86>
Date:   Mon, 11 Jan 2021 12:34:00 -0700
From:   Nathan Chancellor <natechancellor@...il.com>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     Michal Marek <michal.lkml@...kovi.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Sedat Dilek <sedat.dilek@...il.com>
Subject: Re: [PATCH] bpf: Hoise pahole version checks into Kconfig

On Tue, Jan 12, 2021 at 04:19:01AM +0900, Masahiro Yamada wrote:
> On Tue, Jan 12, 2021 at 3:06 AM Nathan Chancellor
> <natechancellor@...il.com> wrote:
> >
> > After commit da5fb18225b4 ("bpf: Support pre-2.25-binutils objcopy for
> > vmlinux BTF"), having CONFIG_DEBUG_INFO_BTF enabled but lacking a valid
> > copy of pahole results in a kernel that will fully compile but fail to
> > link. The user then has to either install pahole or disable
> > CONFIG_DEBUG_INFO_BTF and rebuild the kernel but only after their build
> > has failed, which could have been a significant amount of time depending
> > on the hardware.
> >
> > Avoid a poor user experience and require pahole to be installed with an
> > appropriate version to select and use CONFIG_DEBUG_INFO_BTF, which is
> > standard for options that require a specific tools version.
> >
> > Suggested-by: Sedat Dilek <sedat.dilek@...il.com>
> > Signed-off-by: Nathan Chancellor <natechancellor@...il.com>
> 
> 
> 
> I am not sure if this is the right direction.
> 
> 
> I used to believe moving any tool test to the Kconfig
> was the right thing to do.
> 
> For example, I tried to move the libelf test to Kconfig,
> and make STACK_VALIDATION depend on it.
> 
> https://patchwork.kernel.org/project/linux-kbuild/patch/1531186516-15764-1-git-send-email-yamada.masahiro@socionext.com/
> 
> It was rejected.
> 
> 
> In my understanding, it is good to test target toolchains
> in Kconfig (e.g. cc-option, ld-option, etc).
> 
> As for host tools, in contrast, it is better to _intentionally_
> break the build in order to let users know that something needed is missing.
> Then, they will install necessary tools or libraries.
> It is just a one-time setup, in most cases,
> just running 'apt install' or 'dnf install'.
> 
> 
> 
> Recently, a similar thing happened to GCC_PLUGINS
> https://patchwork.kernel.org/project/linux-kbuild/patch/20201203125700.161354-1-masahiroy@kernel.org/#23855673
> 
> 
> 
> 
> Following this pattern, if a new pahole is not installed,
> it might be better to break the build instead of hiding
> the CONFIG option.
> 
> In my case, it is just a matter of 'apt install pahole'.
> On some distributions, the bundled pahole is not new enough,
> and people may end up with building pahole from the source code.

This is fair enough. However, I think that parts of this patch could
still be salvaged into something that fits this by making it so that if
pahole is not installed (CONFIG_PAHOLE_VERSION=0) or too old, the build
errors at the beginning, rather at the end. I am not sure where the best
place to put that check would be though.

Cheers,
Nathan