From: Stanislav Fomichev <sdf@google.com>
To: bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
haoluo@google.com, jolsa@kernel.org,
Nick Desaulniers <ndesaulniers@google.com>
Subject: [PATCH bpf-next] selftests/bpf: Future-proof connect4_prog.c
Date: Thu, 7 Sep 2023 14:00:23 -0700 [thread overview]
Message-ID: <20230907210023.2467151-1-sdf@google.com> (raw)
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@google.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
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
next reply other threads:[~2023-09-07 21:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-07 21:00 Stanislav Fomichev [this message]
2023-09-08 23:42 ` [PATCH bpf-next] selftests/bpf: Future-proof connect4_prog.c Andrii Nakryiko
2023-09-09 0:28 ` Stanislav Fomichev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230907210023.2467151-1-sdf@google.com \
--to=sdf@google.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=martin.lau@linux.dev \
--cc=ndesaulniers@google.com \
--cc=song@kernel.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox