Skip to content

Commit 291bd20

Browse files
committed
bpf: Test the proper verification of tail calls
Four tests are added: - invalidate_pkt_pointers_by_tail_call checks that one can use the packet pointer after a tail call. This was originally possible and also poses not problems, but was made impossible by 1a4607f. - invalidate_pkt_pointers_by_static_tail_call tests a corner case found by Eduard Zingerman during the discussion of the original fix, which was broken in that fix. - subprog_result_tail_call tests that precision propagation works correctly across tail calls. This did not work before. - caller_stack_write_tail_call tests that the live stack is correctly tracked for a tail call.
1 parent fa37294 commit 291bd20

File tree

3 files changed

+130
-2
lines changed

3 files changed

+130
-2
lines changed

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,3 +292,49 @@ __naked void syzbot_postorder_bug1(void)
292292
"exit;"
293293
::: __clobber_all);
294294
}
295+
296+
struct {
297+
__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
298+
__uint(max_entries, 1);
299+
__type(key, __u32);
300+
__type(value, __u32);
301+
} map_array SEC(".maps");
302+
303+
SEC("socket")
304+
__log_level(2)
305+
__msg("10: (85) call bpf_tail_call#12")
306+
__msg("(2,5) frame 0 insn 5 +written -16")
307+
__msg("(2,5) live stack update done in 2 iterations")
308+
__msg("4: (95) exit")
309+
__msg("(0) frame 0 insn 3 +live -24")
310+
__msg("(0) live stack update done in 2 iterations")
311+
__msg("13: (95) exit")
312+
__msg("(2,5) frame 0 insn 11 +written -8")
313+
__msg("(2,5) live stack update done in 2 iterations")
314+
__naked unsigned long caller_stack_write_tail_call(void)
315+
{
316+
asm volatile (
317+
"r2 = r10;"
318+
"r2 += -8;"
319+
"call write_tail_call;"
320+
"r0 = *(u64 *)(r10 - 24);"
321+
"exit;"
322+
::: __clobber_all);
323+
}
324+
325+
static __used __naked unsigned long write_tail_call(void)
326+
{
327+
asm volatile (
328+
"*(u64 *)(r2 - 8) = 7;"
329+
"r6 = r2;"
330+
"r2 = %[map_array] ll;"
331+
"r3 = 0;"
332+
"call %[bpf_tail_call];"
333+
"*(u64 *)(r6 + 0) = 7;"
334+
"r0 = 0;"
335+
"exit;"
336+
:
337+
: __imm(bpf_tail_call),
338+
__imm_addr(map_array)
339+
: __clobber_all);
340+
}

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,10 +1117,17 @@ int tail_call(struct __sk_buff *sk)
11171117
return 0;
11181118
}
11191119

1120-
/* Tail calls invalidate packet pointers. */
1120+
static __noinline
1121+
int static_tail_call(struct __sk_buff *sk)
1122+
{
1123+
bpf_tail_call_static(sk, &jmp_table, 0);
1124+
return 0;
1125+
}
1126+
1127+
/* Tail calls in sub-programs invalidate packet pointers. */
11211128
SEC("tc")
11221129
__failure __msg("invalid mem access")
1123-
int invalidate_pkt_pointers_by_tail_call(struct __sk_buff *sk)
1130+
int invalidate_pkt_pointers_by_global_tail_call(struct __sk_buff *sk)
11241131
{
11251132
int *p = (void *)(long)sk->data;
11261133

@@ -1131,4 +1138,32 @@ int invalidate_pkt_pointers_by_tail_call(struct __sk_buff *sk)
11311138
return TCX_PASS;
11321139
}
11331140

1141+
/* Tail calls in static sub-programs invalidate packet pointers. */
1142+
SEC("tc")
1143+
__failure __msg("invalid mem access")
1144+
int invalidate_pkt_pointers_by_static_tail_call(struct __sk_buff *sk)
1145+
{
1146+
int *p = (void *)(long)sk->data;
1147+
1148+
if ((void *)(p + 1) > (void *)(long)sk->data_end)
1149+
return TCX_DROP;
1150+
static_tail_call(sk);
1151+
*p = 42; /* this is unsafe */
1152+
return TCX_PASS;
1153+
}
1154+
1155+
/* Direct tail calls do not invalidate packet pointers. */
1156+
SEC("tc")
1157+
__success
1158+
int invalidate_pkt_pointers_by_tail_call(struct __sk_buff *sk)
1159+
{
1160+
int *p = (void *)(long)sk->data;
1161+
1162+
if ((void *)(p + 1) > (void *)(long)sk->data_end)
1163+
return TCX_DROP;
1164+
bpf_tail_call_static(sk, &jmp_table, 0);
1165+
*p = 42; /* this is NOT unsafe: tail calls don't return */
1166+
return TCX_PASS;
1167+
}
1168+
11341169
char _license[] SEC("license") = "GPL";

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,4 +793,51 @@ __naked int stack_slot_aliases_precision(void)
793793
);
794794
}
795795

796+
struct {
797+
__uint(type, BPF_MAP_TYPE_PROG_ARRAY);
798+
__uint(max_entries, 1);
799+
__type(key, __u32);
800+
__type(value, __u32);
801+
} map_array SEC(".maps");
802+
803+
__naked __noinline __used
804+
static unsigned long identity_tail_call(void)
805+
{
806+
/* the simplest identity function involving a tail call */
807+
asm volatile (
808+
"r6 = r2;"
809+
"r2 = %[map_array] ll;"
810+
"r3 = 0;"
811+
"call %[bpf_tail_call];"
812+
"r0 = r6;"
813+
"exit;"
814+
:
815+
: __imm(bpf_tail_call),
816+
__imm_addr(map_array)
817+
: __clobber_all);
818+
}
819+
820+
SEC("?raw_tp")
821+
__failure __log_level(2)
822+
__msg("6: (0f) r1 += r0")
823+
__msg("mark_precise: frame0: regs=r0 stack= before 5: (bf) r1 = r6")
824+
__msg("mark_precise: frame0: regs=r0 stack= before 4: (27) r0 *= 4")
825+
__msg("mark_precise: frame0: parent state regs=r0 stack=: R0=Pscalar() R6=map_value(map=.data.vals,ks=4,vs=16) R10=fp0")
826+
__msg("math between map_value pointer and register with unbounded min value is not allowed")
827+
__naked int subprog_result_tail_call(void)
828+
{
829+
asm volatile (
830+
"r2 = 3;"
831+
"call identity_tail_call;"
832+
"r0 *= 4;"
833+
"r1 = %[vals];"
834+
"r1 += r0;"
835+
"r0 = *(u32 *)(r1 + 0);"
836+
"exit;"
837+
:
838+
: __imm_ptr(vals)
839+
: __clobber_common
840+
);
841+
}
842+
796843
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)