Re: [PATCH RFC bpf-next 5/7] libbpf: Implement bpf_gen__struct_ops_update_elem() for light skeletons

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]

> libbpf: Implement bpf_gen__struct_ops_update_elem() for light skeletons

TL;DR: In gen_loader.c, the 8-byte store of a prog fd at a variable offset may
be rejected by the verifier on strict-alignment kernels (e.g. riscv64), so
light skeletons with struct_ops callbacks could fail to load with -EACCES.

> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> --- a/tools/lib/bpf/gen_loader.c
> +++ b/tools/lib/bpf/gen_loader.c

[ ... ]

> +void bpf_gen__struct_ops_update_elem(struct bpf_gen *gen, int map_idx,
> +				     struct btf *btf,
> +				     struct bpf_struct_ops *st_ops)
> +{

[ ... ]

> +		/* R2 = dynamic kernel member offset from off_slot */
> +		emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX_VALUE,
> +						 0, 0, 0, off_slot));
> +		emit(gen, BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, 0));
> +		emit(gen, BPF_JMP_IMM(BPF_JLE, BPF_REG_2,
> +				      KERN_VDATA_MAX_SIZE - sizeof(__u64), 2));
> +		emit(gen, BPF_MOV64_IMM(BPF_REG_7, -E2BIG));
> +		emit_check_err(gen);
> +
> +		/* R1 = kern_vdata_off + offset */
> +		emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX_VALUE,
> +						 0, 0, 0, st_ops->kern_vdata_off));
> +		emit(gen, BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_2));

[ ... ]

> +		/* Store prog_fd (R0) into kern_vdata + offset (R1) */
> +		emit(gen, BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0, 0));

Can this 8-byte store be rejected by the verifier on strict-alignment
configurations?

After the JLE check, R2 only has var_off = tnum_range(0, 4088), so the low
three bits are unknown.  R1 = blob + kern_vdata_off + R2 then goes through
check_ptr_alignment() -> check_generic_ptr_alignment():

    reg_off = tnum_add(reg->var_off, tnum_const(off));
    if (!tnum_is_aligned(reg_off, size)) {
            ... "misaligned %saccess off %s+%d size %d\n" ...
            return -EACCES;

strict is set for every program when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
is not enabled, which is the default on riscv64 (it is only selected by
RISCV_EFFICIENT_UNALIGNED_ACCESS, which depends on NONPORTABLE).  The light
skeleton loader is loaded without BPF_F_ANY_ALIGNMENT, so on such kernels a
light skeleton with a struct_ops callback would fail to load with -EACCES.

The other variable-offset blob access in gen_loader, emit_st_ops_member_val(),
passes the pointer to bpf_probe_read_kernel(), and helper memory arguments
are not alignment-checked.  Other direct blob stores use constant, 8-byte
aligned offsets.

Would masking the offset, e.g. BPF_ALU64_IMM(BPF_AND, BPF_REG_2, ~7), or
copying through bpf_probe_read_kernel() make the alignment provable?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42289585064