Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global subprogs in RCU/{preempt,irq}-disabled sections #8584

Open
wants to merge 3 commits into
base: bpf-next_base
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,7 @@ struct bpf_prog_aux {
bool jits_use_priv_stack;
bool priv_stack_requested;
bool changes_pkt_data;
bool might_sleep;
u64 prog_array_member_cnt; /* counts how many times as member of prog_array */
struct mutex ext_mutex; /* mutex for is_extended and prog_array_member_cnt */
struct bpf_arena *arena;
Expand Down
1 change: 1 addition & 0 deletions include/linux/bpf_verifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ struct bpf_subprog_info {
/* true if bpf_fastcall stack region is used by functions that can't be inlined */
bool keep_fastcall_stack: 1;
bool changes_pkt_data: 1;
bool might_sleep: 1;

enum priv_stack_mode priv_stack_mode;
u8 arg_cnt;
Expand Down
62 changes: 48 additions & 14 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -10317,23 +10317,18 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
if (subprog_is_global(env, subprog)) {
const char *sub_name = subprog_name(env, subprog);

/* Only global subprogs cannot be called with a lock held. */
if (env->cur_state->active_locks) {
verbose(env, "global function calls are not allowed while holding a lock,\n"
"use static function instead\n");
return -EINVAL;
}

/* Only global subprogs cannot be called with preemption disabled. */
if (env->cur_state->active_preempt_locks) {
verbose(env, "global function calls are not allowed with preemption disabled,\n"
"use static function instead\n");
return -EINVAL;
}

if (env->cur_state->active_irq_id) {
verbose(env, "global function calls are not allowed with IRQs disabled,\n"
"use static function instead\n");
if (env->subprog_info[subprog].might_sleep &&
(env->cur_state->active_rcu_lock || env->cur_state->active_preempt_locks ||
env->cur_state->active_irq_id || !in_sleepable(env))) {
verbose(env, "global functions that may sleep are not allowed in non-sleepable context,\n"
"i.e., in a RCU/IRQ/preempt-disabled section, or in\n"
"a non-sleepable BPF program context\n");
return -EINVAL;
}

Expand Down Expand Up @@ -16703,6 +16698,14 @@ static void mark_subprog_changes_pkt_data(struct bpf_verifier_env *env, int off)
subprog->changes_pkt_data = true;
}

static void mark_subprog_might_sleep(struct bpf_verifier_env *env, int off)
{
struct bpf_subprog_info *subprog;

subprog = find_containing_subprog(env, off);
subprog->might_sleep = true;
}

/* 't' is an index of a call-site.
* 'w' is a callee entry point.
* Eventually this function would be called when env->cfg.insn_state[w] == EXPLORED.
Expand All @@ -16716,6 +16719,7 @@ static void merge_callee_effects(struct bpf_verifier_env *env, int t, int w)
caller = find_containing_subprog(env, t);
callee = find_containing_subprog(env, w);
caller->changes_pkt_data |= callee->changes_pkt_data;
caller->might_sleep |= callee->might_sleep;
}

/* non-recursive DFS pseudo code
Expand Down Expand Up @@ -17183,9 +17187,20 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
mark_prune_point(env, t);
mark_jmp_point(env, t);
}
if (bpf_helper_call(insn) && bpf_helper_changes_pkt_data(insn->imm))
mark_subprog_changes_pkt_data(env, t);
if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
if (bpf_helper_call(insn)) {
const struct bpf_func_proto *fp;

ret = get_helper_proto(env, insn->imm, &fp);
/* If called in a non-sleepable context program will be
* rejected anyway, so we should end up with precise
* sleepable marks on subprogs, except for dead code
* elimination.
*/
if (ret == 0 && fp->might_sleep)
mark_subprog_might_sleep(env, t);
if (bpf_helper_changes_pkt_data(insn->imm))
mark_subprog_changes_pkt_data(env, t);
} else if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
struct bpf_kfunc_call_arg_meta meta;

ret = fetch_kfunc_meta(env, insn, &meta, NULL);
Expand All @@ -17204,6 +17219,13 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
*/
mark_force_checkpoint(env, t);
}
/* Same as helpers, if called in a non-sleepable context
* program will be rejected anyway, so we should end up
* with precise sleepable marks on subprogs, except for
* dead code elimination.
*/
if (ret == 0 && is_kfunc_sleepable(&meta))
mark_subprog_might_sleep(env, t);
}
return visit_func_call_insn(t, insns, env, insn->src_reg == BPF_PSEUDO_CALL);

