Skip to content

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
rex-rs:rex-linuxfrom
SeungJong-Ha:rex-fixes
Open

bpf, rex: fix a NULL deref on program load under SELinux, and a bpf_attr ABI desync with tools/#2
SeungJong-Ha wants to merge 2 commits into
rex-rs:rex-linuxfrom
SeungJong-Ha:rex-fixes

Conversation

@SeungJong-Ha

Copy link
Copy Markdown

Two independent fixes, found while running Rex's samples/bmc as a fourth arm
next to eBPF-BMC so that all arms could be measured on one kernel. Both defects
are in rex-linux at 905dbe3 and are unrelated to that measurement — they are
reproduced 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 paths

bpf_prog_load_rex() and bpf_prog_load_rex_base() free the LSM blob on their
error path but never allocate it:

functionload hookfree hook
bpf_prog_load()yesyes
bpf_prog_load_rex()noyes
bpf_prog_load_rex_base()noyes

With CONFIG_SECURITY_SELINUX=y, prog->aux->security stays NULL and
selinux_bpf_prog() dereferences it, so the first Rex program load takes the
machine down.

Reproduction

scripts/q-script/.config already sets CONFIG_SECURITY=y and already lists
selinux in CONFIG_LSM, so enabling CONFIG_SECURITY_SELINUX is the only
config change needed. It adds no field to struct bpf_prog_aux, so the same
librex.so and the same samples/bmc binary are used on both sides.
Booted
under virtme-ng; the loader attaches samples/bmc to a veth via
BPF_PROG_LOAD_REX_BASE.

Before (905dbe3, 7.1.0-rex+):

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

After (same tree + patch 1/2, same config, same binaries):

loader_alive=yes xdp_on_veth1=1 tx_pinned=yes
libbpf: prog 'xdp_tx_filter': pinned at '/sys/fs/bpf/xdp_tx_filter'

The fix adds 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.

2/2 — union bpf_attr was never mirrored into tools/

Rex adds a 64-byte union to bpf_attr's prog_load struct (map_offs,
dyn_relas, the symbol tables, rustfd, 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. Every field after the insertion point
is then at a different offset in libbpf than in the kernel:

 kernel tools (before)
core_relo_cnt 132 132 <- last field in agreement
fd_array 200 136
core_relos 208 144
core_relo_rec_size 216 152
log_true_size 220 156
prog_token_fd 224 160
fd_array_cnt 228 164
sizeof(bpf_attr) 248 184

libbpf calls bpf(cmd, attr, 184); bpf_check_uarg_tail_zero() accepts the
short 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_array is how libbpf passes module BTF fds, so on a Rex kernel every BPF
program that calls a kfunc provided by a module is rejected before its first
instruction (kernel/bpf/verifier.c:2828):

kfunc offset > 0 without fd_array is invalid
failed to find BTF for kernel function

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, plain
eBPF-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.h is a hand-maintained copy. In the 146 commits
on rex-linux it was touched exactly once, by 81d257d1fabc
("rex/termination: Code for IPI based termination"), which added only the
bpf_cmd values — the names libbpf has to compile against. The union landed in
the 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:140 does warn when the two copies drift, but the rule
ends 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_sym and rex_text_sym are not synced.
    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 needs the annotations stripped, which is your call.
  • The trailing whitespace on the map_cnt line is dropped on both sides so the
    copies end up identical rather than identical-modulo-whitespace.
  • A deeper issue this patch does not address: the union is inserted
    mid-struct rather than appended after fd_array_cnt. That means any libbpf
    built against stock kernel headers — a distro libbpf package, say — lays
    bpf_attr out the old way and is wrong on a Rex kernel, not just the in-tree
    copy this patch fixes. Appending instead would be backward compatible, but it
    changes the layout librex writes, so it is left alone here.

Verification summary

1/22/2
buildskernel/bpf/syscall.o, and a full kernel with CONFIG_SECURITY_SELINUX=ytools/lib/bpflibbpf.so.1.8.0
runtime, on 905dbe3panic before / clean load after (above)offsets equal after the patch (table above)
checkpatch --strict0 errors, 0 warnings0 errors, 1 warning

The one checkpatch warning in 2/2 is line length on the rustfd line, copied
verbatim from include/uapi/linux/bpf.h; shortening it would defeat the point
of 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: per
Documentation/process/coding-assistants.rst.

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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@SeungJong-Ha