Skip to content

Commit 4967bd5

Browse files
committed
Merge branch 'bpf-reject-negative-const-offsets-for-buffer-pointers'
Sun Jian says: ==================== bpf: Reject negative const offsets for buffer pointers Reject negative effective offsets for PTR_TO_TP_BUFFER and PTR_TO_BUF accesses. Calculate the effective access start using signed arithmetic to prevent unsigned access-end accounting from wrapping, and cover both load-time rejection and the raw tracepoint writable attach-time path. --- Changes in v5: - Simplify __check_buffer_access() to reject a negative effective start after confirming that var_off is constant. Validate the combined offset instead of rejecting negative instruction offsets separately. Drop the duplicate BPF_MAX_VAR_OFF check because pointer arithmetic already bounds constant offsets, and remove the redundant size < 0 check. - Switch the raw tracepoint writable attach tests from nbd_send_request to bpf_testmod_test_writable_bare_tp, avoiding the NBD configuration dependency and its false-pass condition. - Split the attach coverage into named subtests and require bpf_raw_tracepoint_open() to return -EINVAL. - Add verifier coverage for a negative constant PTR_TO_BUF offset. Changes in v4: - Correct the Fixes tag to point to 022ac07, where pointer offsets were folded into reg->var_off. - Drop the end > U32_MAX check, which is unreachable after bounding const var_off with BPF_MAX_VAR_OFF while keeping instruction offsets and access sizes bounded. Changes in v3: - Check constant var_off against +/-BPF_MAX_VAR_OFF before computing the effective access range, matching the existing verifier pointer offset convention. - Keep explicit rejection of negative instruction offsets and keep bounded negative constant var_off valid when the effective offset is non-negative. Changes in v2: - Split the kernel fix and selftests into separate patches. - Add an attach-time raw tracepoint writable test that exercises max_tp_access against nbd_send_request's writable size. - Adjust selftest formatting to use the 100 character line width. Tested: - ./test_progs -v -t verifier_raw_tp_writable - ./test_progs -v -t verifier_ptr_to_buf - ./test_progs -v -t raw_tp_writable_reject_bad_access - ./test_progs -v -t raw_tp_writable_test_run v4: https://lore.kernel.org/bpf/20260708090151.151729-1-sun.jian.kdev@gmail.com/ v3: https://lore.kernel.org/bpf/20260708040715.116680-1-sun.jian.kdev@gmail.com/ v2: https://lore.kernel.org/bpf/20260707060804.93561-1-sun.jian.kdev@gmail.com/ v1: https://lore.kernel.org/bpf/20260703035137.109608-1-sun.jian.kdev@gmail.com/ ==================== Link: https://patch.msgid.link/20260714093846.18159-1-sun.jian.kdev@gmail.com Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
2 parents 2d8af4e + 6f59deb commit 4967bd5

6 files changed

Lines changed: 121 additions & 55 deletions

File tree

kernel/bpf/verifier.c

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5326,14 +5326,11 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
53265326
static int __check_buffer_access(struct bpf_verifier_env *env,
53275327
const char *buf_info,
53285328
const struct bpf_reg_state *reg,
5329-
argno_t argno, int off, int size)
5329+
argno_t argno, int off, int size,
5330+
u32 *access_end)
53305331
{
5331-
if (off < 0) {
5332-
verbose(env,
5333-
"%s invalid %s buffer access: off=%d, size=%d\n",
5334-
reg_arg_name(env, argno), buf_info, off, size);
5335-
return -EACCES;
5336-
}
5332+
s64 start;
5333+
53375334
if (!tnum_is_const(reg->var_off)) {
53385335
char tn_buf[48];
53395336

@@ -5344,21 +5341,30 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
53445341
return -EACCES;
53455342
}
53465343

5344+
start = (s64)reg->var_off.value + off;
5345+
if (start < 0) {
5346+
verbose(env,
5347+
"%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
5348+
reg_arg_name(env, argno), buf_info, off, (s64)reg->var_off.value);
5349+
return -EACCES;
5350+
}
5351+
5352+
*access_end = start + size;
53475353
return 0;
53485354
}
53495355

