torvalds/linux
torvalds/linux
2
0
Fork 0
mirror of git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-09-04 20:19:47 +08:00
Code

bpf: verifier: Refactor helper access type tracking

Browse Source 
Previously, the verifier was treating all PTR_TO_STACK registers passed
to a helper call as potentially written to by the helper. However, all
calls to check_stack_range_initialized() already have precise access type
information available.

Rather than treat ACCESS_HELPER as a proxy for BPF_WRITE, pass
enum bpf_access_type to check_stack_range_initialized() to more
precisely track helper arguments.

One benefit from this precision is that registers tracked as valid
spills and passed as a read-only helper argument remain tracked after
the call.  Rather than being marked STACK_MISC afterwards.

An additional benefit is the verifier logs are also more precise. For
this particular error, users will enjoy a slightly clearer message. See
included selftest updates for examples.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
Link: https://lore.kernel.org/r/ff885c0e5859e0cd12077c3148ff0754cad4f7ed.1736886479.git.dxu@dxuuu.xyz
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
Daniel Xu 2025-01-14 13:28:44 -07:00 committed by Alexei Starovoitov
parent 8ac412a336
commit 37cce22dbd
13 changed files with 42 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
kernel/bpf/verifier.c
View File

@ -5303,7 +5303,7 @@ enum bpf_access_src {
static int check_stack_range_initialized(struct bpf_verifier_env *env,
int regno, int off, int access_size,
bool zero_size_allowed,
enum bpf_access_src type,
enum bpf_access_type type,
struct bpf_call_arg_meta *meta);
static struct bpf_reg_state *reg_state(struct bpf_verifier_env *env, int regno)
@ -5336,7 +5336,7 @@ static int check_stack_read_var_off(struct bpf_verifier_env *env,
/* Note that we pass a NULL meta, so raw access will not be permitted.
*/
err = check_stack_range_initialized(env, ptr_regno, off, size,
false, ACCESS_DIRECT, NULL);
false, BPF_READ, NULL);
if (err)
return err;
@ -7190,7 +7190,7 @@ static int check_stack_slot_within_bounds(struct bpf_verifier_env *env,
static int check_stack_access_within_bounds(
struct bpf_verifier_env *env,
int regno, int off, int access_size,
enum bpf_access_src src, enum bpf_access_type type)
enum bpf_access_type type)
{
struct bpf_reg_state *regs = cur_regs(env);
struct bpf_reg_state *reg = regs + regno;
@ -7199,10 +7199,7 @@ static int check_stack_access_within_bounds(
int err;
char *err_extra;
if (src == ACCESS_HELPER)
/* We don't know if helpers are reading or writing (or both). */
err_extra = " indirect access to";
else if (type == BPF_READ)
if (type == BPF_READ)
err_extra = " read from";
else
err_extra = " write to";
@ -7420,7 +7417,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
} else if (reg->type == PTR_TO_STACK) {
/* Basic bounds checks. */
err = check_stack_access_within_bounds(env, regno, off, size, ACCESS_DIRECT, t);
err = check_stack_access_within_bounds(env, regno, off, size, t);
if (err)
return err;
@ -7640,13 +7637,11 @@ static int check_atomic(struct bpf_verifier_env *env, int insn_idx, struct bpf_i
static int check_stack_range_initialized(
struct bpf_verifier_env *env, int regno, int off,
int access_size, bool zero_size_allowed,
enum bpf_access_src type, struct bpf_call_arg_meta *meta)
enum bpf_access_type type, struct bpf_call_arg_meta *meta)
{
struct bpf_reg_state *reg = reg_state(env, regno);
struct bpf_func_state *state = func(env, reg);
int err, min_off, max_off, i, j, slot, spi;
char *err_extra = type == ACCESS_HELPER ? " indirect" : "";
enum bpf_access_type bounds_check_type;
/* Some accesses can write anything into the stack, others are
* read-only.
*/
@ -7657,18 +7652,10 @@ static int check_stack_range_initialized(
return -EACCES;
}
if (type == ACCESS_HELPER) {
/* The bounds checks for writes are more permissive than for
* reads. However, if raw_mode is not set, we'll do extra
* checks below.
*/
bounds_check_type = BPF_WRITE;
if (type == BPF_WRITE)
clobber = true;
} else {
bounds_check_type = BPF_READ;
}
err = check_stack_access_within_bounds(env, regno, off, access_size,
type, bounds_check_type);
err = check_stack_access_within_bounds(env, regno, off, access_size, type);
if (err)
return err;
@ -7685,8 +7672,8 @@ static int check_stack_range_initialized(
char tn_buf[48];
tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
verbose(env, "R%d%s variable offset stack access prohibited for !root, var_off=%s\n",
regno, err_extra, tn_buf);
verbose(env, "R%d variable offset stack access prohibited for !root, var_off=%s\n",
regno, tn_buf);
return -EACCES;
}
/* Only initialized buffer on stack is allowed to be accessed
@ -7767,14 +7754,14 @@ static int check_stack_range_initialized(
}
if (tnum_is_const(reg->var_off)) {
verbose(env, "invalid%s read from stack R%d off %d+%d size %d\n",
err_extra, regno, min_off, i - min_off, access_size);
verbose(env, "invalid read from stack R%d off %d+%d size %d\n",
regno, min_off, i - min_off, access_size);
} else {
char tn_buf[48];
tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
verbose(env, "invalid%s read from stack R%d var_off %s+%d size %d\n",
err_extra, regno, tn_buf, i - min_off, access_size);
verbose(env, "invalid read from stack R%d var_off %s+%d size %d\n",
regno, tn_buf, i - min_off, access_size);
}
return -EACCES;
mark:
@ -7849,7 +7836,7 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
return check_stack_range_initialized(
env,
regno, reg->off, access_size,
zero_size_allowed, ACCESS_HELPER, meta);
zero_size_allowed, access_type, meta);
case PTR_TO_BTF_ID:
return check_ptr_to_btf_access(env, regs, regno, reg->off,
access_size, BPF_READ, -1);

6
tools/testing/selftests/bpf/progs/dynptr_fail.c
View File

@ -192,7 +192,7 @@ done:
/* Can't add a dynptr to a map */
SEC("?raw_tp")
__failure __msg("invalid indirect read from stack")
__failure __msg("invalid read from stack")
int add_dynptr_to_map1(void *ctx)
{
struct bpf_dynptr ptr;
@ -210,7 +210,7 @@ int add_dynptr_to_map1(void *ctx)
/* Can't add a struct with an embedded dynptr to a map */
SEC("?raw_tp")
__failure __msg("invalid indirect read from stack")
__failure __msg("invalid read from stack")
int add_dynptr_to_map2(void *ctx)
{
struct test_info x;
@ -398,7 +398,7 @@ int data_slice_missing_null_check2(void *ctx)
* dynptr argument
*/
SEC("?raw_tp")
__failure __msg("invalid indirect read from stack")
__failure __msg("invalid read from stack")
int invalid_helper1(void *ctx)
{
struct bpf_dynptr ptr;

2
tools/testing/selftests/bpf/progs/test_global_func10.c
View File

@ -26,7 +26,7 @@ __noinline int foo(const struct Big *big)
}
SEC("cgroup_skb/ingress")
__failure __msg("invalid indirect access to stack")
__failure __msg("invalid read from stack")
int global_func10(struct __sk_buff *skb)
{
const struct Small small = {.x = skb->len };

5
tools/testing/selftests/bpf/progs/uninit_stack.c
View File

@ -70,7 +70,8 @@ __naked int helper_uninit_to_misc(void *ctx)
r1 = r10; \
r1 += -128; \
r2 = 32; \
call %[bpf_trace_printk]; \
r3 = 0; \
call %[bpf_probe_read_user]; \
/* Call to dummy() forces print_verifier_state(..., true), \
* thus showing the stack state, matched by __msg(). \
*/ \
@ -79,7 +80,7 @@ __naked int helper_uninit_to_misc(void *ctx)
exit; \
"
:
: __imm(bpf_trace_printk),
: __imm(bpf_probe_read_user),
__imm(dummy)
: __clobber_all);
}

2
tools/testing/selftests/bpf/progs/verifier_basic_stack.c
View File

@ -28,7 +28,7 @@ __naked void stack_out_of_bounds(void)
SEC("socket")
__description("uninitialized stack1")
__success __log_level(4) __msg("stack depth 8")
__failure_unpriv __msg_unpriv("invalid indirect read from stack")
__failure_unpriv __msg_unpriv("invalid read from stack")
__naked void uninitialized_stack1(void)
{
asm volatile (" \

4
tools/testing/selftests/bpf/progs/verifier_const_or.c
View File

@ -25,7 +25,7 @@ __naked void constant_should_keep_constant_type(void)
SEC("tracepoint")
__description("constant register |= constant should not bypass stack boundary checks")
__failure __msg("invalid indirect access to stack R1 off=-48 size=58")
__failure __msg("invalid write to stack R1 off=-48 size=58")
__naked void not_bypass_stack_boundary_checks_1(void)
{
asm volatile (" \
@ -62,7 +62,7 @@ __naked void register_should_keep_constant_type(void)
SEC("tracepoint")
__description("constant register |= constant register should not bypass stack boundary checks")
__failure __msg("invalid indirect access to stack R1 off=-48 size=58")
__failure __msg("invalid write to stack R1 off=-48 size=58")
__naked void not_bypass_stack_boundary_checks_2(void)
{
asm volatile (" \

12
tools/testing/selftests/bpf/progs/verifier_helper_access_var_len.c
View File

@ -67,7 +67,7 @@ SEC("socket")
__description("helper access to variable memory: stack, bitwise AND, zero included")
/* in privileged mode reads from uninitialized stack locations are permitted */
__success __failure_unpriv
__msg_unpriv("invalid indirect read from stack R2 off -64+0 size 64")
__msg_unpriv("invalid read from stack R2 off -64+0 size 64")
__retval(0)
__naked void stack_bitwise_and_zero_included(void)
{
@ -100,7 +100,7 @@ __naked void stack_bitwise_and_zero_included(void)
SEC("tracepoint")
__description("helper access to variable memory: stack, bitwise AND + JMP, wrong max")
__failure __msg("invalid indirect access to stack R1 off=-64 size=65")
__failure __msg("invalid write to stack R1 off=-64 size=65")
__naked void bitwise_and_jmp_wrong_max(void)
{
asm volatile (" \
@ -187,7 +187,7 @@ l0_%=: r0 = 0; \
SEC("tracepoint")
__description("helper access to variable memory: stack, JMP, bounds + offset")
__failure __msg("invalid indirect access to stack R1 off=-64 size=65")
__failure __msg("invalid write to stack R1 off=-64 size=65")
__naked void memory_stack_jmp_bounds_offset(void)
{
asm volatile (" \
@ -211,7 +211,7 @@ l0_%=: r0 = 0; \
SEC("tracepoint")
__description("helper access to variable memory: stack, JMP, wrong max")
__failure __msg("invalid indirect access to stack R1 off=-64 size=65")
__failure __msg("invalid write to stack R1 off=-64 size=65")
__naked void memory_stack_jmp_wrong_max(void)
{
asm volatile (" \
@ -260,7 +260,7 @@ SEC("socket")
__description("helper access to variable memory: stack, JMP, no min check")
/* in privileged mode reads from uninitialized stack locations are permitted */
__success __failure_unpriv
__msg_unpriv("invalid indirect read from stack R2 off -64+0 size 64")
__msg_unpriv("invalid read from stack R2 off -64+0 size 64")
__retval(0)
__naked void stack_jmp_no_min_check(void)
{
@ -750,7 +750,7 @@ SEC("socket")
__description("helper access to variable memory: 8 bytes leak")
/* in privileged mode reads from uninitialized stack locations are permitted */
__success __failure_unpriv
__msg_unpriv("invalid indirect read from stack R2 off -64+32 size 64")
__msg_unpriv("invalid read from stack R2 off -64+32 size 64")
__retval(0)
__naked void variable_memory_8_bytes_leak(void)
{

2
tools/testing/selftests/bpf/progs/verifier_int_ptr.c
View File

@ -96,7 +96,7 @@ __naked void arg_ptr_to_long_misaligned(void)
SEC("cgroup/sysctl")
__description("arg pointer to long size < sizeof(long)")
__failure __msg("invalid indirect access to stack R4 off=-4 size=8")
__failure __msg("invalid write to stack R4 off=-4 size=8")
__naked void to_long_size_sizeof_long(void)
{
asm volatile (" \

2
tools/testing/selftests/bpf/progs/verifier_mtu.c
View File

@ -8,7 +8,7 @@ SEC("tc/ingress")
__description("uninit/mtu: write rejected")
__success
__caps_unpriv(CAP_BPF|CAP_NET_ADMIN)
__failure_unpriv __msg_unpriv("invalid indirect read from stack")
__failure_unpriv __msg_unpriv("invalid read from stack")
int tc_uninit_mtu(struct __sk_buff *ctx)
{
__u32 mtu;

4
tools/testing/selftests/bpf/progs/verifier_raw_stack.c
View File

@ -236,7 +236,7 @@ __naked void load_bytes_spilled_regs_data(void)
SEC("tc")
__description("raw_stack: skb_load_bytes, invalid access 1")
__failure __msg("invalid indirect access to stack R3 off=-513 size=8")
__failure __msg("invalid write to stack R3 off=-513 size=8")
__naked void load_bytes_invalid_access_1(void)
{
asm volatile (" \
@ -255,7 +255,7 @@ __naked void load_bytes_invalid_access_1(void)
SEC("tc")
__description("raw_stack: skb_load_bytes, invalid access 2")
__failure __msg("invalid indirect access to stack R3 off=-1 size=8")
__failure __msg("invalid write to stack R3 off=-1 size=8")
__naked void load_bytes_invalid_access_2(void)
{
asm volatile (" \

2
tools/testing/selftests/bpf/progs/verifier_unpriv.c
View File

@ -199,7 +199,7 @@ __naked void pass_pointer_to_helper_function(void)
SEC("socket")
__description("unpriv: indirectly pass pointer on stack to helper function")
__success __failure_unpriv
__msg_unpriv("invalid indirect read from stack R2 off -8+0 size 8")
__msg_unpriv("invalid read from stack R2 off -8+0 size 8")
__retval(0)
__naked void on_stack_to_helper_function(void)
{

8
tools/testing/selftests/bpf/progs/verifier_var_off.c
View File

@ -203,7 +203,7 @@ __naked void stack_write_clobbers_spilled_regs(void)
SEC("sockops")
__description("indirect variable-offset stack access, unbounded")
__failure __msg("invalid unbounded variable-offset indirect access to stack R4")
__failure __msg("invalid unbounded variable-offset write to stack R4")
__naked void variable_offset_stack_access_unbounded(void)
{
asm volatile (" \
@ -236,7 +236,7 @@ l0_%=: r0 = 0; \
SEC("lwt_in")
__description("indirect variable-offset stack access, max out of bound")
__failure __msg("invalid variable-offset indirect access to stack R2")
__failure __msg("invalid variable-offset read from stack R2")
__naked void access_max_out_of_bound(void)
{
asm volatile (" \
@ -269,7 +269,7 @@ __naked void access_max_out_of_bound(void)
*/
SEC("socket")
__description("indirect variable-offset stack access, zero-sized, max out of bound")
__failure __msg("invalid variable-offset indirect access to stack R1")
__failure __msg("invalid variable-offset write to stack R1")
__naked void zero_sized_access_max_out_of_bound(void)
{
asm volatile (" \
@ -294,7 +294,7 @@ __naked void zero_sized_access_max_out_of_bound(void)
SEC("lwt_in")
__description("indirect variable-offset stack access, min out of bound")
__failure __msg("invalid variable-offset indirect access to stack R2")
__failure __msg("invalid variable-offset read from stack R2")
__naked void access_min_out_of_bound(void)
{
asm volatile (" \

2
tools/testing/selftests/bpf/verifier/calls.c
View File

@ -2252,7 +2252,7 @@
BPF_EXIT_INSN(),
},
.fixup_map_hash_48b = { 7 },
.errstr_unpriv = "invalid indirect read from stack R2 off -8+0 size 8",
.errstr_unpriv = "invalid read from stack R2 off -8+0 size 8",
.result_unpriv = REJECT,
/* in privileged mode reads from uninitialized stack locations are permitted */
.result = ACCEPT,