[PATCH bpf v5 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.

Commit 022ac0750883 ("bpf: use reg->var_off instead of reg->off for
pointers") moved constant pointer offsets from reg->off to reg->var_off.
However, __check_buffer_access() continued to check only the instruction
offset. An access with reg->var_off equal to -8 and an instruction offset
of zero therefore passes verification.

For writable raw tracepoints, the access end is also calculated from the
unsigned reg->var_off.value. An eight-byte access starting at -8 wraps
the calculated end to zero, allowing the program to load and attach
without increasing max_tp_access.

After ensuring that reg->var_off is constant, calculate the effective
access start using signed arithmetic and reject it when it is negative.
Use the validated start to calculate the access end for both
PTR_TO_TP_BUFFER and PTR_TO_BUF.

Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")
Acked-by: Shung-Hsi Yu <shung-hsi...@suse.com>
Cc: sta...@vger.kernel.org # 5.2.0
Signed-off-by: Sun Jian <sun.jian.k...@gmail.com>
---
 kernel/bpf/verifier.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6515d4d3c003..9f1333676365 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5326,14 +5326,11 @@ 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)
 {
-       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;
-       }
+       s64 start;
+
        if (!tnum_is_const(reg->var_off)) {
                char tn_buf[48];
 
@@ -5344,6 +5341,15 @@ static int __check_buffer_access(struct bpf_verifier_env 
*env,
                return -EACCES;
        }
 
+       start = (s64)reg->var_off.value + 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, 
(s64)reg->var_off.value);
+               return -EACCES;
+       }
+
+       *access_end = start + size;
        return 0;
 }
 
@@ -5351,14 +5357,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 +5376,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