Skip to content

Commit 201166d

Browse files
author
Alexei Starovoitov
committed
Merge branch 'fix-bpf_throw-global-subprogs-interaction'
Kumar Kartikeya Dwivedi says: ==================== Fix bpf_throw() vs global subprogs interaction There is a bug where bpf_throw()'s reachability across global subprogs is missed by the verifier, leading to successful verification when any kernel resource or lock is held across global subprog call boundary. Fix this by effect summarization like other related side effects and propagate exception reachability into callees. Changelog: ---------- v1 -> v2 v1: https://lore.kernel.org/bpf/20260516022426.2109698-1-memxor@gmail.com * Reorder might_throw bit to avoid bpf-next conflicts. (Alexei) ==================== Link: https://patch.msgid.link/20260517075530.3461166-1-memxor@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
2 parents a828abb + 511a5db commit 201166d

4 files changed

Lines changed: 53 additions & 7 deletions

File tree

include/linux/bpf_verifier.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ struct bpf_subprog_info {
729729
*/
730730
s16 fastcall_stack_off;
731731
bool has_tail_call: 1;
732+
bool might_throw: 1;
732733
bool tail_call_reachable: 1;
733734
bool has_ld_abs: 1;
734735
bool is_cb: 1;
@@ -1308,6 +1309,7 @@ void bpf_fmt_stack_mask(char *buf, ssize_t buf_sz, u64 stack_mask);
13081309
bool bpf_subprog_is_global(const struct bpf_verifier_env *env, int subprog);
13091310

13101311
int bpf_find_subprog(struct bpf_verifier_env *env, int off);
1312+
bool bpf_is_throw_kfunc(struct bpf_insn *insn);
13111313
int bpf_compute_const_regs(struct bpf_verifier_env *env);
13121314
int bpf_prune_dead_branches(struct bpf_verifier_env *env);
13131315
int bpf_check_cfg(struct bpf_verifier_env *env);

kernel/bpf/cfg.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,19 @@ static void mark_subprog_might_sleep(struct bpf_verifier_env *env, int off)
6464
subprog->might_sleep = true;
6565
}
6666

67+
static void mark_subprog_might_throw(struct bpf_verifier_env *env, int off)
68+
{
69+
struct bpf_subprog_info *subprog;
70+
71+
subprog = bpf_find_containing_subprog(env, off);
72+
subprog->might_throw = true;
73+
}
74+
6775
/* 't' is an index of a call-site.
6876
* 'w' is a callee entry point.
6977
* Eventually this function would be called when env->cfg.insn_state[w] == EXPLORED.
7078
* Rely on DFS traversal order and absence of recursive calls to guarantee that
71-
* callee's change_pkt_data marks would be correct at that moment.
79+
* callee's effect marks would be correct at that moment.
7280
*/
7381
static void merge_callee_effects(struct bpf_verifier_env *env, int t, int w)
7482
{
@@ -78,6 +86,7 @@ static void merge_callee_effects(struct bpf_verifier_env *env, int t, int w)
7886
callee = bpf_find_containing_subprog(env, w);
7987
caller->changes_pkt_data |= callee->changes_pkt_data;
8088
caller->might_sleep |= callee->might_sleep;
89+
caller->might_throw |= callee->might_throw;
8190
}
8291

8392
enum {
@@ -509,6 +518,8 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
509518
mark_subprog_might_sleep(env, t);
510519
if (ret == 0 && bpf_is_kfunc_pkt_changing(&meta))
511520
mark_subprog_changes_pkt_data(env, t);
521+
if (ret == 0 && bpf_is_throw_kfunc(insn))
522+
mark_subprog_might_throw(env, t);
512523
}
513524
return visit_func_call_insn(t, insns, env, insn->src_reg == BPF_PSEUDO_CALL);
514525

kernel/bpf/verifier.c

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,6 @@ static bool is_dynptr_ref_function(enum bpf_func_id func_id)
442442
static bool is_sync_callback_calling_kfunc(u32 btf_id);
443443
static bool is_async_callback_calling_kfunc(u32 btf_id);
444444
static bool is_callback_calling_kfunc(u32 btf_id);
445-
static bool is_bpf_throw_kfunc(struct bpf_insn *insn);
446445

