* [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs
@ 2019-11-07 17:00 Yonghong Song
2019-11-07 23:10 ` Song Liu
2019-11-11 13:07 ` Daniel Borkmann
0 siblings, 2 replies; 6+ messages in thread
From: Yonghong Song @ 2019-11-07 17:00 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
With latest llvm compiler, running test_progs will have
the following verifier failure for test_sysctl_loop1.o:
libbpf: load bpf program failed: Permission denied
libbpf: -- BEGIN DUMP LOG ---
libbpf:
invalid indirect read from stack var_off (0x0; 0xff)+196 size 7
...
libbpf: -- END LOG --
libbpf: failed to load program 'cgroup/sysctl'
libbpf: failed to load object 'test_sysctl_loop1.o'
The related bytecodes look below:
0000000000000308 LBB0_8:
97: r4 = r10
98: r4 += -288
99: r4 += r7
100: w8 &= 255
101: r1 = r10
102: r1 += -488
103: r1 += r8
104: r2 = 7
105: r3 = 0
106: call 106
107: w1 = w0
108: w1 += -1
109: if w1 > 6 goto -24 <LBB0_5>
110: w0 += w8
111: r7 += 8
112: w8 = w0
113: if r7 != 224 goto -17 <LBB0_8>
and source code:
for (i = 0; i < ARRAY_SIZE(tcp_mem); ++i) {
ret = bpf_strtoul(value + off, MAX_ULONG_STR_LEN, 0,
tcp_mem + i);
if (ret <= 0 || ret > MAX_ULONG_STR_LEN)
return 0;
off += ret & MAX_ULONG_STR_LEN;
}
Current verifier is not able to conclude register w0 before '+'
at insn 110 has a range of 1 to 7 and thinks it is from 0 - 255.
This leads to more conservative range for w8 at insn 112,
and later verifier complaint.
Let us workaound this issue until we found a compiler and/or
verifier solution. The workaround in this patch is
to make variable 'ret' volatile, which will force a reload
and then '&' operation to ensure better value range.
With this patch, I got the below byte code for the loop,
0000000000000328 LBB0_9:
101: r4 = r10
102: r4 += -288
103: r4 += r7
104: w8 &= 255
105: r1 = r10
106: r1 += -488
107: r1 += r8
108: r2 = 7
109: r3 = 0
110: call 106
111: *(u32 *)(r10 - 64) = r0
112: r1 = *(u32 *)(r10 - 64)
113: if w1 s< 1 goto -28 <LBB0_5>
114: r1 = *(u32 *)(r10 - 64)
115: if w1 s> 7 goto -30 <LBB0_5>
116: r1 = *(u32 *)(r10 - 64)
117: w1 &= 7
118: w1 += w8
119: r7 += 8
120: w8 = w1
121: if r7 != 224 goto -21 <LBB0_9>
Insn 117 did the '&' operation and we got more precise
value range for 'w8' at insn 120.
The test is happy then.
#3/17 test_sysctl_loop1.o:OK
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/progs/test_sysctl_loop1.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Changelog:
v1 -> v2:
- Added byte codes after the code change in the commit message
diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
index 608a06871572..d22e438198cf 100644
--- a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
+++ b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c
@@ -44,7 +44,10 @@ int sysctl_tcp_mem(struct bpf_sysctl *ctx)
unsigned long tcp_mem[TCP_MEM_LOOPS] = {};
char value[MAX_VALUE_STR_LEN];
unsigned char i, off = 0;
- int ret;
+ /* a workaround to prevent compiler from generating
+ * codes verifier cannot handle yet.
+ */
+ volatile int ret;
if (ctx->write)
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs
2019-11-07 17:00 [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs Yonghong Song
@ 2019-11-07 23:10 ` Song Liu
2019-11-11 13:07 ` Daniel Borkmann
1 sibling, 0 replies; 6+ messages in thread
From: Song Liu @ 2019-11-07 23:10 UTC (permalink / raw)
To: Yonghong Song; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
> On Nov 7, 2019, at 9:00 AM, Yonghong Song <yhs@fb.com> wrote:
>
[...]
> Let us workaound this issue until we found a compiler and/or
> verifier solution. The workaround in this patch is
> to make variable 'ret' volatile, which will force a reload
> and then '&' operation to ensure better value range.
> With this patch, I got the below byte code for the loop,
> 0000000000000328 LBB0_9:
> 101: r4 = r10
> 102: r4 += -288
> 103: r4 += r7
> 104: w8 &= 255
> 105: r1 = r10
> 106: r1 += -488
> 107: r1 += r8
> 108: r2 = 7
> 109: r3 = 0
> 110: call 106
> 111: *(u32 *)(r10 - 64) = r0
> 112: r1 = *(u32 *)(r10 - 64)
> 113: if w1 s< 1 goto -28 <LBB0_5>
> 114: r1 = *(u32 *)(r10 - 64)
> 115: if w1 s> 7 goto -30 <LBB0_5>
> 116: r1 = *(u32 *)(r10 - 64)
> 117: w1 &= 7
> 118: w1 += w8
> 119: r7 += 8
> 120: w8 = w1
> 121: if r7 != 224 goto -21 <LBB0_9>
> Insn 117 did the '&' operation and we got more precise
> value range for 'w8' at insn 120.
Thanks for adding this information.
> The test is happy then.
> #3/17 test_sysctl_loop1.o:OK
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs
2019-11-07 17:00 [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs Yonghong Song
2019-11-07 23:10 ` Song Liu
@ 2019-11-11 13:07 ` Daniel Borkmann
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2019-11-11 13:07 UTC (permalink / raw)
To: Yonghong Song, bpf; +Cc: Alexei Starovoitov, kernel-team
On 11/7/19 6:00 PM, Yonghong Song wrote:
> With latest llvm compiler, running test_progs will have
> the following verifier failure for test_sysctl_loop1.o:
> libbpf: load bpf program failed: Permission denied
> libbpf: -- BEGIN DUMP LOG ---
> libbpf:
> invalid indirect read from stack var_off (0x0; 0xff)+196 size 7
> ...
> libbpf: -- END LOG --
> libbpf: failed to load program 'cgroup/sysctl'
> libbpf: failed to load object 'test_sysctl_loop1.o'
>
> The related bytecodes look below:
> 0000000000000308 LBB0_8:
> 97: r4 = r10
> 98: r4 += -288
> 99: r4 += r7
> 100: w8 &= 255
> 101: r1 = r10
> 102: r1 += -488
> 103: r1 += r8
> 104: r2 = 7
> 105: r3 = 0
> 106: call 106
> 107: w1 = w0
> 108: w1 += -1
> 109: if w1 > 6 goto -24 <LBB0_5>
> 110: w0 += w8
> 111: r7 += 8
> 112: w8 = w0
[...]
Applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs
@ 2020-04-28 2:56 Ma Xinjian
2020-04-28 6:13 ` Yonghong Song
0 siblings, 1 reply; 6+ messages in thread
From: Ma Xinjian @ 2020-04-28 2:56 UTC (permalink / raw)
To: yhs, ast, bpf, daniel, kernel-team; +Cc: Li, Philip
Hi Yonghong.
During our team's test, we find a similar issue. when run test_sysctl of
bpf, we met error:
```
libbpf: -- END LOG --
libbpf: failed to load program 'cgroup/sysctl'
libbpf: failed to load object './test_sysctl_prog.o'
(test_sysctl.c:1490: errno: Permission denied) >>> Loading program
(./test_sysctl_prog.o) error.
```
Testing env: "Debian GNU/Linux 9 (stretch)"
kernel: 5.6 5.7-rc1 5.7-rc2 5.7-rc3
clang/llvm version: v11.0.0.
we have tested a log of commits, following commits are part of them
drwxrwxr-x 2 root root 1 Apr 28 07:01
f30416fdde922eaa655934e050026930fefbd260
drwxrwxr-x 2 root root 2 Apr 26 11:38
10bc12588dac532fad044b2851dde8e7b9121e88
drwxrwxr-x 2 root root 1 Apr 26 07:01
969e7edd88ceb4791eb1cac22828290f0ae30b3d
drwxrwxr-x 2 root root 1 Apr 23 13:51
bbf386f02b05db017fda66875cc5edef70779244
drwxrwxr-x 2 root root 1 Apr 22 10:01
2de52422acf04662b45599f77c14ce1b2cec2b81
drwxrwxr-x 2 root root 1 Apr 21 07:07
a9b137f9ffba8cb25dfd7dd1fb613e8aac121b37
drwxrwxr-x 2 root root 1 Apr 17 07:01
40d139c620f83509fe18acbff5ec358298e99def
drwxrwxr-x 2 root root 1 Apr 16 07:02
bee6c234ed28ae7349cb83afa322dfd8394590ee
And I have tested, If I add following patch like you did, test_sysctl pass:
diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
b/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
index 2d0b0b82a78a..8e3da8d2e7c9 100644
--- a/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
+++ b/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
@@ -45,7 +45,10 @@ int sysctl_tcp_mem(struct bpf_sysctl *ctx)
unsigned long tcp_mem[3] = {0, 0, 0};
char value[MAX_VALUE_STR_LEN];
unsigned char i, off = 0;
- int ret;
+ /* a workaround to prevent compiler from generating
+ * codes verifier cannot handle yet.
+ */
+ volatile int ret;
if (ctx->write)
return 0;
root@vm-snb-15
/usr/src/perf_selftests-x86_64-rhel-7.6-kselftests-6a8b55ed4056ea5559ebe4f6a4b247f627870d4c/tools/testing/selftests/bpf#
./test_sysctl
...
Summary: 40 PASSED, 0 FAILED
--
Best Regards.
Ma Xinjian
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs
2020-04-28 2:56 Ma Xinjian
@ 2020-04-28 6:13 ` Yonghong Song
2020-04-28 22:33 ` Alexei Starovoitov
0 siblings, 1 reply; 6+ messages in thread
From: Yonghong Song @ 2020-04-28 6:13 UTC (permalink / raw)
To: Ma Xinjian, ast, bpf, daniel, kernel-team; +Cc: Li, Philip
On 4/27/20 7:56 PM, Ma Xinjian wrote:
> Hi Yonghong.
>
> During our team's test, we find a similar issue. when run test_sysctl of
> bpf, we met error:
>
> ```
>
> libbpf: -- END LOG --
> libbpf: failed to load program 'cgroup/sysctl'
> libbpf: failed to load object './test_sysctl_prog.o'
> (test_sysctl.c:1490: errno: Permission denied) >>> Loading program
> (./test_sysctl_prog.o) error.
>
> ```
>
> Testing env: "Debian GNU/Linux 9 (stretch)"
>
> kernel: 5.6 5.7-rc1 5.7-rc2 5.7-rc3
>
> clang/llvm version: v11.0.0.
>
> we have tested a log of commits, following commits are part of them
>
> drwxrwxr-x 2 root root 1 Apr 28 07:01
> f30416fdde922eaa655934e050026930fefbd260
> drwxrwxr-x 2 root root 2 Apr 26 11:38
> 10bc12588dac532fad044b2851dde8e7b9121e88
> drwxrwxr-x 2 root root 1 Apr 26 07:01
> 969e7edd88ceb4791eb1cac22828290f0ae30b3d
> drwxrwxr-x 2 root root 1 Apr 23 13:51
> bbf386f02b05db017fda66875cc5edef70779244
> drwxrwxr-x 2 root root 1 Apr 22 10:01
> 2de52422acf04662b45599f77c14ce1b2cec2b81
> drwxrwxr-x 2 root root 1 Apr 21 07:07
> a9b137f9ffba8cb25dfd7dd1fb613e8aac121b37
> drwxrwxr-x 2 root root 1 Apr 17 07:01
> 40d139c620f83509fe18acbff5ec358298e99def
>
> drwxrwxr-x 2 root root 1 Apr 16 07:02
> bee6c234ed28ae7349cb83afa322dfd8394590ee
>
>
> And I have tested, If I add following patch like you did, test_sysctl pass:
>
> diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
> b/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
> index 2d0b0b82a78a..8e3da8d2e7c9 100644
> --- a/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
> +++ b/tools/testing/selftests/bpf/progs/test_sysctl_prog.c
> @@ -45,7 +45,10 @@ int sysctl_tcp_mem(struct bpf_sysctl *ctx)
> unsigned long tcp_mem[3] = {0, 0, 0};
> char value[MAX_VALUE_STR_LEN];
> unsigned char i, off = 0;
> - int ret;
> + /* a workaround to prevent compiler from generating
> + * codes verifier cannot handle yet.
> + */
> + volatile int ret;
>
> if (ctx->write)
> return 0;
Right. This is related to alu32 mode. The detailed description
https://lore.kernel.org/bpf/20191107170045.2503480-1-yhs@fb.com/
We are still working on this, either a verifier solution or a compiler
workaround.
>
> root@vm-snb-15
> /usr/src/perf_selftests-x86_64-rhel-7.6-kselftests-6a8b55ed4056ea5559ebe4f6a4b247f627870d4c/tools/testing/selftests/bpf#
> ./test_sysctl
>
> ...
>
> Summary: 40 PASSED, 0 FAILED
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs
2020-04-28 6:13 ` Yonghong Song
@ 2020-04-28 22:33 ` Alexei Starovoitov
0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2020-04-28 22:33 UTC (permalink / raw)
To: Yonghong Song
Cc: Ma Xinjian, Alexei Starovoitov, bpf, Daniel Borkmann,
Kernel Team, Li, Philip
On Mon, Apr 27, 2020 at 11:14 PM Yonghong Song <yhs@fb.com> wrote:
> > - int ret;
> > + /* a workaround to prevent compiler from generating
> > + * codes verifier cannot handle yet.
> > + */
> > + volatile int ret;
> >
> > if (ctx->write)
> > return 0;
>
> Right. This is related to alu32 mode. The detailed description
> https://lore.kernel.org/bpf/20191107170045.2503480-1-yhs@fb.com/
>
> We are still working on this, either a verifier solution or a compiler
> workaround.
Thanks for the report.
I pushed the same fix for progs/test_sysctl_prog.c into bpf-next.
Now test_sysctl passes.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-28 22:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 17:00 [PATCH bpf-next v2] [tools/bpf] workaround a verifier failure for test_progs Yonghong Song
2019-11-07 23:10 ` Song Liu
2019-11-11 13:07 ` Daniel Borkmann
2020-04-28 2:56 Ma Xinjian
2020-04-28 6:13 ` Yonghong Song
2020-04-28 22:33 ` Alexei Starovoitov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.