[PATCH bpf v4 1/2] bpf: Reject negative const offsets for buffer pointers

[Sun Jian]

The verifier rejects variable offsets for PTR_TO_TP_BUFFER and PTR_TO_BUF
accesses, but it currently accepts a constant negative offset produced by
pointer arithmetic.

For example, a raw tracepoint writable program can load ctx[0] as a
PTR_TO_TP_BUFFER, move it by -8, and then access the adjusted pointer.
The access is before the tracepoint writable buffer base and should be
rejected.

Keep rejecting negative instruction offsets explicitly. Once the register
offset is known to be constant, reject const var_off values outside the
same +/-BPF_MAX_VAR_OFF range used by other verifier pointer offset
checks before computing the effective access range.

Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")
Signed-off-by: Sun Jian <sun.jian.k...@gmail.com>
---
 kernel/bpf/verifier.c | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 21a365d436a5..6fe4fcf93e5f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct 
bpf_verifier_env *env)
 static int __check_buffer_access(struct bpf_verifier_env *env,
                                 const char *buf_info,
                                 const struct bpf_reg_state *reg,
-                                argno_t argno, int off, int size)
+                                argno_t argno, int off, int size,
+                                u32 *access_end)
 {
+       s64 start, var_off;
+
        if (off < 0) {
                verbose(env,
                        "%s invalid %s buffer access: off=%d, size=%d\n",
                        reg_arg_name(env, argno), buf_info, off, size);
                return -EACCES;
        }
+
        if (!tnum_is_const(reg->var_off)) {
                char tn_buf[48];
 
@@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env 
*env,
                return -EACCES;
        }
 
+       var_off = (s64)reg->var_off.value;
+       if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
+               verbose(env, "%s %s buffer offset %lld is not allowed\n",
+                       reg_arg_name(env, argno), buf_info, var_off);
+               return -EACCES;
+       }
+
+       start = var_off + off;
+       if (start < 0) {
+               verbose(env,
+                       "%s invalid negative %s buffer offset: off=%d, 
var_off=%lld\n",
+                       reg_arg_name(env, argno), buf_info, off, var_off);
+               return -EACCES;
+       }
+
+       if (size < 0) {
+               verbose(env, "%s invalid %s buffer access: off=%lld, size=%d\n",
+                       reg_arg_name(env, argno), buf_info, start, size);
+               return -EACCES;
+       }
+
+       *access_end = (u32)start + (u32)size;
+
        return 0;
 }
 
@@ -5351,14 +5378,14 @@ static int check_tp_buffer_access(struct 
bpf_verifier_env *env,
                                  const struct bpf_reg_state *reg,
                                  argno_t argno, int off, int size)
 {
+       u32 access_end;
        int err;
 
-       err = __check_buffer_access(env, "tracepoint", reg, argno, off, size);
+       err = __check_buffer_access(env, "tracepoint", reg, argno, off, size, 
&access_end);
        if (err)
                return err;
 
-       env->prog->aux->max_tp_access = max(reg->var_off.value + off + size,
-                                           env->prog->aux->max_tp_access);
+       env->prog->aux->max_tp_access = max(access_end, 
env->prog->aux->max_tp_access);
 
        return 0;
 }
@@ -5370,13 +5397,14 @@ static int check_buffer_access(struct bpf_verifier_env 
*env,
                               u32 *max_access)
 {
        const char *buf_info = type_is_rdonly_mem(reg->type) ? "rdonly" : 
"rdwr";
+       u32 access_end;
        int err;
 
-       err = __check_buffer_access(env, buf_info, reg, argno, off, size);
+       err = __check_buffer_access(env, buf_info, reg, argno, off, size, 
&access_end);
        if (err)
                return err;
 
-       *max_access = max(reg->var_off.value + off + size, *max_access);
+       *max_access = max(access_end, *max_access);
 
        return 0;
 }
-- 
2.43.0