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
>
> 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
