Re: [RFC 0/9] bpf: Add buildid check support

From: Alexei Starovoitov
Date: Thu Apr 05 2018 - 21:37:35 EST


On Thu, Apr 05, 2018 at 05:16:36PM +0200, Jiri Olsa wrote:
> hi,
> eBPF programs loaded for kprobes are allowed to read kernel
> internal structures. We check the provided kernel version
> to ensure that the program is loaded for the proper kernel.
>
> The problem is that the version check is not enough, because
> it only follows the version setup from kernel's Makefile.
> However, the internal kernel structures change based on the
> .config data, so in practise we have different kernels with
> same version.
>
> The eBPF kprobe program thus then get loaded for different
> kernel than it's been built for, get wrong data (silently)
> and provide misleading output.
>
> This patchset implements additional check in eBPF loading code
> on provided build ID (from kernel's elf image, .notes section
> GNU build ID) to ensure we load the eBPF program on correct
> kernel.
>
> Also available in here (based on bpf-next/master):
> https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> bpf/checksum
>
> This patchset consists of several changes:
>
> - adding CONFIG_BUILDID_H option that instructs the build
> to generate uapi header file with build ID data, that
> will be included by eBPF program
>
> - adding CONFIG_BPF_BUILDID_CHECK option and new bpf_attr
> field to allow build ID checking when loading the eBPF
> program
>
> - changing libbpf to read and pass build ID to the kernel
>
> - several small side fixes
>
> - example perf eBPF code in bpf-samples/bpf-stdout-example.c
> to show the build ID support/usage.
>
> # perf record -vv -e ./bpf-samples/bpf-stdout-example.c kill 2>&1 | grep buildid
> libbpf: section(7) buildid, size 21, link 0, flags 3, type=1
> libbpf: kernel buildid of ./bpf-samples/bpf-stdout-example.c is: 6e25edeb408513184e2753bebad25d42314501a0
>
> The buildid is provided the same way we provide kernel
> version, in a special "buildid" section:
>
> # cat ./bpf-samples/bpf-stdout-example.c
> ...
> #include <linux/buildid.h>
>
> char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA;
> ...
>
> where LINUX_BUILDID_DATA is defined in the generated buildid.h.
>
> please note it's an RFC ;-) any comments and suggestions are welcome

I think this is overkill.

We're very heavy users of kprobe+bpf. It's used for lots
of different cases and usage is constantly growing,
but I haven't seen a single case of :

> The eBPF kprobe program thus then get loaded for different
> kernel than it's been built for, get wrong data (silently)
> and provide misleading output.

but I saw plenty of the opposite. People pre-compile the program
and hack kernel version when they load, since they know in advance
that kprobe+bpf doesn't use any kernel specific things.
The existing kernel version check for kprobe+bpf is already annoying
to them.
This buildid check they can easily bypass the same way.
imo the whole thing just adds complexity and doesn't solve anything.
Sorry, this is a Nack.