Commit a33a6eaa authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf: Allow reads from uninit stack'



Merge commit bf9bec4c ("Merge branch 'bpf: Allow reads from uninit stack'")
from bpf-next to bpf tree to address verification issues in some programs
due to stack usage.

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents e8c8361c bf9bec4c
Loading
Loading
Loading
Loading
+10 −1
Original line number Diff line number Diff line
@@ -3826,6 +3826,8 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
						continue;
					if (type == STACK_MISC)
						continue;
					if (type == STACK_INVALID && env->allow_uninit_stack)
						continue;
					verbose(env, "invalid read from stack off %d+%d size %d\n",
						off, i, size);
					return -EACCES;
@@ -3863,6 +3865,8 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
				continue;
			if (type == STACK_ZERO)
				continue;
			if (type == STACK_INVALID && env->allow_uninit_stack)
				continue;
			verbose(env, "invalid read from stack off %d+%d size %d\n",
				off, i, size);
			return -EACCES;
@@ -5754,7 +5758,8 @@ static int check_stack_range_initialized(
		stype = &state->stack[spi].slot_type[slot % BPF_REG_SIZE];
		if (*stype == STACK_MISC)
			goto mark;
		if (*stype == STACK_ZERO) {
		if ((*stype == STACK_ZERO) ||
		    (*stype == STACK_INVALID && env->allow_uninit_stack)) {
			if (clobber) {
				/* helper can write anything into the stack */
				*stype = STACK_MISC;
@@ -13936,6 +13941,10 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
		if (old->stack[spi].slot_type[i % BPF_REG_SIZE] == STACK_INVALID)
			continue;
		if (env->allow_uninit_stack &&
		    old->stack[spi].slot_type[i % BPF_REG_SIZE] == STACK_MISC)
			continue;
		/* explored stack has more populated slots than current stack
		 * and these slots were used
		 */
+9 −0
Original line number Diff line number Diff line
// SPDX-License-Identifier: GPL-2.0

#include <test_progs.h>
#include "uninit_stack.skel.h"

void test_uninit_stack(void)
{
	RUN_TESTS(uninit_stack);
}
+4 −4
Original line number Diff line number Diff line
@@ -5,12 +5,12 @@
#include "bpf_misc.h"

struct Small {
	int x;
	long x;
};

struct Big {
	int x;
	int y;
	long x;
	long y;
};

__noinline int foo(const struct Big *big)
@@ -22,7 +22,7 @@ __noinline int foo(const struct Big *big)
}

SEC("cgroup_skb/ingress")
__failure __msg("invalid indirect read from stack")
__failure __msg("invalid indirect access to stack")
int global_func10(struct __sk_buff *skb)
{
	const struct Small small = {.x = skb->len };
+87 −0
Original line number Diff line number Diff line
// SPDX-License-Identifier: GPL-2.0

#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
#include "bpf_misc.h"

/* Read an uninitialized value from stack at a fixed offset */
SEC("socket")
__naked int read_uninit_stack_fixed_off(void *ctx)
{
	asm volatile ("					\
		r0 = 0;					\
		/* force stack depth to be 128 */	\
		*(u64*)(r10 - 128) = r1;		\
		r1 = *(u8 *)(r10 - 8 );			\
		r0 += r1;				\
		r1 = *(u8 *)(r10 - 11);			\
		r1 = *(u8 *)(r10 - 13);			\
		r1 = *(u8 *)(r10 - 15);			\
		r1 = *(u16*)(r10 - 16);			\
		r1 = *(u32*)(r10 - 32);			\
		r1 = *(u64*)(r10 - 64);			\
		/* read from a spill of a wrong size, it is a separate	\
		 * branch in check_stack_read_fixed_off()		\
		 */					\
		*(u32*)(r10 - 72) = r1;			\
		r1 = *(u64*)(r10 - 72);			\
		r0 = 0;					\
		exit;					\
"
		      ::: __clobber_all);
}

/* Read an uninitialized value from stack at a variable offset */
SEC("socket")
__naked int read_uninit_stack_var_off(void *ctx)
{
	asm volatile ("					\
		call %[bpf_get_prandom_u32];		\
		/* force stack depth to be 64 */	\
		*(u64*)(r10 - 64) = r0;			\
		r0 = -r0;				\
		/* give r0 a range [-31, -1] */		\
		if r0 s<= -32 goto exit_%=;		\
		if r0 s>= 0 goto exit_%=;		\
		/* access stack using r0 */		\
		r1 = r10;				\
		r1 += r0;				\
		r2 = *(u8*)(r1 + 0);			\
exit_%=:	r0 = 0;					\
		exit;					\
"
		      :
		      : __imm(bpf_get_prandom_u32)
		      : __clobber_all);
}

static __noinline void dummy(void) {}

/* Pass a pointer to uninitialized stack memory to a helper.
 * Passed memory block should be marked as STACK_MISC after helper call.
 */
SEC("socket")
__log_level(7) __msg("fp-104=mmmmmmmm")
__naked int helper_uninit_to_misc(void *ctx)
{
	asm volatile ("					\
		/* force stack depth to be 128 */	\
		*(u64*)(r10 - 128) = r1;		\
		r1 = r10;				\
		r1 += -128;				\
		r2 = 32;				\
		call %[bpf_trace_printk];		\
		/* Call to dummy() forces print_verifier_state(..., true),	\
		 * thus showing the stack state, matched by __msg().		\
		 */					\
		call %[dummy];				\
		r0 = 0;					\
		exit;					\
"
		      :
		      : __imm(bpf_trace_printk),
			__imm(dummy)
		      : __clobber_all);
}

char _license[] SEC("license") = "GPL";
+8 −5
Original line number Diff line number Diff line
@@ -2221,19 +2221,22 @@
	 * that fp-8 stack slot was unused in the fall-through
	 * branch and will accept the program incorrectly
	 */
	BPF_JMP_IMM(BPF_JGT, BPF_REG_1, 2, 2),
	BPF_EMIT_CALL(BPF_FUNC_get_prandom_u32),
	BPF_JMP_IMM(BPF_JGT, BPF_REG_0, 2, 2),
	BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
	BPF_JMP_IMM(BPF_JA, 0, 0, 0),
	BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
	BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
	BPF_LD_MAP_FD(BPF_REG_1, 0),
	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
	BPF_MOV64_IMM(BPF_REG_0, 0),
	BPF_EXIT_INSN(),
	},
	.fixup_map_hash_48b = { 6 },
	.errstr = "invalid indirect read from stack R2 off -8+0 size 8",
	.result = REJECT,
	.prog_type = BPF_PROG_TYPE_XDP,
	.fixup_map_hash_48b = { 7 },
	.errstr_unpriv = "invalid indirect read from stack R2 off -8+0 size 8",
	.result_unpriv = REJECT,
	/* in privileged mode reads from uninitialized stack locations are permitted */
	.result = ACCEPT,
},
{
	"calls: ctx read at start of subprog",
Loading