bpf: correct stack liveness for tail calls

This updates bpf_insn_successors() reflecting that control flow might
jump over the instructions between tail call and function exit, verifier
might assume that some writes to parent stack always happen, which is
not the case.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Martin Teichmann <martin.teichmann@xfel.eu>
Link: https://lore.kernel.org/r/20251119160355.1160932-4-martin.teichmann@xfel.eu
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 5441341f1ab93157bf3be6dc8fe5082839f6cc28..8d0b60fa5f2be1ffdca5b863199435860b9d2a9c 100644 (file)
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -527,7 +527,6 @@ struct bpf_insn_aux_data {
                struct {
                        u32 map_index;          /* index into used_maps[] */
                        u32 map_off;            /* offset from value base address */
-                       struct bpf_iarray *jt;  /* jump table for gotox instruction */
                };
                struct {
                        enum bpf_reg_type reg_type;     /* type of pseudo_btf_id */
@@ -550,6 +549,7 @@ struct bpf_insn_aux_data {
                /* remember the offset of node field within type to rewrite */
                u64 insert_off;
        };
+       struct bpf_iarray *jt;  /* jump table for gotox or bpf_tailcall call instruction */
        struct btf_struct_meta *kptr_struct_meta;
        u64 map_key_state; /* constant (32 bit) key tracking for maps */
        int ctx_field_size; /* the ctx field size for load insn, maybe 0 */
@@ -652,6 +652,7 @@ struct bpf_subprog_info {
        u32 start; /* insn idx of function entry point */
        u32 linfo_idx; /* The idx to the main_prog->aux->linfo */
        u32 postorder_start; /* The idx to the env->cfg.insn_postorder */
+       u32 exit_idx; /* Index of one of the BPF_EXIT instructions in this subprogram */
        u16 stack_depth; /* max. stack depth used by this function */
        u16 stack_extra;
        /* offsets in range [stack_depth .. fastcall_stack_off)
@@ -669,9 +670,9 @@ struct bpf_subprog_info {
        bool keep_fastcall_stack: 1;
        bool changes_pkt_data: 1;
        bool might_sleep: 1;
+       u8 arg_cnt:3;
 
        enum priv_stack_mode priv_stack_mode;
-       u8 arg_cnt;
        struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS];
 };
 

diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
index a7240013fd9d9dea0d396ae39e3d330625d4cbbe..60db5d655495b52a22303856e68417df77facbd2 100644 (file)
--- a/kernel/bpf/liveness.c
+++ b/kernel/bpf/liveness.c
@@ -482,11 +482,12 @@ bpf_insn_successors(struct bpf_verifier_env *env, u32 idx)
        struct bpf_prog *prog = env->prog;
        struct bpf_insn *insn = &prog->insnsi[idx];
        const struct opcode_info *opcode_info;
-       struct bpf_iarray *succ;
+       struct bpf_iarray *succ, *jt;
        int insn_sz;
 
-       if (unlikely(insn_is_gotox(insn)))
-               return env->insn_aux_data[idx].jt;
+       jt = env->insn_aux_data[idx].jt;
+       if (unlikely(jt))
+               return jt;
 
        /* pre-allocated array of size up to 2; reset cnt, as it may have been used already */
        succ = env->succ;

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9426367fc911388f95c7934f6286a1ef7c57b1f2..0828718a8ba710dd148127d00335ab7251041890 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3555,8 +3555,12 @@ static int check_subprogs(struct bpf_verifier_env *env)
                        subprog[cur_subprog].has_ld_abs = true;
                if (BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32)
                        goto next;
-               if (BPF_OP(code) == BPF_EXIT || BPF_OP(code) == BPF_CALL)
+               if (BPF_OP(code) == BPF_CALL)
                        goto next;
+               if (BPF_OP(code) == BPF_EXIT) {
+                       subprog[cur_subprog].exit_idx = i;
+                       goto next;
+               }
                off = i + bpf_jmp_offset(&insn[i]) + 1;
                if (off < subprog_start || off >= subprog_end) {
                        verbose(env, "jump out of range from insn %d to %d\n", i, off);
@@ -18156,6 +18160,25 @@ static int visit_gotox_insn(int t, struct bpf_verifier_env *env)
        return keep_exploring ? KEEP_EXPLORING : DONE_EXPLORING;
 }
 
+static int visit_tailcall_insn(struct bpf_verifier_env *env, int t)
+{
+       static struct bpf_subprog_info *subprog;
+       struct bpf_iarray *jt;
+
+       if (env->insn_aux_data[t].jt)
+               return 0;
+
+       jt = iarray_realloc(NULL, 2);
+       if (!jt)
+               return -ENOMEM;
+
+       subprog = bpf_find_containing_subprog(env, t);
+       jt->items[0] = t + 1;
+       jt->items[1] = subprog->exit_idx;
+       env->insn_aux_data[t].jt = jt;
+       return 0;
+}
+
 /* Visits the instruction at index t and returns one of the following:
  *  < 0 - an error occurred
  *  DONE_EXPLORING - the instruction was fully explored
@@ -18216,6 +18239,8 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
                                mark_subprog_might_sleep(env, t);
                        if (bpf_helper_changes_pkt_data(insn->imm))
                                mark_subprog_changes_pkt_data(env, t);
+                       if (insn->imm == BPF_FUNC_tail_call)
+                               visit_tailcall_insn(env, t);
                } else if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
                        struct bpf_kfunc_call_arg_meta meta;
 
@@ -21477,7 +21502,7 @@ static void clear_insn_aux_data(struct bpf_verifier_env *env, int start, int len
        int i;
 
        for (i = start; i < end; i++) {
-               if (insn_is_gotox(&insns[i])) {
+               if (aux_data[i].jt) {
                        kvfree(aux_data[i].jt);
                        aux_data[i].jt = NULL;
                }