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
| ||
|
Message-ID: <0de3722d-7111-246c-b558-b26d032dc5d0@meta.com> Date: Thu, 4 May 2023 09:48:23 -0700 From: Yonghong Song <yhs@...a.com> To: Michal Suchánek <msuchanek@...e.de>, Quentin Monnet <quentin@...valent.com> Cc: Andrii Nakryiko <andrii.nakryiko@...il.com>, Alexander Lobakin <aleksander.lobakin@...el.com>, Alexander Lobakin <alobakin@...lbox.org>, Alexei Starovoitov <ast@...nel.org>, Shung-Hsi Yu <shung-hsi.yu@...e.com>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Maciej Fijalkowski <maciej.fijalkowski@...el.com>, Song Liu <songliubraving@...com>, Kumar Kartikeya Dwivedi <memxor@...il.com>, bpf@...r.kernel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 bpf 02/11] bpftool: define a local bpf_perf_link to fix accessing its fields On 5/4/23 1:18 AM, Michal Suchánek wrote: > Hello, > > On Thu, May 04, 2023 at 12:43:52AM +0100, Quentin Monnet wrote: >> On Fri, 21 Apr 2023 at 08:39, Michal Suchánek <msuchanek@...e.de> wrote: >>> >>> On Thu, Apr 20, 2023 at 04:07:38PM -0700, Andrii Nakryiko wrote: >>>> On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <msuchanek@...e.de> wrote: >>>>> >>>>> On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote: >>>>>> From: Michal Suchánek <msuchanek@...e.de> >>>>>> Date: Fri, 14 Apr 2023 11:54:57 +0200 >>>>>> >>>>>>> Hello, >>>>>> >>>>>> Hey-hey, >>>>>> >>>>>>> >>>>>>> On Thu, Apr 21, 2022 at 12:38:58AM +0000, Alexander Lobakin wrote: >>>>>>>> When building bpftool with !CONFIG_PERF_EVENTS: >>>>>>>> >>>>>>>> skeleton/pid_iter.bpf.c:47:14: error: incomplete definition of type 'struct bpf_perf_link' >>>>>>>> perf_link = container_of(link, struct bpf_perf_link, link); >>>>>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>>>>>> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of' >>>>>>>> ((type *)(__mptr - offsetof(type, member))); \ >>>>>>>> ^~~~~~~~~~~~~~~~~~~~~~ >>>>>>>> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:68:60: note: expanded from macro 'offsetof' >>>>>>>> #define offsetof(TYPE, MEMBER) ((unsigned long)&((TYPE *)0)->MEMBER) >>>>>>>> ~~~~~~~~~~~^ >>>>>>>> skeleton/pid_iter.bpf.c:44:9: note: forward declaration of 'struct bpf_perf_link' >>>>>>>> struct bpf_perf_link *perf_link; >>>>>>>> ^ >>>>>>>> >>>>>>>> &bpf_perf_link is being defined and used only under the ifdef. >>>>>>>> Define struct bpf_perf_link___local with the `preserve_access_index` >>>>>>>> attribute inside the pid_iter BPF prog to allow compiling on any >>>>>>>> configs. CO-RE will substitute it with the real struct bpf_perf_link >>>>>>>> accesses later on. >>>>>>>> container_of() is not CO-REd, but it is a noop for >>>>>>>> bpf_perf_link <-> bpf_link and the local copy is a full mirror of >>>>>>>> the original structure. >>>>>>>> >>>>>>>> Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output") >>>>>>> >>>>>>> This does not solve the problem completely. Kernels that don't have >>>>>>> CONFIG_PERF_EVENTS in the first place are also missing the enum value >>>>>>> BPF_LINK_TYPE_PERF_EVENT which is used as the condition for handling the >>>>>>> cookie. >>>>>> >>>>>> Sorry, I haven't been working with my home/private stuff for more than a >>>>>> year already. I may get back to it some day when I'm tired of Lua (curse >>>>>> words, sorry :D), but for now the series is "a bit" abandoned. >>>>> >>>>> This part still appllies and works for me with the caveat that >>>>> BPF_LINK_TYPE_PERF_EVENT also needs to be defined. >>>>> >>>>>> I think there was alternative solution proposed there, which promised to >>>>>> be more flexible. But IIRC it also doesn't touch the enum (was it added >>>>>> recently? Because it was building just fine a year ago on config without >>>>>> perf events). >>>>> >>>>> It was added in 5.15. Not sure there is a kernel.org LTS kernel usable >>>>> for CO-RE that does not have it, technically 5.4 would work if it was >>>>> built monolithic, it does not have module BTF, only kernel IIRC. >>>>> >>>>> Nonetheless, the approach to handling features completely missing in the >>>>> running kernel should be figured out one way or another. I would be >>>>> surprised if this was the last feature to be added that bpftool needs to >>>>> know about. >>>> >>>> Are we talking about bpftool built from kernel sources or from Github? >>>> Kernel source version should have access to latest UAPI headers and so >>>> BPF_LINK_TYPE_PERF_EVENT should be available. Github version, if it >>>> doesn't do that already, can use UAPI headers distributed (and used >>>> for building) with libbpf through submodule. >>> >>> It does have a copy of the uapi headers but apparently does not use >>> them. Using them directly might cause conflict with vmlinux.h, though. >> >> Indeed, using the UAPI header here conflicts with vmlinux.h. >> >> Looking again at some code I started last year but never finalised, I >> used the following approach, redefining BPF_LINK_TYPE_PERF_EVENT with >> CO-RE: >> >> enum bpf_link_type___local { >> BPF_LINK_TYPE_PERF_EVENT___local = 7, >> }; > > That's the same as I did except I used simple define instead of this > fake enum. > > The enum only has value when it is complete and the compiler can check > that a switch uses only known values, and can confuse things when values > are missing. Currently, enum value CORE is done though a llvm builtin function. So if the enum value is used in switch cases like switch(...) case BPF_LINK_TYPE_PERF_EVENT: ... CORE relocation will not work in that case since the compiler expects BPF_LINK_TYPE_PERF_EVENT to be a constant. > Thanks > > Michal
Powered by blists - more mailing lists