Expand Down Expand Up @@ -17320,6 +17342,7 @@ static int check_cfg(struct bpf_verifier_env *env)
}
ret = 0; /* cfg looks good */
env->prog->aux->changes_pkt_data = env->subprog_info[0].changes_pkt_data;
env->prog->aux->might_sleep = env->subprog_info[0].might_sleep;

err_free:
kvfree(insn_state);
Expand Down Expand Up @@ -20845,6 +20868,7 @@ static int jit_subprogs(struct bpf_verifier_env *env)
func[i]->aux->tail_call_reachable = env->subprog_info[i].tail_call_reachable;
func[i]->aux->exception_cb = env->subprog_info[i].is_exception_cb;
func[i]->aux->changes_pkt_data = env->subprog_info[i].changes_pkt_data;
func[i]->aux->might_sleep = env->subprog_info[i].might_sleep;
if (!i)
func[i]->aux->exception_boundary = env->seen_exception;
func[i] = bpf_int_jit_compile(func[i]);
Expand Down Expand Up @@ -22723,6 +22747,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
if (tgt_prog) {
struct bpf_prog_aux *aux = tgt_prog->aux;
bool tgt_changes_pkt_data;
bool tgt_might_sleep;

if (bpf_prog_is_dev_bound(prog->aux) &&
!bpf_prog_dev_bound_match(prog, tgt_prog)) {
Expand Down Expand Up @@ -22765,6 +22790,15 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
"Extension program changes packet data, while original does not\n");
return -EINVAL;
}

tgt_might_sleep = aux->func
? aux->func[subprog]->aux->might_sleep
: aux->might_sleep;
if (prog->aux->might_sleep && !tgt_might_sleep) {
bpf_log(log,
"Extension program may sleep, while original does not\n");
return -EINVAL;
}
}
if (!tgt_prog->jited) {
bpf_log(log, "Can attach to only JITed progs\n");
Expand Down
107 changes: 0 additions & 107 deletions tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c

This file was deleted.

3 changes: 3 additions & 0 deletions tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ static const char * const inproper_region_tests[] = {
"nested_rcu_region",
"rcu_read_lock_global_subprog_lock",
"rcu_read_lock_global_subprog_unlock",
"rcu_read_lock_sleepable_helper_global_subprog",
"rcu_read_lock_sleepable_kfunc_global_subprog",
"rcu_read_lock_sleepable_global_subprog_indirect",
};

static void test_inproper_region(void)
Expand Down
3 changes: 3 additions & 0 deletions tools/testing/selftests/bpf/prog_tests/spin_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ static struct {
{ "lock_id_mismatch_innermapval_mapval", "bpf_spin_unlock of different lock" },
{ "lock_global_subprog_call1", "global function calls are not allowed while holding a lock" },
{ "lock_global_subprog_call2", "global function calls are not allowed while holding a lock" },
{ "lock_global_sleepable_helper_subprog", "global function calls are not allowed while holding a lock" },
{ "lock_global_sleepable_kfunc_subprog", "global function calls are not allowed while holding a lock" },
{ "lock_global_sleepable_subprog_indirect", "global function calls are not allowed while holding a lock" },
};

static int match_regex(const char *pattern, const char *string)
Expand Down
Loading
Loading