Skip to content

Commit 3d6ada8

Browse files
Yonghong Songintel-lab-lkp
Yonghong Song
authored andcommitted
bpf: Warn with __bpf_trap() kfunc maybe due to uninitialized variable
Marc Suñé (Isovalent, part of Cisco) reported an issue where an uninitialized variable caused generating bpf prog binary code not working as expected. The reproducer is in [1] where the flags “-Wall -Werror” are enabled, but there is no warning as the compiler takes advantage of uninitialized variable to do aggressive optimization. The optimized code looks like below: ; { 0: bf 16 00 00 00 00 00 00 r6 = r1 ; bpf_printk("Start"); 1: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x0 ll 0000000000000008: R_BPF_64_64 .rodata 3: b4 02 00 00 06 00 00 00 w2 = 0x6 4: 85 00 00 00 06 00 00 00 call 0x6 ; DEFINE_FUNC_CTX_POINTER(data) 5: 61 61 4c 00 00 00 00 00 w1 = *(u32 *)(r6 + 0x4c) ; bpf_printk("pre ipv6_hdrlen_offset"); 6: 18 01 00 00 06 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x6 ll 0000000000000030: R_BPF_64_64 .rodata 8: b4 02 00 00 17 00 00 00 w2 = 0x17 9: 85 00 00 00 06 00 00 00 call 0x6 <END> The verifier will report the following failure: 9: (85) call bpf_trace_printk#6 last insn is not an exit or jmp The above verifier log does not give a clear hint about how to fix the problem and user may take quite some time to figure out that the issue is due to compiler taking advantage of uninitialized variable. In llvm internals, uninitialized variable usage may generate 'unreachable' IR insn and these 'unreachable' IR insns may indicate uninitialized variable impact on code optimization. So far, llvm BPF backend ignores 'unreachable' IR hence the above code is generated. With clang21 patch [2], those 'unreachable' IR insn are converted to func __bpf_trap(). In order to maintain proper control flow graph for bpf progs, [2] also adds an 'exit' insn after bpf_trap() if __bpf_trap() is the last insn in the function. The new code looks like: ; { 0: bf 16 00 00 00 00 00 00 r6 = r1 ; bpf_printk("Start"); 1: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x0 ll 0000000000000008: R_BPF_64_64 .rodata 3: b4 02 00 00 06 00 00 00 w2 = 0x6 4: 85 00 00 00 06 00 00 00 call 0x6 ; DEFINE_FUNC_CTX_POINTER(data) 5: 61 61 4c 00 00 00 00 00 w1 = *(u32 *)(r6 + 0x4c) ; bpf_printk("pre ipv6_hdrlen_offset"); 6: 18 01 00 00 06 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x6 ll 0000000000000030: R_BPF_64_64 .rodata 8: b4 02 00 00 17 00 00 00 w2 = 0x17 9: 85 00 00 00 06 00 00 00 call 0x6 10: 85 10 00 00 ff ff ff ff call -0x1 0000000000000050: R_BPF_64_32 __bpf_trap 11: 95 00 00 00 00 00 00 00 exit <END> In kernel, a new kfunc __bpf_trap() is added. During insn verification, any hit with __bpf_trap() will result in verification failure. The kernel is able to provide better log message for debugging. With llvm patch [2] and without this patch (no __bpf_trap() kfunc for existing kernel), e.g., for old kernels, the verifier outputs 10: <invalid kfunc call> kfunc '__bpf_trap' is referenced but wasn't resolved Basically, kernel does not support __bpf_trap() kfunc. This still didn't give clear signals about possible reason. With llvm patch [2] and with this patch, the verifier outputs 10: (85) call __bpf_trap#74479 unexpected __bpf_trap() due to uninitialized variable? It gives much better hints for verification failure. [1] https://github.com/msune/clang_bpf/blob/main/Makefile#L3 [2] llvm/llvm-project#131731 Signed-off-by: Yonghong Song <[email protected]>
1 parent f01453b commit 3d6ada8

File tree

2 files changed

+10
-0
lines changed

2 files changed

+10
-0
lines changed

kernel/bpf/helpers.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3273,6 +3273,10 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
32733273
local_irq_restore(*flags__irq_flag);
32743274
}
32753275

3276+
__bpf_kfunc void __bpf_trap(void)
3277+
{
3278+
}
3279+
32763280
__bpf_kfunc_end_defs();
32773281

32783282
BTF_KFUNCS_START(generic_btf_ids)
@@ -3386,6 +3390,7 @@ BTF_ID_FLAGS(func, bpf_copy_from_user_dynptr, KF_SLEEPABLE)
33863390
BTF_ID_FLAGS(func, bpf_copy_from_user_str_dynptr, KF_SLEEPABLE)
33873391
BTF_ID_FLAGS(func, bpf_copy_from_user_task_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
33883392
BTF_ID_FLAGS(func, bpf_copy_from_user_task_str_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
3393+
BTF_ID_FLAGS(func, __bpf_trap)
33893394
BTF_KFUNCS_END(common_btf_ids)
33903395

33913396
static const struct btf_kfunc_id_set common_kfunc_set = {

kernel/bpf/verifier.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12105,6 +12105,7 @@ enum special_kfunc_type {
1210512105
KF_bpf_res_spin_unlock,
1210612106
KF_bpf_res_spin_lock_irqsave,
1210712107
KF_bpf_res_spin_unlock_irqrestore,
12108+
KF___bpf_trap,
1210812109
};
1210912110

1211012111
BTF_ID_LIST(special_kfunc_list)
@@ -12170,6 +12171,7 @@ BTF_ID(func, bpf_res_spin_lock)
1217012171
BTF_ID(func, bpf_res_spin_unlock)
1217112172
BTF_ID(func, bpf_res_spin_lock_irqsave)
1217212173
BTF_ID(func, bpf_res_spin_unlock_irqrestore)
12174+
BTF_ID(func, __bpf_trap)
1217312175

1217412176
static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
1217512177
{
@@ -13641,6 +13643,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
1364113643
return err;
1364213644
}
1364313645
__mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32));
13646+
} else if (!insn->off && insn->imm == special_kfunc_list[KF___bpf_trap]) {
13647+
verbose(env, "unexpected __bpf_trap() due to uninitialized variable?\n");
13648+
return -EFAULT;
1364413649
}
1364513650

1364613651
if (is_kfunc_destructive(&meta) && !capable(CAP_SYS_BOOT)) {

0 commit comments

Comments
 (0)