With the new internal clang version I see the following optimization
that makes connect4 program unverifiable.
The following code:
int do_bind()
{
if (bpf_bind() != 0)
return 0;
return 1;
}
int connect_v4_prog()
{
return do_bind() ? 1 : 0;
}
Becomes:
int do_bind()
{
if (bpf_bind() != 0)
return 0;
return 1;
}
int connect_v4_prog()
{
return do_bind();
}
IOW, looks like clang is able to see that do_bind returns only 0 and
1 and the extra branch around 'return do_bind' is not needed.
This, however, seems to break the verifier, which assumes that
bpf2bpf calls can return 0-0xffffffff.
Note, I can produce those programs only with the internal fork of clang.
The latest one from git still produced correct bytecode. It might be
some options/optimizations that we enable and that are still
disabled for the general upstream users, not sure. I've desided
to send this patch out anyway since it seems like a correct optimization
the compiler might do.
So to be future-proof, reshape the code a bit to return bpf_bind
result directly. This will not give any hint to the clang about
the return value and will force it generate that '? 1: 0' branch
at the callee.
Good program:
0000000000000000 <do_bind>:
0: b4 02 00 00 7f 00 00 04 w2 = 0x400007f
1: 63 2a f4 ff 00 00 00 00 *(u32 *)(r10 - 0xc) = r2
2: b4 02 00 00 02 00 00 00 w2 = 0x2
3: 63 2a f0 ff 00 00 00 00 *(u32 *)(r10 - 0x10) = r2
4: b7 02 00 00 00 00 00 00 r2 = 0x0
5: 63 2a fc ff 00 00 00 00 *(u32 *)(r10 - 0x4) = r2
6: 63 2a f8 ff 00 00 00 00 *(u32 *)(r10 - 0x8) = r2
7: bf a2 00 00 00 00 00 00 r2 = r10
8: 07 02 00 00 f0 ff ff ff r2 += -0x10
9: b4 03 00 00 10 00 00 00 w3 = 0x10
10: 85 00 00 00 40 00 00 00 call 0x40
11: bf 01 00 00 00 00 00 00 r1 = r0
12: b4 00 00 00 01 00 00 00 w0 = 0x1
13: 15 01 01 00 00 00 00 00 if r1 == 0x0 goto +0x1 <LBB0_2>
14: b4 00 00 00 00 00 00 00 w0 = 0x0
00000000000001b0 <LBB1_30>:
54: bc 60 00 00 00 00 00 00 w0 = w6
55: 95 00 00 00 00 00 00 00 exit
0000000000000578 <LBB1_28>:
...
180: 85 10 00 00 ff ff ff ff call -0x1
181: b4 06 00 00 01 00 00 00 w6 = 0x1
182: 56 00 7f ff 00 00 00 00 if w0 != 0x0 goto -0x81 <LBB1_30>
183: b4 06 00 00 00 00 00 00 w6 = 0x0
184: 05 00 7d ff 00 00 00 00 goto -0x83 <LBB1_30>
Bad program:
0000000000000000 <do_bind>:
0: b4 02 00 00 7f 00 00 04 w2 = 0x400007f
1: 63 2a f4 ff 00 00 00 00 *(u32 *)(r10 - 0xc) = r2
2: b4 02 00 00 02 00 00 00 w2 = 0x2
3: 63 2a f0 ff 00 00 00 00 *(u32 *)(r10 - 0x10) = r2
4: b7 02 00 00 00 00 00 00 r2 = 0x0
5: 63 2a fc ff 00 00 00 00 *(u32 *)(r10 - 0x4) = r2
6: 63 2a f8 ff 00 00 00 00 *(u32 *)(r10 - 0x8) = r2
7: bf a2 00 00 00 00 00 00 r2 = r10
8: 07 02 00 00 f0 ff ff ff r2 += -0x10
9: b4 03 00 00 10 00 00 00 w3 = 0x10
10: 85 00 00 00 40 00 00 00 call 0x40
11: bf 01 00 00 00 00 00 00 r1 = r0
12: b4 00 00 00 01 00 00 00 w0 = 0x1
13: 15 01 01 00 00 00 00 00 if r1 == 0x0 goto +0x1 <LBB0_2>
14: b4 00 00 00 00 00 00 00 w0 = 0x0
00000000000001b0 <LBB1_3>:
54: bc 60 00 00 00 00 00 00 w0 = w6
55: 95 00 00 00 00 00 00 00 exit
0000000000000578 <LBB1_28>:
...
180: 85 10 00 00 ff ff ff ff call -0x1
181: bc 06 00 00 00 00 00 00 w6 = w0
182: 05 00 7f ff 00 00 00 00 goto -0x81 <LBB1_3>
Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
---
tools/testing/selftests/bpf/progs/connect4_prog.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/connect4_prog.c b/tools/testing/selftests/bpf/progs/connect4_prog.c
index 7ef49ec04838..b7fc46a0787b 100644
--- a/tools/testing/selftests/bpf/progs/connect4_prog.c
+++ b/tools/testing/selftests/bpf/progs/connect4_prog.c
@@ -41,10 +41,7 @@ int do_bind(struct bpf_sock_addr *ctx)
sa.sin_port = bpf_htons(0);
sa.sin_addr.s_addr = bpf_htonl(SRC_REWRITE_IP4);
- if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
- return 0;
-
- return 1;
+ return bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa));
}
static __inline int verify_cc(struct bpf_sock_addr *ctx,
@@ -194,7 +191,7 @@ int connect_v4_prog(struct bpf_sock_addr *ctx)
ctx->user_ip4 = bpf_htonl(DST_REWRITE_IP4);
ctx->user_port = bpf_htons(DST_REWRITE_PORT4);
- return do_bind(ctx) ? 1 : 0;
+ return do_bind(ctx) ? 0 : 1;
}
char _license[] SEC("license") = "GPL";
--
2.42.0.283.g2d96d420d3-goog
