From: Andrei Matei <andreimatei1@gmail.com>
To: bpf@vger.kernel.org
Cc: sunhao.th@gmail.com, andrii.nakryiko@gmail.com,
eddyz87@gmail.com, Andrei Matei <andreimatei1@gmail.com>,
Andrii Nakryiko <andrii@kernel.org>
Subject: [PATCH bpf-next v5 3/3] bpf: guard stack limits against 32bit overflow
Date: Wed, 6 Dec 2023 23:11:50 -0500 [thread overview]
Message-ID: <20231207041150.229139-4-andreimatei1@gmail.com> (raw)
In-Reply-To: <20231207041150.229139-1-andreimatei1@gmail.com>
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
Signed-off-by: Andrei Matei <andreimatei1@gmail.com>
Reported-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Acked-by: Andrii Nakryiko <andrii@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 137240681fa9..6832ed743765 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6577,7 +6577,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)
{
@@ -6606,7 +6606,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;
@@ -6619,7 +6619,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.40.1
next prev parent reply other threads:[~2023-12-07 4:12 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-07 4:11 [PATCH bpf-next v5 0/3] bpf: fix verification of indirect var-off stack access Andrei Matei
2023-12-07 4:11 ` [PATCH bpf-next v5 1/3] " Andrei Matei
2023-12-07 4:11 ` [PATCH bpf-next v5 2/3] bpf: add verifier regression test for previous patch Andrei Matei
2023-12-07 4:11 ` Andrei Matei [this message]
2023-12-07 19:10 ` [PATCH bpf-next v5 0/3] bpf: fix verification of indirect var-off stack access patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231207041150.229139-4-andreimatei1@gmail.com \
--to=andreimatei1@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=eddyz87@gmail.com \
--cc=sunhao.th@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox