linux-kernel - Re: [PATCH v2 00/17] Support dynamic opening of capstone/llvm remove BUILD

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAP-5=fX2n4nCTcSXY9+jU--X010hS9Q-chBWcwEyDzEV05D=FQ@mail.gmail.com>
Date: Thu, 23 Jan 2025 15:36:14 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	Nathan Chancellor <nathan@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>, 
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
	Aditya Gupta <adityag@...ux.ibm.com>, "Steinar H. Gunderson" <sesse@...gle.com>, 
	Charlie Jenkins <charlie@...osinc.com>, Changbin Du <changbin.du@...wei.com>, 
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>, James Clark <james.clark@...aro.org>, 
	Kajol Jain <kjain@...ux.ibm.com>, Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, 
	Li Huafei <lihuafei1@...wei.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
	Andi Kleen <ak@...ux.intel.com>, Chaitanya S Prakash <chaitanyas.prakash@....com>, 
	linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org, 
	llvm@...ts.linux.dev, Song Liu <song@...nel.org>, bpf@...r.kernel.org
Subject: Re: [PATCH v2 00/17] Support dynamic opening of capstone/llvm remove BUILD_NONDISTRO

On Thu, Jan 23, 2025 at 1:59 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Tue, Jan 21, 2025 at 10:23:15PM -0800, Ian Rogers wrote:
> > Linking against libcapstone and libLLVM can be a significant increase
> > in dependencies and size of memory footprint. For something like `perf
> > record` the disassembler and addr2line functionality won't be
> > used. Support dynamically loading these libraries using dlopen and
> > then calling the appropriate functions found using dlsym.
>
> It's not clear from the description how you would use dlopen/dlsym.
> Based on an offline discussion, you want to leave the current linking
> model as is, and to support dlopen/dlsym when it's NOT detected at
> build-time, right?

Yep. Current behavior is no header file than these options fail, new
behavior is that we try to use dlopen/dlsym and fail if the dlopen
fails.

> For that, you need to carry some definitions of the functions and types
> for the used APIs.  But I'm not sure if it's right to carry them in the
> perf code base.

Right. I mention that here:
https://lore.kernel.org/lkml/CAP-5=fUhNuybCU-2_5EgcCwgwXnxvyFMvyhzKe=ZP1bssQwXHw@mail.gmail.com/
For LLVM we need 3 typedefs and 5 #defines, for capstone we need 2
structs and 5 enums (if we #ifdef some x86 only formatting code).

The problem in not carrying those definitions is:
1) if the header file isn't present a build won't support
LLVM/capstone even by dlopen - everything falls through to
objdump/addr2line that have known performance issues;
2) package maintainers either need to spot a warning message to
realize they've done this by having a missing header file (hard to
spot and brittle) or we require the build to fail and people without
capstone.h opt out of the build error with NO_CAPSTONE=1 - something
perf developers will probably not like;
3) the LLVM/capstone code needs #ifdefs and __maybe_unused to suppress
compiler warnings, or perhaps we have a minimal version of those
files, leading to extra code complexity.

I believe the approach here is no worse than what we do with vmlinux.h
for BPF code and is robust as depending on dlsym being able to look up
the function names. It is not perfect but I think it is more perfect
and less complex than the alternative.

> >
> > BUILD_NONDISTRO is used to build perf against the license incompatible
> > libbfd and libiberty libraries. As this has been opt-in for nearly 2
> > years, commit dd317df07207 ("perf build: Make binutil libraries opt
> > in"), remove the code to simplify the code base.
>
> This part can be a separate series.

Right, I posted it as a series here:
https://lore.kernel.org/lkml/20250111202851.1075338-1-irogers@google.com/
as mentioned in the v2 notes below. The issue was that Arnaldo pointed
out removing BUILD_NONDISTRO removed disassemble_bpf that had only
been implemented for libbfd. This series adds the LLVM/capstone
definitions built into the symbol__disassemble refactor. Merging that
series would conflict with this series, so I posted everything
together to avoid having series of patches depending upon one another.
I also wanted to check that what is in disasm.c in the end is
reasonable, which I believe it is with significantly reduced
ifdef-ery.

> >
> > The patch series:
> > 1) does some initial clean up;
> > 2) moves the capstone and LLVM code to their own C files,
> > 3) simplifies a little the capstone code;
>
> I like changes up to this in general.  Let me take a look at the
> patches.

Thanks,
Ian
Ian

> Thanks,
> Namhyung