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:

function load hook free hook
bpf_prog_load() yes yes
bpf_prog_load_rex() no yes
bpf_prog_load_rex_base() no yes

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/2 2/2
builds kernel/bpf/syscall.o, and a full kernel with CONFIG_SECURITY_SELINUX=y tools/lib/bpflibbpf.so.1.8.0
runtime, on 905dbe3 panic before / clean load after (above) offsets equal after the patch (table above)
checkpatch --strict 0 errors, 0 warnings 0 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 free to 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