aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorAlexei Starovoitov <ast@kernel.org>2023-07-18 15:21:09 -0700
committerAlexei Starovoitov <ast@kernel.org>2023-07-18 15:21:10 -0700
commita8237cc87e3de1adb3f2f6a8056621e7e578cc00 (patch)
tree7c75374670917ca4c75817c5f4807208e6e6096d
parentocteontx2-pf: Dont allocate BPIDs for LBK interfaces (diff)
parentselftests/bpf: Add more tests for check_max_stack_depth bug (diff)
downloadwireguard-linux-a8237cc87e3de1adb3f2f6a8056621e7e578cc00.tar.xz
wireguard-linux-a8237cc87e3de1adb3f2f6a8056621e7e578cc00.zip
Merge branch 'two-more-fixes-for-check_max_stack_depth'
Kumar Kartikeya Dwivedi says: ==================== Two more fixes for check_max_stack_depth I noticed two more bugs while reviewing the code, description and examples available in the patches. One leads to incorrect subprog index to be stored in the frame stack maintained by the function (leading to incorrect tail_call_reachable marks, among other things). The other problem is missing exploration pass of other async callbacks when they are not called from the main prog. Call chains rooted at them can thus bypass the stack limits (32 call frames * max permitted stack depth per function). Changelog: ---------- v1 -> v2 v1: https://lore.kernel.org/bpf/20230713003118.1327943-1-memxor@gmail.com * Fix commit message for patch 2 (Alexei) ==================== Link: https://lore.kernel.org/r/20230717161530.1238-1-memxor@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
-rw-r--r--kernel/bpf/verifier.c32
-rw-r--r--tools/testing/selftests/bpf/progs/async_stack_depth.c25
2 files changed, 48 insertions, 9 deletions
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 930b5555cfd3..02a021c524ab 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5573,16 +5573,17 @@ static int update_stack_depth(struct bpf_verifier_env *env,
* Since recursion is prevented by check_cfg() this algorithm
* only needs a local stack of MAX_CALL_FRAMES to remember callsites
*/
-static int check_max_stack_depth(struct bpf_verifier_env *env)
+static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx)
{
- int depth = 0, frame = 0, idx = 0, i = 0, subprog_end;
struct bpf_subprog_info *subprog = env->subprog_info;
struct bpf_insn *insn = env->prog->insnsi;
+ int depth = 0, frame = 0, i, subprog_end;
bool tail_call_reachable = false;
int ret_insn[MAX_CALL_FRAMES];
int ret_prog[MAX_CALL_FRAMES];
int j;
+ i = subprog[idx].start;
process_func:
/* protect against potential stack overflow that might happen when
* bpf2bpf calls get combined with tailcalls. Limit the caller's stack
@@ -5621,7 +5622,7 @@ process_func:
continue_func:
subprog_end = subprog[idx + 1].start;
for (; i < subprog_end; i++) {
- int next_insn;
+ int next_insn, sidx;
if (!bpf_pseudo_call(insn + i) && !bpf_pseudo_func(insn + i))
continue;
@@ -5631,14 +5632,14 @@ continue_func:
/* find the callee */
next_insn = i + insn[i].imm + 1;
- idx = find_subprog(env, next_insn);
- if (idx < 0) {
+ sidx = find_subprog(env, next_insn);
+ if (sidx < 0) {
WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
next_insn);
return -EFAULT;
}
- if (subprog[idx].is_async_cb) {
- if (subprog[idx].has_tail_call) {
+ if (subprog[sidx].is_async_cb) {
+ if (subprog[sidx].has_tail_call) {
verbose(env, "verifier bug. subprog has tail_call and async cb\n");
return -EFAULT;
}
@@ -5647,6 +5648,7 @@ continue_func:
continue;
}
i = next_insn;
+ idx = sidx;
if (subprog[idx].has_tail_call)
tail_call_reachable = true;
@@ -5682,6 +5684,22 @@ continue_func:
goto continue_func;
}
+static int check_max_stack_depth(struct bpf_verifier_env *env)
+{
+ struct bpf_subprog_info *si = env->subprog_info;
+ int ret;
+
+ for (int i = 0; i < env->subprog_cnt; i++) {
+ if (!i || si[i].is_async_cb) {
+ ret = check_max_stack_depth_subprog(env, i);
+ if (ret < 0)
+ return ret;
+ }
+ continue;
+ }
+ return 0;
+}
+
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
static int get_callee_stack_depth(struct bpf_verifier_env *env,
const struct bpf_insn *insn, int idx)
diff --git a/tools/testing/selftests/bpf/progs/async_stack_depth.c b/tools/testing/selftests/bpf/progs/async_stack_depth.c
index 477ba950bb43..3517c0e01206 100644
--- a/tools/testing/selftests/bpf/progs/async_stack_depth.c
+++ b/tools/testing/selftests/bpf/progs/async_stack_depth.c
@@ -22,9 +22,16 @@ static int timer_cb(void *map, int *key, struct bpf_timer *timer)
return buf[69];
}
+__attribute__((noinline))
+static int bad_timer_cb(void *map, int *key, struct bpf_timer *timer)
+{
+ volatile char buf[300] = {};
+ return buf[255] + timer_cb(NULL, NULL, NULL);
+}
+
SEC("tc")
-__failure __msg("combined stack size of 2 calls")
-int prog(struct __sk_buff *ctx)
+__failure __msg("combined stack size of 2 calls is 576. Too large")
+int pseudo_call_check(struct __sk_buff *ctx)
{
struct hmap_elem *elem;
volatile char buf[256] = {};
@@ -37,4 +44,18 @@ int prog(struct __sk_buff *ctx)
return bpf_timer_set_callback(&elem->timer, timer_cb) + buf[0];
}
+SEC("tc")
+__failure __msg("combined stack size of 2 calls is 608. Too large")
+int async_call_root_check(struct __sk_buff *ctx)
+{
+ struct hmap_elem *elem;
+ volatile char buf[256] = {};
+
+ elem = bpf_map_lookup_elem(&hmap, &(int){0});
+ if (!elem)
+ return 0;
+
+ return bpf_timer_set_callback(&elem->timer, bad_timer_cb) + buf[0];
+}
+
char _license[] SEC("license") = "GPL";