| From 67f3b3b07b9bc5d545c0e8bf1bb3c910fe1836b6 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 6 Dec 2023 23:11:50 -0500 |
| Subject: bpf: Guard stack limits against 32bit overflow |
| |
| From: Andrei Matei <andreimatei1@gmail.com> |
| |
| [ Upstream commit 1d38a9ee81570c4bd61f557832dead4d6f816760 ] |
| |
| This patch promotes the arithmetic around checking stack bounds to be |
| done in the 64-bit domain, instead of the current 32bit. The arithmetic |
| implies adding together a 64-bit register with a int offset. The |
| register was checked to be below 1<<29 when it was variable, but not |
| when it was fixed. The offset either comes from an instruction (in which |
| case it is 16 bit), from another register (in which case the caller |
| checked it to be below 1<<29 [1]), or from the size of an argument to a |
| kfunc (in which case it can be a u32 [2]). Between the register being |
| inconsistently checked to be below 1<<29, and the offset being up to an |
| u32, it appears that we were open to overflowing the `int`s which were |
| currently used for arithmetic. |
| |
| [1] https://github.com/torvalds/linux/blob/815fb87b753055df2d9e50f6cd80eb10235fe3e9/kernel/bpf/verifier.c#L7494-L7498 |
| [2] https://github.com/torvalds/linux/blob/815fb87b753055df2d9e50f6cd80eb10235fe3e9/kernel/bpf/verifier.c#L11904 |
| |
| Reported-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> |
| Signed-off-by: Andrei Matei <andreimatei1@gmail.com> |
| Signed-off-by: Andrii Nakryiko <andrii@kernel.org> |
| Acked-by: Andrii Nakryiko <andrii@kernel.org> |
| Link: https://lore.kernel.org/bpf/20231207041150.229139-4-andreimatei1@gmail.com |
| Stable-dep-of: 6b4a64bafd10 ("bpf: Fix accesses to uninit stack slots") |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| kernel/bpf/verifier.c | 6 +++--- |
| 1 file changed, 3 insertions(+), 3 deletions(-) |
| |
| diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c |
| index 76834ecc59a9..47599505cdf8 100644 |
| --- a/kernel/bpf/verifier.c |
| +++ b/kernel/bpf/verifier.c |
| @@ -6371,7 +6371,7 @@ static int check_ptr_to_map_access(struct bpf_verifier_env *env, |
| * The minimum valid offset is -MAX_BPF_STACK for writes, and |
| * -state->allocated_stack for reads. |
| */ |
| -static int check_stack_slot_within_bounds(int off, |
| +static int check_stack_slot_within_bounds(s64 off, |
| struct bpf_func_state *state, |
| enum bpf_access_type t) |
| { |
| @@ -6400,7 +6400,7 @@ static int check_stack_access_within_bounds( |
| struct bpf_reg_state *regs = cur_regs(env); |
| struct bpf_reg_state *reg = regs + regno; |
| struct bpf_func_state *state = func(env, reg); |
| - int min_off, max_off; |
| + s64 min_off, max_off; |
| int err; |
| char *err_extra; |
| |
| @@ -6413,7 +6413,7 @@ static int check_stack_access_within_bounds( |
| err_extra = " write to"; |
| |
| if (tnum_is_const(reg->var_off)) { |
| - min_off = reg->var_off.value + off; |
| + min_off = (s64)reg->var_off.value + off; |
| max_off = min_off + access_size; |
| } else { |
| if (reg->smax_value >= BPF_MAX_VAR_OFF || |
| -- |
| 2.43.0 |
| |