53505356
static int check_tp_buffer_access(struct bpf_verifier_env *env,
53515357
const struct bpf_reg_state *reg,
53525358
argno_t argno, int off, int size)
53535359
{
5360+
u32 access_end;
53545361
int err;
53555362

5356-
err = __check_buffer_access(env, "tracepoint", reg, argno, off, size);
5363+
err = __check_buffer_access(env, "tracepoint", reg, argno, off, size, &access_end);
53575364
if (err)
53585365
return err;
53595366

5360-
env->prog->aux->max_tp_access = max(reg->var_off.value + off + size,
5361-
env->prog->aux->max_tp_access);
5367+
env->prog->aux->max_tp_access = max(access_end, env->prog->aux->max_tp_access);
53625368

53635369
return 0;
53645370
}
@@ -5370,13 +5376,14 @@ static int check_buffer_access(struct bpf_verifier_env *env,
53705376
u32 *max_access)
53715377
{
53725378
const char *buf_info = type_is_rdonly_mem(reg->type) ? "rdonly" : "rdwr";
5379+
u32 access_end;
53735380
int err;
53745381

5375-
err = __check_buffer_access(env, buf_info, reg, argno, off, size);
5382+
err = __check_buffer_access(env, buf_info, reg, argno, off, size, &access_end);
53765383
if (err)
53775384
return err;
53785385

5379-
*max_access = max(reg->var_off.value + off + size, *max_access);
5386+
*max_access = max(access_end, *max_access);
53805387

53815388
return 0;
53825389
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
#include <test_progs.h>
4+
#include "test_kmods/bpf_testmod.h"
5+
#include "bpf_util.h"
6+
7+
static void check_attach_reject(const struct bpf_insn *program, size_t prog_len)
8+
{
9+
LIBBPF_OPTS(bpf_prog_load_opts, opts);
10+
char error[4096];
11+
int bpf_fd, tp_fd;
12+
13+
opts.log_level = 2;
14+
opts.log_buf = error;
15+
opts.log_size = sizeof(error);
16+
17+
bpf_fd = bpf_prog_load(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, NULL, "GPL v2",
18+
program, prog_len, &opts);
19+
if (!ASSERT_GE(bpf_fd, 0, "prog_load"))
20+
return;
21+
22+
tp_fd = bpf_raw_tracepoint_open("bpf_testmod_test_writable_bare_tp", bpf_fd);
23+
ASSERT_EQ(tp_fd, -EINVAL, "bpf_raw_tracepoint_open");
24+
if (tp_fd >= 0)
25+
close(tp_fd);
26+
27+
close(bpf_fd);
28+
}
29+
30+
void test_raw_tp_writable_reject_bad_access(void)
31+
{
32+
const struct bpf_insn program[] = {
33+
/* r6 is our tp buffer */
34+
BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, 0),
35+
/* one byte beyond the end of the writable context */
36+
BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
37+
sizeof(struct bpf_testmod_test_writable_ctx)),
38+
BPF_EXIT_INSN(),
39+
};
40+
41+
const struct bpf_insn negative_var_off_program[] = {
42+
BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, 0),
43+
/* make var_off negative, but keep the effective access offset non-negative */
44+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -8),
45+
/* one byte beyond the end of the writable context */
46+
BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
47+
sizeof(struct bpf_testmod_test_writable_ctx) + 8),
48+
BPF_EXIT_INSN(),
49+
};
50+
51+
if (test__start_subtest("past_end"))
52+
check_attach_reject(program, ARRAY_SIZE(program));
53+
54+
if (test__start_subtest("negative_var_off_past_end"))
55+
check_attach_reject(negative_var_off_program,
56+
ARRAY_SIZE(negative_var_off_program));
57+
}

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c

Lines changed: 0 additions & 43 deletions
This file was deleted.

tools/testing/selftests/bpf/prog_tests/verifier.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
#include "verifier_precision.skel.h"
7979
#include "verifier_prevent_map_lookup.skel.h"
8080
#include "verifier_private_stack.skel.h"
81+
#include "verifier_ptr_to_buf.skel.h"
8182
#include "verifier_raw_stack.skel.h"
8283
#include "verifier_raw_tp_writable.skel.h"
8384
#include "verifier_reg_equal.skel.h"
@@ -230,6 +231,7 @@ void test_verifier_or_jmp32_k(void) { RUN(verifier_or_jmp32_k); }
230231
void test_verifier_precision(void) { RUN(verifier_precision); }
231232
void test_verifier_prevent_map_lookup(void) { RUN(verifier_prevent_map_lookup); }
232233
void test_verifier_private_stack(void) { RUN(verifier_private_stack); }
234+
void test_verifier_ptr_to_buf(void) { RUN(verifier_ptr_to_buf); }
233235
void test_verifier_raw_stack(void) { RUN(verifier_raw_stack); }
234236
void test_verifier_raw_tp_writable(void) { RUN(verifier_raw_tp_writable); }
235237
void test_verifier_reg_equal(void) { RUN(verifier_reg_equal); }
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
#include <vmlinux.h>
4+
#include <bpf/bpf_helpers.h>
5+
#include "bpf_misc.h"
6+
7+
SEC("iter/bpf_map_elem")
8+
__description("PTR_TO_BUF: reject negative const offset")
9+
__failure
10+
__msg("invalid negative rdwr buffer offset")
11+
__naked void ptr_to_buf_reject_negative_const_offset(void)
12+
{
13+
asm volatile ("r0 = 0; \
14+
r2 = *(u64 *)(r1 + %[value_off]); \
15+
if r2 == 0 goto l0_%=; \
16+
r2 += -8; \
17+
r0 = *(u64 *)(r2 + 0); \
18+
l0_%=: \
19+
exit; \
20+
"
21+
:
22+
: __imm_const(value_off,
23+
offsetof(struct bpf_iter__bpf_map_elem, value))
24+
: __clobber_all);
25+
}
26+
27+
char _license[] SEC("license") = "GPL";

tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,20 @@ l0_%=: /* shift the buffer pointer to a variable location */\
4747
: __clobber_all);
4848
}
4949

50+
SEC("raw_tracepoint.w")
51+
__description("raw_tracepoint_writable: reject negative const offset")
52+
__failure
53+
__msg("invalid negative tracepoint buffer offset")
54+
__naked void tracepoint_writable_reject_negative_const_offset(void)
55+
{
56+
asm volatile (" \
57+
r6 = *(u64 *)(r1 + 0); \
58+
r6 += -8; \
59+
r0 = *(u64 *)(r6 + 0); \
60+
exit; \
61+
" :
62+
:
63+
: __clobber_all);
64+
}
65+
5066
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)