diff mbox series

[bpf-next,v5,3/3] bpf: minor cleanup around stack bounds

Message ID 20231208032519.260451-4-andreimatei1@gmail.com (mailing list archive)
State Accepted
Commit 2929bfac006d8f8e22b307d04e0d71bcb84db698
Delegated to: BPF
Headers show
Series bpf: fix accesses to uninit stack slots | expand

Checks

Context Check Description
bpf/vmtest-bpf-PR fail merge-conflict
netdev/tree_selection success Clearly marked for bpf
netdev/apply fail Patch does not apply to bpf

Commit Message

Andrei Matei Dec. 8, 2023, 3:25 a.m. UTC
Push the rounding up of stack offsets into the function responsible for
growing the stack, rather than relying on all the callers to do it.
Uncertainty about whether the callers did it or not tripped up people in
a previous review.

Signed-off-by: Andrei Matei <andreimatei1@gmail.com>
---
 kernel/bpf/verifier.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Eduard Zingerman Dec. 8, 2023, 9:10 p.m. UTC | #1
On Thu, 2023-12-07 at 22:25 -0500, Andrei Matei wrote:
> Push the rounding up of stack offsets into the function responsible for
> growing the stack, rather than relying on all the callers to do it.
> Uncertainty about whether the callers did it or not tripped up people in
> a previous review.
> 
> Signed-off-by: Andrei Matei <andreimatei1@gmail.com>

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
diff mbox series

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index de1e29fa467e..fb690539d5f6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1264,7 +1264,11 @@  static int resize_reference_state(struct bpf_func_state *state, size_t n)
  */
 static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state *state, int size)
 {
-	size_t old_n = state->allocated_stack / BPF_REG_SIZE, n = size / BPF_REG_SIZE;
+	size_t old_n = state->allocated_stack / BPF_REG_SIZE, n;
+
+	/* The stack size is always a multiple of BPF_REG_SIZE. */
+	size = round_up(size, BPF_REG_SIZE);
+	n = size / BPF_REG_SIZE;
 
 	if (old_n >= n)
 		return 0;
@@ -6638,7 +6642,10 @@  static int check_stack_access_within_bounds(
 		return err;
 	}
 
-	return grow_stack_state(env, state, round_up(-min_off, BPF_REG_SIZE));
+	/* Note that there is no stack access with offset zero, so the needed stack
+	 * size is -min_off, not -min_off+1.
+	 */
+	return grow_stack_state(env, state, -min_off /* size */);
 }
 
 /* check whether memory at (regno + off) is accessible for t = (read | write)