On Mon, May 3, 2021 at 1:20 PM Grant Seltzer Richman
<grantseltzer@xxxxxxxxx> wrote:
>
> On Mon, May 3, 2021 at 2:43 PM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Mon, May 3, 2021 at 11:32 AM Grant Seltzer Richman
> > <grantseltzer@xxxxxxxxx> wrote:
> > >
> > > On Wed, Apr 28, 2021 at 5:15 PM Andrii Nakryiko
> > > <andrii.nakryiko@xxxxxxxxx> wrote:
> > > >
> > > > On Wed, Apr 28, 2021 at 1:53 PM Grant Seltzer Richman
> > > > <grantseltzer@xxxxxxxxx> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > I'm working on enabling CO:RE in a project I work on, tracee, and am
> > > > > running into the dilemma of missing macros that we previously were
> > > > > able to import from their various header files. I understand that
> > > > > macros don't make their way into BTF and therefore the generated
> > > > > vmlinux.h won't have them. However I can't import the various header
> > > > > files because of multiple-definition issues.
> > > >
> > > > Sadly, copy/pasting has been the only way so far.
> > > >
> > > > >
> > > > > Do people typically redefine each of these macros for their project?
> > > > > If so is there anything I should be careful of, such as architectural
> > > > > differences. Does anyone have creative ideas, even if not developed
> > > > > fully yet that I can possibly contribute to libbpf?
> > > >
> > > > We've discussed adding Clang built-in to detect if a specific type is
> > > > already defined and doing something like this in vmlinux.h:
> > > >
> > > > #if !__builtin_is_type_defined(struct task_struct)
> > > > struct task_struct {
> > > > ...
> > > > }
> > > > #endif
> > > >
> > > > And just do that for every struct, union, typedef. That would allow
> > > > vmlinux.h to co-exist (somewhat) with other types.
> > > >
> > > > Another alternative is to not use vmlinux.h and use just linux
> > > > headers, but mark necessary types with
> > > > __attribute__((preserve_access_index)) to make them CO-RE relocatable.
> > > > You can add that to existing types with the same pragma that vmlinux.h
> > > > uses.
> > >
> > > I'm attempting to try doing the above. I'm just replacing
> > > bpf_probe_read with bpf_core_read and not importing vmlinux.h, just
> > > all the kernel headers I need.
> >
> > Yes, that will work, bpf_core_read() uses preserve_access_index
> > built-in to achieve the same effect.
> >
> > >
> > > When you say "Add that to existing types with the same pragma that
> > > vmlinux.h uses", Should I be able to add the following to my bpf
> > > source file before importing my headers?
> > >
> > > ifndef BPF_NO_PRESERVE_ACCESS_INDEX
> > > #pragma clang attribute push (__attribute__((preserve_access_index)),
> > > apply_to = record)
> > > #endif
> > >
> > > and then pop the attribute at the bottom of the file, or after the
> > > header includes.
> >
> > Yeah, that's the idea and that's what vmlinux.h does for all its
> > structs. It doesn't add __attribute__((preserve_access_index)) after
> > each struct/union. So I wonder why you are getting those unknown
> > attribute errors. Can you paste an example?
>
> Here's a couple examples of the warnings:
>
> ```
> tracee/tracee.bpf.c:5:46: warning: unknown attribute
> 'preserve_access_index' ignored [-Wunknown-attributes]
> #pragma clang attribute push (__attribute__((preserve_access_index)),
> apply_to = record)
> ^
> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:185:1:
> note: when applied to this declaration
> struct ipv6_fl_socklist;
> ^
> tracee/tracee.bpf.c:5:46: warning: unknown attribute
> 'preserve_access_index' ignored [-Wunknown-attributes]
> #pragma clang attribute push (__attribute__((preserve_access_index)),
> apply_to = record)
> ^
> /lib/modules/5.10.21-200.fc33.x86_64/source/include/linux/ipv6.h:187:1:
> note: when applied to this declaration
> struct inet6_cork {
> ```
>
> after these warnings are emitted (it seems as if there's one for every
> data type, though I can't confirm), I get errors that look like this:
>
> ```
> tracee/tracee.bpf.c:445:22: error: nested
> builtin_preserve_access_index() not supported
> return READ_KERN(READ_KERN(task->thread_pid)->numbers[level].nr);
> ^
> tracee/tracee.bpf.c:206:27: note: expanded from macro 'READ_KERN'
> bpf_core_read(&_val, sizeof(_val), &ptr); \
> ```
> I believe this is just a result of the warnings above, but if you're
> curious it's what i'm doing here:
> https://github.com/aquasecurity/tracee/blob/core-experiment/tracee-ebpf/tracee/tracee.bpf.c#L204-L208
>
Looking at your Makefile, you are not using `clang -target bpf` to
compile BPF object files, which is probably what causes you trouble.
preserve_access_index is a BPF target-only attribute. There is no need
to do the legacy clang -emit-llvm | llc, especially when you are using
CO-RE.
> >
> > Also check that you use Clang that supports preserve_access_index, of course.
>
> I'm using clang 11.0 on Fedora 33. All dependencies appear properly
> installed (libelf, zlib, dwarves [provides pahole], llvm, llc,
> llvm-devel,...)
>
> >
> > >
> > > I've tried this and get a whole bunch of 'unknown attribute' warnings,
> > > leading me to believe that I either have something installed
> > > incorrectly or don't understand how to use clang attributes. Do I need
> > > to edit the types in the actual header files?
> >
> > No, the whole idea is to not touch original headers.
>
> Got it - that's good to know.
>
> >
> > >
> > > Thank you very very much for the help!
> > > - Grant
> > > >
> > > > >
> > > > > Thanks so much,
> > > > > Grant Seltzer
