]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
bpf: Summarize sleepable global subprogs
authorKumar Kartikeya Dwivedi <memxor@gmail.com>
Sat, 1 Mar 2025 15:18:44 +0000 (07:18 -0800)
committerAlexei Starovoitov <ast@kernel.org>
Sat, 15 Mar 2025 18:48:25 +0000 (11:48 -0700)
The verifier currently does not permit global subprog calls when a lock
is held, preemption is disabled, or when IRQs are disabled. This is
because we don't know whether the global subprog calls sleepable
functions or not.

In case of locks, there's an additional reason: functions called by the
global subprog may hold additional locks etc. The verifier won't know
while verifying the global subprog whether it was called in context
where a spin lock is already held by the program.

Perform summarization of the sleepable nature of a global subprog just
like changes_pkt_data and then allow calls to global subprogs for
non-sleepable ones from atomic context.

While making this change, I noticed that RCU read sections had no
protection against sleepable global subprog calls, include it in the
checks and fix this while we're at it.

Care needs to be taken to not allow global subprog calls when regular
bpf_spin_lock is held. When resilient spin locks is held, we want to
potentially have this check relaxed, but not for now.

Also make sure extensions freplacing global functions cannot do so
in case the target is non-sleepable, but the extension is. The other
combination is ok.

Tests are included in the next patch to handle all special conditions.

Fixes: 9bb00b2895cb ("bpf: Add kfunc bpf_rcu_read_lock/unlock()")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20250301151846.1552362-2-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
include/linux/bpf.h
include/linux/bpf_verifier.h
kernel/bpf/verifier.c

index 15164787ce7f8dc3c4e0bdbd699a16733e979f40..1ee715983619a6b92b4446e335f17f35eb3bc85c 100644 (file)
@@ -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;
index bbd013c38ff9ffb87f52cc5a9324addd35f7cec3..d338f2a96bbae907dc84b7d2e1868868512f084a 100644 (file)
@@ -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;
index dcd0da4e62fca155fbd3d81c1ce5ca5c50c041fb..eb1624f6e743440ef8c65c73cf50dfa4df74f400 100644 (file)
@@ -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;
                }
 
@@ -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.
@@ -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
@@ -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);
@@ -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);
 
@@ -17320,6 +17342,7 @@ walk_cfg:
        }
        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);
@@ -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]);
@@ -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)) {
@@ -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");