[PATCH v2] bpf: fix out-of-bounds frame access for async callback stack args

[Henry Martin <bsdhenrymartin@gmail.com>]

check_stack_arg_read() resolves the caller frame with

    caller = vstate->frame[vstate->curframe - 1];

and copies caller->stack_arg_regs[spi] into the current register
state.  It bounds spi against the callee's declared incoming stack
arguments, but nothing validates the caller side.  Two kinds of
callback frames reach this code without a validated caller.

Async callbacks (bpf_timer_set_callback and friends) are queued via
push_async_cb(), which builds a fresh verifier state with frame[0]
as the only frame and curframe left at its kzalloc default 0, so the
code above evaluates frame[-1]: an out-of-bounds read behind the
bpf_verifier_state.frame array whose result is used as a pointer.

Synchronous callbacks (bpf_loop, bpf_for_each_map_elem, ...) run with
curframe >= 1, but push_callback_call() ignores every error from
btf_check_subprog_call() except -EFAULT, so a callback subprogram
whose prototype does not match the helper's callback ABI (e.g. more
than five BTF arguments) is still queued for exploration.  Inside
such a callback an r11-based incoming stack argument load passes the
spi bound check, while the kernel helper never passes stack arguments
at runtime:

  - caller->stack_arg_regs is a heap array sized by the caller's
    out_stack_arg_cnt; if no outgoing stack arguments were prepared,
    the load dereferences NULL or reads past the allocation;
  - if a previous subprogram call left initialized outgoing stack
    arguments behind, the stale register state is copied into the
    callback's registers even though the runtime ABI passes nothing
    there.

Kernel helpers pass at most five callback arguments, all in
registers, so an incoming stack argument load can never be valid in
a callback frame.  Reject such loads when the current frame has no
caller (curframe == 0) or is a callback frame (state->in_callback_fn).

This issue was discovered by Tencent CodeBuddy Security.

Fixes: 0f6bd5e7a804 ("bpf: Support stack arguments for bpf functions")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v2: Also reject r11-based incoming stack argument loads in synchronous
    callback frames (state->in_callback_fn), not just the async
    curframe == 0 case: push_callback_call() queues callbacks whose
    prototype failed btf_check_subprog_call() with -EINVAL, leaving
    caller->stack_arg_regs unvalidated or stale (sashiko review).
    The curframe check stays necessary because the async timer and
    workqueue setter set_timer_callback_state() does not set
    in_callback_fn.

---
 kernel/bpf/verifier.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5f874979b8d75..c838ec39590b9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4232,6 +4232,11 @@ static int check_stack_arg_read(struct bpf_verifier_env *env, struct bpf_func_st
 		return -EACCES;
 	}
 
+	if (vstate->curframe == 0 || state->in_callback_fn) {
+		verbose(env, "stack arg read from a callback frame\n");
+		return -EINVAL;
+	}
+
 	caller = vstate->frame[vstate->curframe - 1];
 	arg = &caller->stack_arg_regs[spi];
 	cur = vstate->frame[vstate->curframe];
-- 
2.43.7