bpf, rex: fix a NULL deref on program load under SELinux, and a bpf_attr ABI desync with tools/ - #2
Open
SeungJong-Ha wants to merge 2 commits into
Open
bpf, rex: fix a NULL deref on program load under SELinux, and a bpf_attr ABI desync with tools/#2SeungJong-Ha wants to merge 2 commits into
SeungJong-Ha wants to merge 2 commits into
Conversation
bpf_prog_load_rex() and bpf_prog_load_rex_base() free the LSM blob on
their error path but never allocate it. Neither calls
security_bpf_prog_load(), while upstream's bpf_prog_load() calls both:
bpf_prog_load() load hook + free hook
bpf_prog_load_rex() free hook only
bpf_prog_load_rex_base() free hook only
With CONFIG_SECURITY_SELINUX=y, prog->aux->security therefore stays NULL
and selinux_bpf_prog() dereferences it. The first Rex program load takes
the machine down:
BUG: kernel NULL pointer dereference, address: 0000000000000000
Oops: Oops: 0000 [#1] SMP
RIP: 0010:selinux_bpf_prog+0x1e/0x50
Call Trace:
bpf_prog_load_rex_base+0xb1c/0xe50
Kernel panic - not syncing: Fatal exception
scripts/q-script/.config has CONFIG_SECURITY_SELINUX off, which is why
this has not been hit here. On a kernel that enables it, no Rex program
can be loaded at all.
Add the call where bpf_prog_load() makes it: right after
bpf_obj_name_cpy(), branching to the same free_prog_sec label. token is
NULL because neither Rex path takes one.
Reproduced on this branch, on 905dbe3. CONFIG_SECURITY is already
set and CONFIG_LSM already lists selinux, so enabling
CONFIG_SECURITY_SELINUX in scripts/q-script/.config is the only change
needed -- it adds no field to struct bpf_prog_aux, so the same librex and
the same samples/bmc binary are used on both sides. Booted under
virtme-ng, loading samples/bmc via BPF_PROG_LOAD_REX_BASE:
before panic as above, on the first load
after loader stays up, program attaches to XDP,
xdp_tx_filter pins under /sys/fs/bpf
Fixes: fea5e30 ("BPF_PROG_LOAD_DJW")
Fixes: 94373bb ("implement subprog-loading functions to support multiple programs in the same file")
Assisted-by: Claude:claude-opus-5-1m
Signed-off-by: SeungJong Ha <engineer.jjhama@gmail.com>
Rex puts a 64-byte union into union bpf_attr's prog_load struct -- map_offs, dyn_relas, the symbol tables, rustfd and the base-prog pair -- in front of fd_array, and adds BPF_PROG_TYPE_REX_BASE to enum bpf_prog_type. Both went into include/uapi/linux/bpf.h only. The tools/ copy did get the new bpf_cmd values (81d257d), so libbpf can issue the Rex commands while laying bpf_attr out the old way. Every field from fd_array on is then at a different offset in libbpf than in the kernel: kernel tools (before) fd_array 200 136 core_relos 208 144 log_true_size 220 156 sizeof(bpf_attr) 248 184 libbpf writes fd_array at 136, the verifier reads 0 at 200, and any program calling a kfunc from a module is rejected before its first instruction: kfunc offset > 0 without fd_array is invalid failed to find BTF for kernel function The module itself is fine -- loaded, kfuncs registered, BTF present in /sys/kernel/btf -- which makes that message misleading. This affects every module kfunc on a Rex kernel; programs that use no module kfunc are unaffected, which is why it can hide for a long time. Copy both into tools/, which is what the kernel does with tools/ copies anyway. After this the two headers agree on all four offsets above. struct rex_rela_dyn, rex_dyn_sym and rex_text_sym stay out: they carry __user annotations that the tools/ copy uses nowhere, and they are separate types rather than part of union bpf_attr, so they do not affect its layout. Syncing them would need the annotations stripped, which is a separate decision. The trailing whitespace on the map_cnt line is dropped on both sides so the copies are identical rather than identical-modulo-whitespace. Fixes: 0fad382 ("map support") Assisted-by: Claude:claude-opus-5-1m Signed-off-by: SeungJong Ha <engineer.jjhama@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent fixes, found while running Rex's
samples/bmcas a fourth armnext to eBPF-BMC so that all arms could be measured on one kernel. Both defects
are in
rex-linuxat905dbe3and are unrelated to that measurement — they arereproduced below on your tree, with no source change other than the patches
themselves.
1/2 —
security_bpf_prog_load()is never called on the Rex load pathsbpf_prog_load_rex()andbpf_prog_load_rex_base()free the LSM blob on theirerror path but never allocate it:
bpf_prog_load()bpf_prog_load_rex()bpf_prog_load_rex_base()With
CONFIG_SECURITY_SELINUX=y,prog->aux->securitystays NULL andselinux_bpf_prog()dereferences it, so the first Rex program load takes themachine down.
Reproduction
scripts/q-script/.configalready setsCONFIG_SECURITY=yand already listsselinuxinCONFIG_LSM, so enablingCONFIG_SECURITY_SELINUXis the onlyconfig change needed. It adds no field to
struct bpf_prog_aux, so the samelibrex.soand the samesamples/bmcbinary are used on both sides. Bootedunder virtme-ng; the loader attaches
samples/bmcto a veth viaBPF_PROG_LOAD_REX_BASE.Before (
905dbe3,7.1.0-rex+):After (same tree + patch 1/2, same config, same binaries):
The fix adds the call where
bpf_prog_load()makes it — right afterbpf_obj_name_cpy(), branching to the samefree_prog_seclabel.tokenisNULL because neither Rex path takes one.
2/2 —
union bpf_attrwas never mirrored intotools/Rex adds a 64-byte union to
bpf_attr'sprog_loadstruct (map_offs,dyn_relas, the symbol tables,rustfd, the base-prog pair) in front offd_array, and addsBPF_PROG_TYPE_REX_BASEtoenum bpf_prog_type. Bothwent into
include/uapi/linux/bpf.honly. Every field after the insertion pointis then at a different offset in libbpf than in the kernel:
libbpf calls
bpf(cmd, attr, 184);bpf_check_uarg_tail_zero()accepts theshort struct and zero-fills the tail, so the kernel reads 0 at 200 rather
than garbage, and the pointer libbpf wrote at 136 lands inside the Rex union
where the ordinary load path ignores it. It fails silently.
fd_arrayis how libbpf passes module BTF fds, so on a Rex kernel every BPFprogram that calls a kfunc provided by a module is rejected before its first
instruction (
kernel/bpf/verifier.c:2828):kfuncs in vmlinux BTF (offset 0) are unaffected, which is why this can hide.
This is not specific to Rex programs — it breaks plain eBPF programs on a Rex
kernel too.
How it surfaced
In a 4-arm BMC benchmark, the arm that uses no kfunc (
baseline, plaineBPF-BMC) loaded and served at a 92% cache hit rate, while the two arms calling
a kfunc from our own module fell through to userspace memcached at 0% hit. The
module was fine — loaded, kfuncs registered, BTF present in
/sys/kernel/btf—which is what makes "failed to find BTF for kernel function" point the wrong
way.
Why it lasted
tools/include/uapi/linux/bpf.his a hand-maintained copy. In the 146 commitson
rex-linuxit was touched exactly once, by81d257d1fabc("rex/termination: Code for IPI based termination"), which added only the
bpf_cmdvalues — the names libbpf has to compile against. The union landed inthe kernel header two years earlier, in
0fad382d08c7("map support",2022-04-15), and was never mirrored: nothing references those fields by name in
libbpf, so its absence is not a compile error, only a wrong layout.
tools/lib/bpf/Makefile:140does warn when the two copies drift, but the ruleends in
|| true, so it is one line in the middle of a kernel build.Scope, and what is deliberately left out
struct rex_rela_dyn,rex_dyn_symandrex_text_symare not synced.They carry
__userannotations that thetools/copy uses nowhere, and theyare separate types rather than part of
union bpf_attr, so they do not affectits layout. Syncing them needs the annotations stripped, which is your call.
map_cntline is dropped on both sides so thecopies end up identical rather than identical-modulo-whitespace.
mid-struct rather than appended after
fd_array_cnt. That means any libbpfbuilt against stock kernel headers — a distro
libbpfpackage, say — laysbpf_attrout the old way and is wrong on a Rex kernel, not just the in-treecopy this patch fixes. Appending instead would be backward compatible, but it
changes the layout librex writes, so it is left alone here.
Verification summary
kernel/bpf/syscall.o, and a full kernel withCONFIG_SECURITY_SELINUX=ytools/lib/bpf→libbpf.so.1.8.0905dbe3--strictThe one checkpatch warning in 2/2 is line length on the
rustfdline, copiedverbatim from
include/uapi/linux/bpf.h; shortening it would defeat the pointof the sync.
Both patches are also carried on a v7.2-based tree, where they build and where
the Rex loader attaches and serves; the runtime evidence for 2/2 (the 92% vs 0%
hit rate above) was collected there rather than on 7.1. The 7.1 evidence for 2/2
is the offset computation and the libbpf build.
Commits carry
Assisted-by:perDocumentation/process/coding-assistants.rst.