On Thu, Jul 7, 2022 at 1:56 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Jul 7, 2022 at 1:28 AM Yaniv Agman <yanivagman@xxxxxxxxx> wrote:
> >
> > בתאריך יום ה׳, 7 ביולי 2022 ב-3:48 מאת Andrii Nakryiko
> > <andrii@xxxxxxxxxx>:
> > >
> > > This RFC patch set is to gather feedback about new
> > > SEC("ksyscall") and SEC("kretsyscall") section definitions meant to simplify
> > > life of BPF users that want to trace Linux syscalls without having to know or
> > > care about things like CONFIG_ARCH_HAS_SYSCALL_WRAPPER and related arch-specific
> > > vs arch-agnostic __<arch>_sys_xxx vs __se_sys_xxx function names, calling
> > > convention woes ("nested" pt_regs), etc. All this is quite annoying to
> > > remember and care about as BPF user, especially if the goal is to write
> > > achitecture- and kernel version-agnostic BPF code (e.g., things like
> > > libbpf-tools, etc).
> > >
> > > By using SEC("ksyscall/xxx")/SEC("kretsyscall/xxx") user clearly communicates
> > > the desire to kprobe/kretprobe kernel function that corresponds to the
> > > specified syscall. Libbpf will take care of all the details of determining
> > > correct function name and calling conventions.
> > >
> > > This patch set also improves BPF_KPROBE_SYSCALL (and renames it to
> > > BPF_KSYSCALL to match SEC("ksyscall")) macro to take into account
> > > CONFIG_ARCH_HAS_SYSCALL_WRAPPER instead of hard-coding whether host
> > > architecture is expected to use syscall wrapper or not (which is less reliable
> > > and can change over time).
> > >
> >
> > Hi Andrii,
> > I would very much liked if there was such a macro, which will make
> > things easier for syscall tracing,
> > but I think that you can't assume that libbpf will have access to
> > kconfig files all the time.
> > For example, if running from a container and not mounting /boot (on
> > environments where the config file is in /boot), libbpf will fail to
> > load CONFIG_ARCH_HAS_SYSCALL_WRAPPER value and assume it to be not
> > defined.
> > Then, on any environment with a "new" kernel where the program runs
> > from a container, it will return the wrong argument values.
> > For this very reason we fall-back in [1] to assume
> > CONFIG_ARCH_HAS_SYSCALL_WRAPPER is defined, as in most environments it
> > will be.
> >
> > [1] https://github.com/aquasecurity/tracee/blob/0f28a2cc14b851308ebaa380d503dea9eaa67271/pkg/ebpf/initialization/kconfig.go#L37
> >
>
> I see, unfortunately without relying on
> CONFIG_ARCH_HAS_SYSCALL_WRAPPER on BPF side it's hard to make this
> correct in all kernel versions. One way would be to keep
> BPF_KPROBE_SYSCALL as is assuming syscall wrapper for x86, s390 and
> arm64, and add BPF_KSYSCALL() macro as I did here, which would depend
> on __kconfig, so in your situation it won't work. SEC("ksyscall") by
> itself will still work, though, if you find it useful.
>
I thought some more about this. This is the second such problem (first
being USDT detecting availability of BPF cookie support) where libbpf
on user-space side performs feature detection and BPF-side code has to
use slightly different feature detection for the same feature.
To solve both problems a bit more generically, I'm thinking to add few
fake __kconfig variable, just like libbpf does with
LINUX_VERSION_CODE. I'll carve out some "namespace" for
libbpf-provided feature detection (e.g., LINUX_HAS_BPF_COOKIE and
LINUX_HAS_SYSCALL_WRAPPER, or something along those lines), and libbpf
will fill them in just like we do with LINUX_VERSION_CODE. Without
actually requiring /proc/config.gz. Thoughts?
>
> > > It would be great to get feedback about the overall feature, but also I'd
> > > appreciate help with testing this, especially for non-x86_64 architectures.
> > >
> > > Cc: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > > Cc: Kenta Tada <kenta.tada@xxxxxxxx>
> > > Cc: Hengqi Chen <hengqi.chen@xxxxxxxxx>
> > >
> > > Andrii Nakryiko (3):
> > > libbpf: improve and rename BPF_KPROBE_SYSCALL
> > > libbpf: add ksyscall/kretsyscall sections support for syscall kprobes
> > > selftests/bpf: use BPF_KSYSCALL and SEC("ksyscall") in selftests
> > >
> > > tools/lib/bpf/bpf_tracing.h | 44 +++++--
> > > tools/lib/bpf/libbpf.c | 109 ++++++++++++++++++
> > > tools/lib/bpf/libbpf.h | 16 +++
> > > tools/lib/bpf/libbpf.map | 1 +
> > > tools/lib/bpf/libbpf_internal.h | 2 +
> > > .../selftests/bpf/progs/bpf_syscall_macro.c | 6 +-
> > > .../selftests/bpf/progs/test_attach_probe.c | 6 +-
> > > .../selftests/bpf/progs/test_probe_user.c | 27 +----
> > > 8 files changed, 172 insertions(+), 39 deletions(-)
> > >
> > > --
> > > 2.30.2
> > >
