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 read from the adjusted pointer.
The register is still tracked as tp_buffer(imm=-8), but the access is
before the tracepoint writable buffer base and should be rejected.
Check the signed effective buffer offset before updating max_tp_access or
other buffer max access accounting. Reject negative effective offsets and
use the checked end offset for max access accounting.
Add a verifier test that rejects a raw tracepoint writable program using a
negative constant offset.
Fixes: 9df1c28bb752 ("bpf: add writable context for raw tracepoints")
Signed-off-by: Sun Jian <[email protected]>
---
kernel/bpf/verifier.c | 41 ++++++++++++++++---
.../bpf/progs/verifier_raw_tp_writable.c | 16 ++++++++
2 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 21a365d436a5..421ca118fcbf 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 (check_add_overflow(var_off, (s64)off, &start)) {
+ verbose(env,
+ "%s invalid %s buffer access: off=%d, var_off=%lld\n",
+ reg_arg_name(env, argno), buf_info, off, var_off);
+ return -EACCES;
+ }
+
+ 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 (start > U32_MAX || size < 0 ||
+ check_add_overflow((u32)start, (u32)size, access_end)) {
+ verbose(env,
+ "%s invalid %s buffer access: off=%lld, size=%d\n",
+ reg_arg_name(env, argno), buf_info, start, size);
+ return -EACCES;
+ }
+
return 0;
}
@@ -5351,13 +5378,15 @@ 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 = max(access_end,
env->prog->aux->max_tp_access);
return 0;
@@ -5370,13 +5399,15 @@ 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;
}
diff --git a/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
b/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
index 14a0172e2141..4055a6443bc2 100644
--- a/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
+++ b/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
@@ -47,4 +47,20 @@ l0_%=: /* shift the buffer pointer to a variable
location */\
: __clobber_all);
}
+SEC("raw_tracepoint.w")
+__description("raw_tracepoint_writable: reject negative const offset")
+__failure
+__msg("invalid negative tracepoint buffer offset")
+__naked void tracepoint_writable_reject_negative_const_offset(void)
+{
+ asm volatile (" \
+ r6 = *(u64 *)(r1 + 0); \
+ r6 += -8; \
+ r0 = *(u64 *)(r6 + 0); \
+ exit; \
+" :
+ :
+ : __clobber_all);
+}
+
char _license[] SEC("license") = "GPL";
--
2.43.0
