The dynptr argument to a bpf_user_ringbuf_drain() callback points to a descriptor on the helper's stack. The descriptor and the sample it describes are valid only during that callback invocation. The callback can save the dynptr pointer, a slice, or a clone in the caller's frame and use it after the drain returns. An escaped descriptor pointer can allow arbitrary kernel read/write once the helper's stack is reused. The verifier assigns the dynptr a type and an id, but does not tie it or its descendants to the callback's lifetime. Give the callback frame an id and make it both the id and the parent_id of R1, then invalidate that id and everything derived from it when the frame is popped. R1 has to be its own parent: slices take the dynptr's id as their parent_id, and bpf_dynptr_data() clobbers R1, so with a separate dynptr id nothing would connect a slice to the frame. Fixes: 205715673844 ("bpf: Add bpf_user_ringbuf_drain() helper") Fixes: 1f737e46ca6a ("bpf: Remove artificial limitations on pointer types eligible for spilling") Cc: stable@vger.kernel.org Reported-by: Nicholas Carlini Suggested-by: Alexei Starovoitov Signed-off-by: Ihor Solodrai --- include/linux/bpf_verifier.h | 2 ++ kernel/bpf/states.c | 3 +++ kernel/bpf/verifier.c | 16 ++++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index b43eb97bcd44..dabdd286182c 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -377,6 +377,8 @@ struct bpf_func_state { * reused after returns, while this ID is preserved across state clones. */ u32 diag_frame_id; + /* parent_id of the args that are gone when this frame returns */ + u32 frame_id; /* subprog number == index within subprog_info * zero == main subprog */ diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c index 50480c42bcad..d8ad497f9b4b 100644 --- a/kernel/bpf/states.c +++ b/kernel/bpf/states.c @@ -998,6 +998,9 @@ static bool func_states_equal(struct bpf_verifier_env *env, struct bpf_func_stat if (old->in_rcu_callback_fn != cur->in_rcu_callback_fn) return false; + if (!check_ids(old->frame_id, cur->frame_id, &env->idmap_scratch)) + return false; + for (i = 0; i < MAX_BPF_REG; i++) if (((1 << i) & live_regs) && !regsafe(env, &old->regs[i], &cur->regs[i], diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5e4700fda66b..a801b8da2b63 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -11804,6 +11804,16 @@ int map_set_for_each_callback_args(struct bpf_verifier_env *env, return 0; } +/* The arg points into the stack frame of the helper that calls the callback */ +static void mark_frame_scoped_arg(struct bpf_verifier_env *env, + struct bpf_func_state *callee, u32 regno) +{ + if (!callee->frame_id) + callee->frame_id = ++env->id_gen; + callee->regs[regno].id = callee->frame_id; + callee->regs[regno].parent_id = callee->frame_id; +} + static int set_callee_state(struct bpf_verifier_env *env, struct bpf_func_state *caller, struct bpf_func_state *callee, int insn_idx) @@ -11955,6 +11965,7 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env, */ bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_0]); mark_dynptr_cb_reg(env, &callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL); + mark_frame_scoped_arg(env, callee, BPF_REG_1); callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3]; /* unused */ @@ -12223,6 +12234,11 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) if (err) return err; invalidate_stack_slices(env, callee->frameno, INT_MIN, 0); + if (callee->frame_id) { + err = invalidate_reference(env, callee->frame_id); + if (err) + return err; + } /* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite, * there function call logic would reschedule callback visit. If iteration -- 2.56.0