bpf: Fix out of bounds access for ringbuf helpers
authorDaniel Borkmann <daniel@iogearbox.net>
Tue, 11 Jan 2022 14:43:41 +0000 (14:43 +0000)
committerDaniel Borkmann <daniel@iogearbox.net>
Wed, 19 Jan 2022 00:21:39 +0000 (01:21 +0100)
Both bpf_ringbuf_submit() and bpf_ringbuf_discard() have ARG_PTR_TO_ALLOC_MEM
in their bpf_func_proto definition as their first argument. They both expect
the result from a prior bpf_ringbuf_reserve() call which has a return type of
RET_PTR_TO_ALLOC_MEM_OR_NULL.

Meaning, after a NULL check in the code, the verifier will promote the register
type in the non-NULL branch to a PTR_TO_MEM and in the NULL branch to a known
zero scalar. Generally, pointer arithmetic on PTR_TO_MEM is allowed, so the
latter could have an offset.

The ARG_PTR_TO_ALLOC_MEM expects a PTR_TO_MEM register type. However, the non-
zero result from bpf_ringbuf_reserve() must be fed into either bpf_ringbuf_submit()
or bpf_ringbuf_discard() but with the original offset given it will then read
out the struct bpf_ringbuf_hdr mapping.

The verifier missed to enforce a zero offset, so that out of bounds access
can be triggered which could be used to escalate privileges if unprivileged
BPF was enabled (disabled by default in kernel).

Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
Reported-by: <tr3e.wang@gmail.com> (SecCoder Security Lab)
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
kernel/bpf/verifier.c

index e0b3f4d..c72c57a 100644 (file)
@@ -5318,9 +5318,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
        case PTR_TO_BUF:
        case PTR_TO_BUF | MEM_RDONLY:
        case PTR_TO_STACK:
+               /* Some of the argument types nevertheless require a
+                * zero register offset.
+                */
+               if (arg_type == ARG_PTR_TO_ALLOC_MEM)
+                       goto force_off_check;
                break;
        /* All the rest must be rejected: */
        default:
+force_off_check:
                err = __check_ptr_off_reg(env, reg, regno,
                                          type == PTR_TO_BTF_ID);
                if (err < 0)