447446
static bool is_bpf_wq_set_callback_kfunc(u32 btf_id);
448447
static bool is_task_work_add_kfunc(u32 func_id);
@@ -5405,7 +5404,7 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
54055404
if (bpf_pseudo_kfunc_call(insn + i) && !insn[i].off) {
54065405
bool err = false;
54075406

5408-
if (!is_bpf_throw_kfunc(insn + i))
5407+
if (!bpf_is_throw_kfunc(insn + i))
54095408
continue;
54105409
for (tmp = idx; tmp >= 0 && !err; tmp = dinfo[tmp].caller) {
54115410
if (subprog[tmp].is_cb) {
@@ -9499,6 +9498,9 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
94999498
return 0;
95009499
}
95019500

9501+
static int process_bpf_exit_full(struct bpf_verifier_env *env,
9502+
bool *do_print_state, bool exception_exit);
9503+
95029504
static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
95039505
int *insn_idx)
95049506
{
@@ -9552,6 +9554,17 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
95529554
caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
95539555
}
95549556

9557+
if (env->subprog_info[subprog].might_throw) {
9558+
struct bpf_verifier_state *branch;
9559+
9560+
branch = push_stack(env, *insn_idx + 1, *insn_idx, false);
9561+
if (IS_ERR(branch)) {
9562+
verbose(env, "failed to push state for global subprog exception path\n");
9563+
return PTR_ERR(branch);
9564+
}
9565+
return process_bpf_exit_full(env, NULL, true);
9566+
}
9567+
95559568
/* continue with next insn after call */
95569569
return 0;
95579570
}
@@ -11782,7 +11795,7 @@ static bool is_async_callback_calling_kfunc(u32 btf_id)
1178211795
is_task_work_add_kfunc(btf_id);
1178311796
}
1178411797

11785-
static bool is_bpf_throw_kfunc(struct bpf_insn *insn)
11798+
bool bpf_is_throw_kfunc(struct bpf_insn *insn)
1178611799
{
1178711800
return bpf_pseudo_kfunc_call(insn) && insn->off == 0 &&
1178811801
insn->imm == special_kfunc_list[KF_bpf_throw];
@@ -12972,8 +12985,6 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_ca
1297212985
}
1297312986

1297412987
static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name);
12975-
static int process_bpf_exit_full(struct bpf_verifier_env *env,
12976-
bool *do_print_state, bool exception_exit);
1297712988

1297812989
static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
1297912990
int *insn_idx_p)
@@ -13354,7 +13365,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
1335413365
if (meta.func_id == special_kfunc_list[KF_bpf_session_cookie])
1335513366
env->prog->call_session_cookie = true;
1335613367

13357-
if (is_bpf_throw_kfunc(insn))
13368+
if (bpf_is_throw_kfunc(insn))
1335813369
return process_bpf_exit_full(env, NULL, true);
1335913370

1336013371
return 0;

tools/testing/selftests/bpf/progs/exceptions_fail.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,28 @@ int reject_with_reference(void *ctx)
208208
return 0;
209209
}
210210

211+
__noinline int global_subprog_may_throw(struct __sk_buff *ctx)
212+
{
213+
if (ctx->len)
214+
bpf_throw(0);
215+
return 0;
216+
}
217+
218+
SEC("?tc")
219+
__failure __msg("Unreleased reference")
220+
int reject_global_subprog_throw_with_reference(struct __sk_buff *ctx)
221+
{
222+
struct foo *f;
223+
224+
f = bpf_obj_new(typeof(*f));
225+
if (!f)
226+
return 0;
227+
if (ctx->protocol)
228+
global_subprog_may_throw(ctx);
229+
bpf_obj_drop(f);
230+
return 0;
231+
}
232+
211233
__noinline static int subprog_ref(struct __sk_buff *ctx)
212234
{
213235
struct foo *f;

0 commit comments

Comments
 (0)