Skip to content

Commit d846d83

Browse files
borkmanngregkh
authored andcommitted
bpf: Fix ld_{abs,ind} failure path analysis in subprogs
[ Upstream commit ee86148 ] Usage of ld_{abs,ind} instructions got extended into subprogs some time ago via commit 09b28d7 ("bpf: Add abnormal return checks."). These are only allowed in subprograms when the latter are BTF annotated and have scalar return types. The code generator in bpf_gen_ld_abs() has an abnormal exit path (r0=0 + exit) from legacy cBPF times. While the enforcement is on scalar return types, the verifier must also simulate the path of abnormal exit if the packet data load via ld_{abs,ind} failed. This is currently not the case. Fix it by having the verifier simulate both success and failure paths, and extend it in similar ways as we do for tail calls. The success path (r0=unknown, continue to next insn) is pushed onto stack for later validation and the r0=0 and return to the caller is done on the fall-through side. Fixes: 09b28d7 ("bpf: Add abnormal return checks.") Reported-by: STAR Labs SG <info@starlabs.sg> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Link: https://lore.kernel.org/r/20260408191242.526279-2-daniel@iogearbox.net Signed-off-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 945816e commit d846d83

1 file changed

Lines changed: 31 additions & 2 deletions

File tree

kernel/bpf/verifier.c

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17943,6 +17943,23 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
1794317943
mark_reg_unknown(env, regs, BPF_REG_0);
1794417944
/* ld_abs load up to 32-bit skb data. */
1794517945
regs[BPF_REG_0].subreg_def = env->insn_idx + 1;
17946+
/*
17947+
* See bpf_gen_ld_abs() which emits a hidden BPF_EXIT with r0=0
17948+
* which must be explored by the verifier when in a subprog.
17949+
*/
17950+
if (env->cur_state->curframe) {
17951+
struct bpf_verifier_state *branch;
17952+
17953+
mark_reg_scratched(env, BPF_REG_0);
17954+
branch = push_stack(env, env->insn_idx + 1, env->insn_idx, false);
17955+
if (IS_ERR(branch))
17956+
return PTR_ERR(branch);
17957+
mark_reg_known_zero(env, regs, BPF_REG_0);
17958+
err = prepare_func_exit(env, &env->insn_idx);
17959+
if (err)
17960+
return err;
17961+
env->insn_idx--;
17962+
}
1794617963
return 0;
1794717964
}
1794817965

@@ -18815,7 +18832,12 @@ static int visit_gotox_insn(int t, struct bpf_verifier_env *env)
1881518832
return keep_exploring ? KEEP_EXPLORING : DONE_EXPLORING;
1881618833
}
1881718834

18818-
static int visit_tailcall_insn(struct bpf_verifier_env *env, int t)
18835+
/*
18836+
* Instructions that can abnormally return from a subprog (tail_call
18837+
* upon success, ld_{abs,ind} upon load failure) have a hidden exit
18838+
* that the verifier must account for.
18839+
*/
18840+
static int visit_abnormal_return_insn(struct bpf_verifier_env *env, int t)
1881918841
{
1882018842
static struct bpf_subprog_info *subprog;
1882118843
struct bpf_iarray *jt;
@@ -18850,6 +18872,13 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
1885018872
/* All non-branch instructions have a single fall-through edge. */
1885118873
if (BPF_CLASS(insn->code) != BPF_JMP &&
1885218874
BPF_CLASS(insn->code) != BPF_JMP32) {
18875+
if (BPF_CLASS(insn->code) == BPF_LD &&
18876+
(BPF_MODE(insn->code) == BPF_ABS ||
18877+
BPF_MODE(insn->code) == BPF_IND)) {
18878+
ret = visit_abnormal_return_insn(env, t);
18879+
if (ret)
18880+
return ret;
18881+
}
1885318882
insn_sz = bpf_is_ldimm64(insn) ? 2 : 1;
1885418883
return push_insn(t, t + insn_sz, FALLTHROUGH, env);
1885518884
}
@@ -18895,7 +18924,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
1889518924
if (bpf_helper_changes_pkt_data(insn->imm))
1889618925
mark_subprog_changes_pkt_data(env, t);
1889718926
if (insn->imm == BPF_FUNC_tail_call) {
18898-
ret = visit_tailcall_insn(env, t);
18927+
ret = visit_abnormal_return_insn(env, t);
1889918928
if (ret)
1890018929
return ret;
1890118930
}

0 commit comments

Comments
 (0)