Skip to content

Commit 61654df

Browse files
teckiKernel Patches Daemon
authored andcommitted
bpf: test the proper verification of tail calls
Three 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. Signed-off-by: Martin Teichmann <[email protected]> Acked-by: Eduard Zingerman <[email protected]>
1 parent 6e1bab7 commit 61654df

File tree

2 files changed

+90
-2
lines changed

2 files changed

+90
-2
lines changed

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: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,4 +793,57 @@ __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("13: (85) call bpf_tail_call#12")
823+
__msg("mark_precise: frame1: last_idx 13 first_idx 0 subseq_idx -1 ")
824+
__msg("returning from callee:")
825+
__msg("frame1: R0=scalar() R6=3 R10=fp0")
826+
__msg("to caller at 4:")
827+
__msg("R0=scalar() R6=map_value(map=.data.vals,ks=4,vs=16) R10=fp0")
828+
__msg("6: (0f) r1 += r0")
829+
__msg("mark_precise: frame0: regs=r0 stack= before 5: (bf) r1 = r6")
830+
__msg("mark_precise: frame0: regs=r0 stack= before 4: (27) r0 *= 4")
831+
__msg("mark_precise: frame0: parent state regs=r0 stack=: R0=Pscalar() R6=map_value(map=.data.vals,ks=4,vs=16) R10=fp0")
832+
__msg("math between map_value pointer and register with unbounded min value is not allowed")
833+
__naked int subprog_result_tail_call(void)
834+
{
835+
asm volatile (
836+
"r2 = 3;"
837+
"call identity_tail_call;"
838+
"r0 *= 4;"
839+
"r1 = %[vals];"
840+
"r1 += r0;"
841+
"r0 = *(u32 *)(r1 + 0);"
842+
"exit;"
843+
:
844+
: __imm_ptr(vals)
845+
: __clobber_common
846+
);
847+
}
848+
796849
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)