bpf: Add buildid check support
Subsystems (8)
Jiri Olsa
reviewercover bpf: Add buildid check support
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: 6e25edeb408513184e2753bebad25d42314501a0The 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
thanks,
jirka
---
Jiri Olsa (9):
perf tools: Make read_build_id function public perf tools: Add fetch_kernel_buildid function
kbuild: Do not pass arguments to link-vmlinux.sh
kbuild: Add filechk2 function
bpf: Add CONFIG_BUILDID_H option
bpf: Add CONFIG_BPF_BUILDID_CHECK option
libbpf: Synchronize uapi bpf.h header
libbpf: Add support to attach buildid to program load perf tools: The buildid usage in example eBPF program
Makefile | 14 +++++++++++++- include/uapi/linux/bpf.h | 2 ++ init/Kconfig | 12 ++++++++++++ kernel/bpf/syscall.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- scripts/Kbuild.include | 24 ++++++++++++++++++++++++ scripts/Makefile | 1 + scripts/extract-buildid.c | 42 ++++++++++++++++++++++++++++++++++++++++++ tools/bpf/bpftool/Makefile | 5 ++++- tools/include/uapi/linux/bpf.h | 3 +++ tools/lib/bpf/bpf.c | 6 ++++-- tools/lib/bpf/bpf.h | 5 +++-- tools/lib/bpf/libbpf.c | 46 ++++++++++++++++++++++++++++++++++++++++------ tools/perf/bpf-samples/bpf-stdout-example.c | 42 ++++++++++++++++++++++++++++++++++++++++++ tools/perf/tests/bpf.c | 9 ++++++++- tools/perf/util/symbol-minimal.c | 50 ++------------------------------------------------ tools/perf/util/util.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/util.h | 6 ++++++ 17 files changed, 355 insertions(+), 62 deletions(-) create mode 100644 scripts/extract-buildid.c create mode 100644 tools/perf/bpf-samples/bpf-stdout-example.c
Jiri Olsa
reviewerPATCH 1/9 perf tools: Make read_build_id function public
And renaming it into parse_notes_buildid to be more precise and usable in following patches.
--- tools/perf/util/symbol-minimal.c | 50 ++-------------------------------------- tools/perf/util/util.c | 48 ++++++++++++++++++++++++++++++++++++++ tools/perf/util/util.h | 4 ++++ 3 files changed, 54 insertions(+), 48 deletions(-)
tools/perf/util/symbol-minimal.c +2 -48
@@ -24,53 +24,6 @@ static bool check_need_swap(int file_endian)return host_endian != file_endian; } -#define NOTE_ALIGN (sz) (((sz) + 3) & ~3) - -#define NT_GNU_BUILD_ID 3 - -static int read_build_id (void *note_data,size_t note_len,void *bf, -size_t size,bool need_swap) -{ -struct { -u32 n_namesz; -u32 n_descsz; -u32 n_type; - } *nhdr; -void *ptr; - - ptr = note_data; -while (ptr < (note_data + note_len)) { -const char *name; -size_t namesz, descsz; - - nhdr = ptr; -if (need_swap) { - nhdr->n_namesz =bswap_32 (nhdr->n_namesz); - nhdr->n_descsz =bswap_32 (nhdr->n_descsz); - nhdr->n_type =bswap_32 (nhdr->n_type); - } - - namesz =NOTE_ALIGN (nhdr->n_namesz); - descsz =NOTE_ALIGN (nhdr->n_descsz); - - ptr +=sizeof (*nhdr); - name = ptr; - ptr += namesz; -if (nhdr->n_type ==NT_GNU_BUILD_ID && - nhdr->n_namesz ==sizeof ("GNU" )) { -if (memcmp (name,"GNU" ,sizeof ("GNU" )) ==0 ) { -size_t sz =min (size, descsz); -memcpy (bf, ptr, sz); -memset (bf + sz,0 , size - sz); -return 0 ; - } - } - ptr += descsz; - } - -return -1 ; -} -int filename__read_debuglink (const char *filename__maybe_unused ,char *debuglink__maybe_unused ,size_t size__maybe_unused )
@@ -153,7 +106,8 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)if (fread (buf, buf_size,1 , fp) !=1 ) goto out_free; - ret =read_build_id (buf, buf_size, bf, size, need_swap); + ret =parse_notes_buildid (buf, buf_size, bf, size, + need_swap);if (ret ==0 ) ret = size;break ;
tools/perf/util/util.c +48 -0
54 lines not on this page · raw message
tools/perf/util/util.h +4 -0
@@ -47,6 +47,10 @@ extern int cacheline_size;int fetch_kernel_version (unsigned int *puint,char *str,size_t str_sz); + +int parse_notes_buildid (void *note_data,size_t note_len,void *bf, +size_t size,bool need_swap); +#define KVER_VERSION (x) (((x) >> 16) & 0xff)#define KVER_PATCHLEVEL (x) (((x) >> 8) & 0xff)#define KVER_SUBLEVEL (x) ((x) & 0xff)
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 2/9 perf tools: Add fetch_kernel_buildid function
In reply to Jiri OlsaAdding fetch_kernel_buildid helper function to
retrieve build id from running kernel. It will
be used in following patches.
--- tools/perf/util/util.c | 18 ++++++++++++++++++ tools/perf/util/util.h | 2 ++ 2 files changed, 20 insertions(+)
tools/perf/util/util.c +18 -0
@@ -495,6 +495,24 @@ int parse_notes_buildid(void *note_data, size_t note_len, void *bf,return -1 ; } +int fetch_kernel_buildid (char *buildid,int size) +{ +char path[PATH_MAX ], *buf; +size_t len; +int err; + +scnprintf (path,PATH_MAX ,"%s/kernel/notes" , +sysfs__mountpoint ()); + +if (filename__read_str (path, &buf, &len)) +return -EINVAL ; + + err =parse_notes_buildid (buf, len, buildid, size, false); + +free (buf); +return err; +} +const char *perf_tip (const char *dirpath) {struct strlist *tips;
tools/perf/util/util.h +2 -0
@@ -48,6 +48,8 @@ extern int cacheline_size;int fetch_kernel_version (unsigned int *puint,char *str,size_t str_sz); +int fetch_kernel_buildid (char *buildid,int size); +int parse_notes_buildid (void *note_data,size_t note_len,void *bf,size_t size,bool need_swap);
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 3/9 kbuild: Do not pass arguments to link-vmlinux.sh
In reply to Jiri OlsaThere's no need to pass LD* arguments to link-vmlinux.sh, because they are passed as variables. The only argument the link-vmlinux.sh supports is the 'clean' argument.
--- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
@@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link cmd_link-vmlinux = \ - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ; \ + $(CONFIG_SHELL) $< ; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
Signature · 1 line
2.13.6
Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh
2018-04-06 0:16 GMT+09:00 Jiri Olsa <[email protected]>:
Quoted 6 lines
There's no need to pass LD* arguments to
link-vmlinux.sh, because they are passed as variables. The only argument thelink-vmlinux.shsupports is the 'clean' argument.Signed-off-by Jiri Olsa <[email protected]>---
Wrong.
$(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
exist here so that any change in them
invokes scripts/linkk-vmlinux.sh
Quoted 16 lines
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Makefile +1
-1@@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link cmd_link-vmlinux = \ - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ; \ + $(CONFIG_SHELL) $< ; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
Signature · 2 lines
Best Regards Masahiro Yamada
Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh
Quoted 5 lines · Masahiro Yamada
2018-04-06 0:16 GMT+09:00 Jiri Olsa <[email protected]>:
Quoted 4 lines
There's no need to pass LD* arguments to
link-vmlinux.sh, because they are passed as variables. The only argument thelink-vmlinux.shsupports is the 'clean' argument.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Jiri Olsa <[email protected]>---
Wrong.
$(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
exist here so that any change in them
invokesscripts/linkk-vmlinux.sh
sry, I can't see that.. but it's just a side fix,
which is actually not needed for the rest
I'll check on more and address this separately
thanks,
jirka
Quoted 29 lines
Quoted 20 lines
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Makefile +1
-1@@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link cmd_link-vmlinux = \ - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ; \ + $(CONFIG_SHELL) $< ; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCESignature · 2 lines
Best Regards Masahiro Yamada
Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh
2018-04-06 3:59 GMT+09:00 Jiri Olsa <[email protected]>:
Quoted 11 lines · Masahiro Yamada
Quoted 10 lines
2018-04-06 0:16 GMT+09:00 Jiri Olsa <[email protected]>:
Quoted 6 lines
There's no need to pass LD* arguments to
link-vmlinux.sh, because they are passed as variables. The only argument thelink-vmlinux.shsupports is the 'clean' argument.Signed-off-by Jiri Olsa <[email protected]>---
Wrong.
Quoted 8 lines
Quoted 3 lines
$(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
exist here so that any change in them
invokesscripts/linkk-vmlinux.shsry, I can't see that.. but it's just a side fix,
which is actually not needed for the restI'll check on more and address this separately
The link command is recorded in .vmlinux.cmd then, it is checked by if_changed in the next rebuild because link-vmlinux is invoked by $(call if_changed,...)
+$(call if_changed,link-vmlinux)
Quoted 36 lines
thanks,
jirkaQuoted 19 lines
Quoted 19 lines
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Makefile +1
-1@@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link cmd_link-vmlinux = \ - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ; \ + $(CONFIG_SHELL) $< ; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)Quoted 10 lines
Makefile ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) · line 1033
Quoted 1 line
Makefile +0
-0vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE--
Best Regards
Masahiro Yamada--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Best Regards Masahiro Yamada
Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh
Quoted 16 lines · Masahiro Yamada
2018-04-06 3:59 GMT+09:00 Jiri Olsa <[email protected]>:
Quoted 15 lines · Masahiro Yamada
Quoted 13 lines
2018-04-06 0:16 GMT+09:00 Jiri Olsa <[email protected]>:
Quoted 6 lines
There's no need to pass LD* arguments to
link-vmlinux.sh, because they are passed as variables. The only argument thelink-vmlinux.shsupports is the 'clean' argument.Signed-off-by Jiri Olsa <[email protected]>---
Wrong.
$(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
exist here so that any change in them
invokesscripts/linkk-vmlinux.sh
Quoted 7 lines
Quoted 4 lines
sry, I can't see that.. but it's just a side fix,
which is actually not needed for the restI'll check on more and address this separately
The link command is recorded in .vmlinux.cmd
i see.. just for the sake command line,
thanks for explanation
jirka
Jiri Olsa
reviewerPATCH 4/9 kbuild: Add filechk2 function
In reply to Jiri OlsaAdding filechk2 function that has the same semantics as filechk, but it takes the target file from the 2nd argument instead of from the '$@' as in the filechk function.
This function is needed when we can't have separate target for the file, like in the following patch.
--- scripts/Kbuild.include | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
scripts/Kbuild.include +24 -0
@@ -67,6 +67,30 @@ define filechk fi endef +# filechk2 is used to check if the content of a generated file is updated. +# It follows the same logic as the filechk except instead of the $@ target +# variable, the checked file is passed in 2nd argument. +# +# Sample usage: +# define filechk_sample +# echo $KERNELRELEASE +# endef +# version.h : Makefile +# $(call filechk2,sample,version.h) +# endef +define filechk2 + $(Q)set -e; \ + $(kecho) ' CHK $(2)'; \ + mkdir -p $(dir $(2)); \ + $(filechk_$(1)) < $< > $(2).tmp; \ + if [ -r $(2) ] && cmp -s $(2) $(2).tmp; then \ + rm -f $(2).tmp; \ + else \ + $(kecho) ' UPD $(2)'; \ + mv -f $(2).tmp $(2); \ + fi +endef + ###### # gcc support functions # See documentation in Documentation/kbuild/makefiles.txt
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 5/9 bpf: Add CONFIG_BUILDID_H option
In reply to Jiri OlsaAdding CONFIG_BUILDID_H option that forces build to generate file with GNU build id value:
include/linux/buildid.h
It contains following macros:
#define LINUX_BUILDID_DATA "\x6c\x41\x0f\xea\xa9\x5d ... #define LINUX_BUILDID_SIZE 20
Those macros will be used in following patches to identify kernel in more precise way when loading eBPF program that can touch kernel internal structures.
There's new build output for the check and update
of the buildid.h:
$ make
...
LD vmlinux.o
MODPOST vmlinux.o
KSYM .tmp_kallsyms1.o
KSYM .tmp_kallsyms2.o
LD vmlinux
SORTEX vmlinux
SYSMAP System.map
CHK include/generated/uapi/linux/buildid.h
UPD include/generated/uapi/linux/buildid.h
...--- Makefile | 12 ++++++++++++ init/Kconfig | 3 +++ scripts/Makefile | 1 + scripts/extract-buildid.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+) create mode 100644 scripts/extract-buildid.c
Makefile +12 -0
@@ -1023,6 +1023,15 @@ endif include/generated/autoksyms.h: FORCE $(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh true +ifdef CONFIG_BUILDID_H +buildid_h := include/linux/buildid.h + +define filechk_buildid.h + buildid=`readelf -n $@ | grep 'Build ID' | sed -e 's/^.*Build ID: \(.*\)$$/\1/'`; \ + scripts/extract-buildid $$buildid +endef +endif + ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link
@@ -1032,6 +1041,9 @@ cmd_link-vmlinux = \ vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE +$(call if_changed,link-vmlinux) +ifdef CONFIG_BUILDID_H + +$(call filechk2,buildid.h,$(buildid_h)) +endif # Build samples along the rest of the kernel ifdef CONFIG_SAMPLES
init/Kconfig +3 -0
@@ -1386,6 +1386,9 @@ config KALLSYMS_BASE_RELATIVE # end of the "standard kernel features (expert users)" menu +config BUILDID_H + bool + # syscall, maps, verifier config BPF_SYSCALL bool "Enable bpf() system call"
scripts/Makefile +1 -0
@@ -19,6 +19,7 @@ hostprogs-$(CONFIG_ASN1) += asn1_compiler hostprogs-$(CONFIG_MODULE_SIG) += sign-file hostprogs-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += extract-cert hostprogs-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert +hostprogs-$(CONFIG_BUILDID_H) += extract-buildid HOSTCFLAGS_sortextable.o = -I$(srctree)/tools/include HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
scripts/extract-buildid.c new file +42 -0
42 lines not on this page · raw message
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 6/9 bpf: Add CONFIG_BPF_BUILDID_CHECK option
In reply to Jiri OlsaAdding CONFIG_BPF_BUILDID_CHECK option that forces kernel to check on provided build id when loading eBPF program. If the build id does not match the one in kernel the program fails to load.
Adding new field into struct bpf_attr. The kern_buildid points to the user memory that contains the buildid.
Kernel expose the notes section via sysfs, but there's currently no other use for kernel's buildid, so I needed to add new __init buildid_init function to parse it out.
--- include/uapi/linux/bpf.h | 2 ++ init/Kconfig | 9 ++++++ kernel/bpf/syscall.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 1 deletion(-)
include/uapi/linux/bpf.h +2 -0
@@ -308,6 +308,8 @@ union bpf_attr {* (context accesses, allowed helpers, etc). */ __u32 expected_attach_type; +/* Checked when prog_type=kprobe and CONFIG_BPF_BUILDID_CHECK. */ +__aligned_u64 kern_buildid; };struct {/* anonymous struct used by BPF_OBJ_* commands */
init/Kconfig +9 -0
@@ -1406,6 +1406,15 @@ config BPF_JIT_ALWAYS_ON Enables BPF JIT and removes BPF interpreter to avoid speculative execution of BPF instructions by the interpreter +config BPF_BUILDID_CHECK + bool "Check buildid for kprobe and tracepoint programs" + depends on BPF_SYSCALL + select BUILDID_H + default n + help + Enables BPF program load code to check on kernel Build ID + for kprobe programs. + config USERFAULTFD bool "Enable userfaultfd() system call" select ANON_INODES
kernel/bpf/syscall.c +83 -1
96 lines not on this page · raw message
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 7/9 libbpf: Synchronize uapi bpf.h header
In reply to Jiri OlsaSynchronize include/uapi/linux/bpf.h with tools version.
--- tools/include/uapi/linux/bpf.h | 3 +++ 1 file changed, 3 insertions(+)
tools/include/uapi/linux/bpf.h +3 -0
@@ -308,6 +308,8 @@ union bpf_attr {* (context accesses, allowed helpers, etc). */ __u32 expected_attach_type; +/* Checked when prog_type=kprobe and CONFIG_BPF_BUILDID_CHECK. */ +__aligned_u64 kern_buildid; };struct {/* anonymous struct used by BPF_OBJ_* commands */
@@ -864,6 +866,7 @@ enum bpf_func_id {/* BPF_FUNC_skb_set_tunnel_key flags. */ #define BPF_F_ZERO_CSUM_TX (1ULL << 1)#define BPF_F_DONT_FRAGMENT (1ULL << 2) +#define BPF_F_SEQ_NUMBER (1ULL << 3)/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and * BPF_FUNC_perf_event_read_value flags.
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 8/9 libbpf: Add support to attach buildid to program load
In reply to Jiri OlsaAdding support to retrieve buildid from elf's "buildid" section and passing it through to the load_program function to kernel bpf syscall.
Fixing perf use of the bpf_load_program function and linking in the vsprintf.o into bpftool to have the scnprintf function in.
--- tools/bpf/bpftool/Makefile | 5 ++++- tools/lib/bpf/bpf.c | 6 ++++-- tools/lib/bpf/bpf.h | 5 +++-- tools/lib/bpf/libbpf.c | 46 ++++++++++++++++++++++++++++++++++++++++------ tools/perf/tests/bpf.c | 9 ++++++++- 5 files changed, 59 insertions(+), 12 deletions(-)
tools/bpf/bpftool/Makefile +4 -1
@@ -75,11 +75,14 @@ include $(wildcard $(OUTPUT)*.d) all: $(OUTPUT)bpftool SRCS = $(wildcard *.c) -OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o +OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o $(OUTPUT)vsprintf.o $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c $(QUIET_CC)$(COMPILE.c) -MMD -o $@ $< +$(OUTPUT)vsprintf.o: $(srctree)/tools/lib/vsprintf.c + $(QUIET_CC)$(COMPILE.c) -MMD -o $@ $< + $(OUTPUT)bpftool: $(OBJS) $(LIBBPF) $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
tools/lib/bpf/bpf.c +4 -2
@@ -168,6 +168,7 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr, attr.log_size =0 ; attr.log_level =0 ; attr.kern_version = load_attr->kern_version; + attr.kern_buildid =ptr_to_u64 (load_attr->buildid);memcpy (attr.prog_name, load_attr->name,min (name_len,BPF_OBJ_NAME_LEN -1 ));
@@ -185,8 +186,8 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,int bpf_load_program (enum bpf_prog_type type,const struct bpf_insn *insns,size_t insns_cnt,const char *license, -__u32 kern_version,char *log_buf, -size_t log_buf_sz) +__u32 kern_version,const char *buildid, +char *log_buf,size_t log_buf_sz) {struct bpf_load_program_attr load_attr;
@@ -198,6 +199,7 @@ int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns, load_attr.insns_cnt = insns_cnt; load_attr.license = license; load_attr.kern_version = kern_version; + load_attr.buildid = buildid;return bpf_load_program_xattr (&load_attr, log_buf, log_buf_sz); }
tools/lib/bpf/bpf.h +3 -2
@@ -49,6 +49,7 @@ struct bpf_load_program_attr {size_t insns_cnt;const char *license;__u32 kern_version; +const char *buildid; };/* Recommend log buffer size */
@@ -57,8 +58,8 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr, char *log_buf, size_t log_buf_sz);int bpf_load_program (enum bpf_prog_type type,const struct bpf_insn *insns,size_t insns_cnt,const char *license, -__u32 kern_version,char *log_buf, -size_t log_buf_sz); +__u32 kern_version,const char *buildid, +char *log_buf,size_t log_buf_sz); int bpf_verify_program(enum bpf_prog_type type, const struct bpf_insn *insns, size_t insns_cnt, int strict_alignment, const char *license, __u32 kern_version,
tools/lib/bpf/libbpf.c +40 -6
106 lines not on this page · raw message
tools/perf/tests/bpf.c +8 -1
@@ -298,6 +298,7 @@ static int check_env(void)int err;unsigned int kver_int;char license[] ="GPL" ; +char buildid[64 ];struct bpf_insn insns[] = {BPF_MOV64_IMM (BPF_REG_0 ,1 ),
@@ -310,9 +311,15 @@ static int check_env(void)return err; } + err =fetch_kernel_buildid (buildid,sizeof (buildid)); +if (err) { +pr_debug ("Unable to get kernel buildid\n" ); +return err; + } + err =bpf_load_program (BPF_PROG_TYPE_KPROBE , insns,sizeof (insns) /sizeof (insns[0 ]), - license, kver_int,NULL ,0 ); + license, kver_int, buildid,NULL ,0 );if (err <0 ) {pr_err ("Missing basic BPF support, skip this test: %s\n" ,strerror (errno));
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 9/9 perf tools: The buildid usage in example eBPF program
In reply to Jiri OlsaThe bpf-samples/bpf-stdout-example.c demonstrates how to put the buildid data into eBPF program.
--- tools/perf/bpf-samples/bpf-stdout-example.c | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tools/perf/bpf-samples/bpf-stdout-example.c
tools/perf/bpf-samples/bpf-stdout-example.c new file +42 -0
@@ -0,0 +1,42 @@ +#include <uapi/linux/bpf.h> +#include <linux/buildid.h> + +#define SEC (NAME ) __attribute__((section(NAME), used)) + +char _license[] SEC("license") = "GPL"; +int _version SEC("version") = LINUX_VERSION_CODE; +char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA; + +static unsigned long long (*bpf_get_smp_processor_id)(void ) = + (void *) BPF_FUNC_get_smp_processor_id; +static int (*bpf_perf_event_output)(void *ctx,void *map, +unsigned long long flags,void *data, +int size) = + (void *) BPF_FUNC_perf_event_output; + +struct bpf_map_def { +unsigned int type; +unsigned int key_size; +unsigned int value_size; +unsigned int max_entries; +unsigned int map_flags; +unsigned int inner_map_idx; +unsigned int numa_node; +}; + +struct bpf_map_def SEC("maps") __bpf_stdout__ = { + .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY, + .key_size = sizeof(int), + .value_size = sizeof(u32), + .max_entries = __NR_CPUS__, +}; + +SEC("probe=sys_read") +int func (void *ctx) +{ +char output_str[] ="Raise a BPF event!" ; + +bpf_perf_event_output (ctx, &__bpf_stdout__,bpf_get_smp_processor_id (), + &output_str,sizeof (output_str)); +return 0 ; +}
Signature · 1 line
2.13.6
Quoted 52 lines · Jiri Olsa
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/checksumThis patchset consists of several changes:
- adding
CONFIG_BUILDID_Hoption that instructs the build to generate uapi header file with build ID data, that will be included by eBPF program- adding
CONFIG_BPF_BUILDID_CHECKoption and newbpf_attrfield 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.cto 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: 6e25edeb408513184e2753bebad25d42314501a0The 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>
Quoted 6 lines
char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA; ...where
LINUX_BUILDID_DATAis defined in the generatedbuildid.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 :
Quoted 3 lines
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.
Quoted 70 lines · Alexei Starovoitov
Quoted 52 lines · Jiri Olsa
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/checksumThis patchset consists of several changes:
- adding
CONFIG_BUILDID_Hoption that instructs the build to generate uapi header file with build ID data, that will be included by eBPF program- adding
CONFIG_BPF_BUILDID_CHECKoption and newbpf_attrfield 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.cto 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: 6e25edeb408513184e2753bebad25d42314501a0The 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>Quoted 6 lines
char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA; ...where
LINUX_BUILDID_DATAis defined in the generatedbuildid.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 :
Quoted 3 lines
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.
Quoted 5 lines
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.
perhaps verifier could detect this (via bpf_probe_read usage) and disable the version check automaticaly for such program?
and in the same way force the version check (or buildid when enabled) once the bpf_probe_read is detected
thanks,
jirka