Skip to content

Commit e80698b

Browse files
committed
Alexei Starovoitov says: ==================== pull-request: bpf 2023-07-19 We've added 4 non-merge commits during the last 1 day(s) which contain a total of 3 files changed, 55 insertions(+), 10 deletions(-). The main changes are: 1) Fix stack depth check in presence of async callbacks, from Kumar Kartikeya Dwivedi. 2) Fix BTI type used for freplace attached functions, from Alexander Duyck. * tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: bpf, arm64: Fix BTI type used for freplace attached functions selftests/bpf: Add more tests for check_max_stack_depth bug bpf: Repeat check_max_stack_depth for async callbacks bpf: Fix subprog idx logic in check_max_stack_depth ==================== Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents aa7cb37 + a3f25d6 commit e80698b

File tree

3 files changed

+55
-10
lines changed

3 files changed

+55
-10
lines changed

arch/arm64/net/bpf_jit_comp.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,13 @@ static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf)
322322
*
323323
*/
324324

325-
emit_bti(A64_BTI_C, ctx);
325+
/* bpf function may be invoked by 3 instruction types:
326+
* 1. bl, attached via freplace to bpf prog via short jump
327+
* 2. br, attached via freplace to bpf prog via long jump
328+
* 3. blr, working as a function pointer, used by emit_call.
329+
* So BTI_JC should used here to support both br and blr.
330+
*/
331+
emit_bti(A64_BTI_JC, ctx);
326332

327333
emit(A64_MOV(1, A64_R(9), A64_LR), ctx);
328334
emit(A64_NOP, ctx);

kernel/bpf/verifier.c

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5573,16 +5573,17 @@ static int update_stack_depth(struct bpf_verifier_env *env,
55735573
* Since recursion is prevented by check_cfg() this algorithm
55745574
* only needs a local stack of MAX_CALL_FRAMES to remember callsites
55755575
*/
5576-
static int check_max_stack_depth(struct bpf_verifier_env *env)
5576+
static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx)
55775577
{
5578-
int depth = 0, frame = 0, idx = 0, i = 0, subprog_end;
55795578
struct bpf_subprog_info *subprog = env->subprog_info;
55805579
struct bpf_insn *insn = env->prog->insnsi;
5580+
int depth = 0, frame = 0, i, subprog_end;
55815581
bool tail_call_reachable = false;
55825582
int ret_insn[MAX_CALL_FRAMES];
55835583
int ret_prog[MAX_CALL_FRAMES];
55845584
int j;
55855585

5586+
i = subprog[idx].start;
55865587
process_func:
55875588
/* protect against potential stack overflow that might happen when
55885589
* bpf2bpf calls get combined with tailcalls. Limit the caller's stack
@@ -5621,7 +5622,7 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
56215622
continue_func:
56225623
subprog_end = subprog[idx + 1].start;
56235624
for (; i < subprog_end; i++) {
5624-
int next_insn;
5625+
int next_insn, sidx;
56255626

56265627
if (!bpf_pseudo_call(insn + i) && !bpf_pseudo_func(insn + i))
56275628
continue;
@@ -5631,14 +5632,14 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
56315632

56325633
/* find the callee */
56335634
next_insn = i + insn[i].imm + 1;
5634-
idx = find_subprog(env, next_insn);
5635-
if (idx < 0) {
5635+
sidx = find_subprog(env, next_insn);
5636+
if (sidx < 0) {
56365637
WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
56375638
next_insn);
56385639
return -EFAULT;
56395640
}
5640-
if (subprog[idx].is_async_cb) {
5641-
if (subprog[idx].has_tail_call) {
5641+
if (subprog[sidx].is_async_cb) {
5642+
if (subprog[sidx].has_tail_call) {
56425643
verbose(env, "verifier bug. subprog has tail_call and async cb\n");
56435644
return -EFAULT;
56445645
}
@@ -5647,6 +5648,7 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
56475648
continue;
56485649
}
56495650
i = next_insn;
5651+
idx = sidx;
56505652

56515653
if (subprog[idx].has_tail_call)
56525654
tail_call_reachable = true;
@@ -5682,6 +5684,22 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
56825684
goto continue_func;
56835685
}
56845686

5687+
static int check_max_stack_depth(struct bpf_verifier_env *env)
5688+
{
5689+
struct bpf_subprog_info *si = env->subprog_info;
5690+
int ret;
5691+
5692+
for (int i = 0; i < env->subprog_cnt; i++) {
5693+
if (!i || si[i].is_async_cb) {
5694+
ret = check_max_stack_depth_subprog(env, i);
5695+
if (ret < 0)
5696+
return ret;
5697+
}
5698+
continue;
5699+
}
5700+
return 0;
5701+
}
5702+
56855703
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
56865704
static int get_callee_stack_depth(struct bpf_verifier_env *env,
56875705
const struct bpf_insn *insn, int idx)

tools/testing/selftests/bpf/progs/async_stack_depth.c

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,16 @@ static int timer_cb(void *map, int *key, struct bpf_timer *timer)
2222
return buf[69];
2323
}
2424

25+
__attribute__((noinline))
26+
static int bad_timer_cb(void *map, int *key, struct bpf_timer *timer)
27+
{
28+
volatile char buf[300] = {};
29+
return buf[255] + timer_cb(NULL, NULL, NULL);
30+
}
31+
2532
SEC("tc")
26-
__failure __msg("combined stack size of 2 calls")
27-
int prog(struct __sk_buff *ctx)
33+
__failure __msg("combined stack size of 2 calls is 576. Too large")
34+
int pseudo_call_check(struct __sk_buff *ctx)
2835
{
2936
struct hmap_elem *elem;
3037
volatile char buf[256] = {};
@@ -37,4 +44,18 @@ int prog(struct __sk_buff *ctx)
3744
return bpf_timer_set_callback(&elem->timer, timer_cb) + buf[0];
3845
}
3946

47+
SEC("tc")
48+
__failure __msg("combined stack size of 2 calls is 608. Too large")
49+
int async_call_root_check(struct __sk_buff *ctx)
50+
{
51+
struct hmap_elem *elem;
52+
volatile char buf[256] = {};
53+
54+
elem = bpf_map_lookup_elem(&hmap, &(int){0});
55+
if (!elem)
56+
return 0;
57+
58+
return bpf_timer_set_callback(&elem->timer, bad_timer_cb) + buf[0];
59+
}
60+
4061
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)