* [PATCH AUTOSEL 5.4 08/10] selftests/bpf: Fix array access with signed variable test
[not found] <20201220033457.2728519-1-sashal@kernel.org>
@ 2020-12-20 3:34 ` Sasha Levin
2020-12-20 12:05 ` Jean-Philippe Brucker
0 siblings, 1 reply; 3+ messages in thread
From: Sasha Levin @ 2020-12-20 3:34 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jean-Philippe Brucker, John Fastabend, Alexei Starovoitov,
Sasha Levin, linux-kselftest, netdev, bpf
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
[ Upstream commit 77ce220c0549dcc3db8226c61c60e83fc59dfafc ]
The test fails because of a recent fix to the verifier, even though this
program is valid. In details what happens is:
7: (61) r1 = *(u32 *)(r0 +0)
Load a 32-bit value, with signed bounds [S32_MIN, S32_MAX]. The bounds
of the 64-bit value are [0, U32_MAX]...
8: (65) if r1 s> 0xffffffff goto pc+1
... therefore this is always true (the operand is sign-extended).
10: (b4) w2 = 11
11: (6d) if r2 s> r1 goto pc+1
When true, the 64-bit bounds become [0, 10]. The 32-bit bounds are still
[S32_MIN, 10].
13: (64) w1 <<= 2
Because this is a 32-bit operation, the verifier propagates the new
32-bit bounds to the 64-bit ones, and the knowledge gained from insn 11
is lost.
14: (0f) r0 += r1
15: (7a) *(u64 *)(r0 +0) = 4
Then the verifier considers r0 unbounded here, rejecting the test. To
make the test work, change insn 8 to check the sign of the 32-bit value.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
tools/testing/selftests/bpf/verifier/array_access.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/verifier/array_access.c b/tools/testing/selftests/bpf/verifier/array_access.c
index f3c33e128709b..a80d806ead15f 100644
--- a/tools/testing/selftests/bpf/verifier/array_access.c
+++ b/tools/testing/selftests/bpf/verifier/array_access.c
@@ -68,7 +68,7 @@
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 9),
BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
- BPF_JMP_IMM(BPF_JSGT, BPF_REG_1, 0xffffffff, 1),
+ BPF_JMP32_IMM(BPF_JSGT, BPF_REG_1, 0xffffffff, 1),
BPF_MOV32_IMM(BPF_REG_1, 0),
BPF_MOV32_IMM(BPF_REG_2, MAX_ENTRIES),
BPF_JMP_REG(BPF_JSGT, BPF_REG_2, BPF_REG_1, 1),
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH AUTOSEL 5.4 08/10] selftests/bpf: Fix array access with signed variable test
2020-12-20 3:34 ` [PATCH AUTOSEL 5.4 08/10] selftests/bpf: Fix array access with signed variable test Sasha Levin
@ 2020-12-20 12:05 ` Jean-Philippe Brucker
2020-12-21 19:09 ` Sasha Levin
0 siblings, 1 reply; 3+ messages in thread
From: Jean-Philippe Brucker @ 2020-12-20 12:05 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, John Fastabend, Alexei Starovoitov,
linux-kselftest, netdev, bpf
Hi,
On Sat, Dec 19, 2020 at 10:34:55PM -0500, Sasha Levin wrote:
> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
>
> [ Upstream commit 77ce220c0549dcc3db8226c61c60e83fc59dfafc ]
>
> The test fails because of a recent fix to the verifier, even though this
That fix is commit b02709587ea3 ("bpf: Fix propagation of 32-bit signed
bounds from 64-bit bounds.") upstream, which only needed backport to 5.9.
So although backporting this patch to 5.4 shouldn't break anything, I
wouldn't bother.
Thanks,
Jean
> program is valid. In details what happens is:
>
> 7: (61) r1 = *(u32 *)(r0 +0)
>
> Load a 32-bit value, with signed bounds [S32_MIN, S32_MAX]. The bounds
> of the 64-bit value are [0, U32_MAX]...
>
> 8: (65) if r1 s> 0xffffffff goto pc+1
>
> ... therefore this is always true (the operand is sign-extended).
>
> 10: (b4) w2 = 11
> 11: (6d) if r2 s> r1 goto pc+1
>
> When true, the 64-bit bounds become [0, 10]. The 32-bit bounds are still
> [S32_MIN, 10].
>
> 13: (64) w1 <<= 2
>
> Because this is a 32-bit operation, the verifier propagates the new
> 32-bit bounds to the 64-bit ones, and the knowledge gained from insn 11
> is lost.
>
> 14: (0f) r0 += r1
> 15: (7a) *(u64 *)(r0 +0) = 4
>
> Then the verifier considers r0 unbounded here, rejecting the test. To
> make the test work, change insn 8 to check the sign of the 32-bit value.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> tools/testing/selftests/bpf/verifier/array_access.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/verifier/array_access.c b/tools/testing/selftests/bpf/verifier/array_access.c
> index f3c33e128709b..a80d806ead15f 100644
> --- a/tools/testing/selftests/bpf/verifier/array_access.c
> +++ b/tools/testing/selftests/bpf/verifier/array_access.c
> @@ -68,7 +68,7 @@
> BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 9),
> BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
> - BPF_JMP_IMM(BPF_JSGT, BPF_REG_1, 0xffffffff, 1),
> + BPF_JMP32_IMM(BPF_JSGT, BPF_REG_1, 0xffffffff, 1),
> BPF_MOV32_IMM(BPF_REG_1, 0),
> BPF_MOV32_IMM(BPF_REG_2, MAX_ENTRIES),
> BPF_JMP_REG(BPF_JSGT, BPF_REG_2, BPF_REG_1, 1),
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH AUTOSEL 5.4 08/10] selftests/bpf: Fix array access with signed variable test
2020-12-20 12:05 ` Jean-Philippe Brucker
@ 2020-12-21 19:09 ` Sasha Levin
0 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2020-12-21 19:09 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: linux-kernel, stable, John Fastabend, Alexei Starovoitov,
linux-kselftest, netdev, bpf
On Sun, Dec 20, 2020 at 01:05:19PM +0100, Jean-Philippe Brucker wrote:
>Hi,
>
>On Sat, Dec 19, 2020 at 10:34:55PM -0500, Sasha Levin wrote:
>> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>
>> [ Upstream commit 77ce220c0549dcc3db8226c61c60e83fc59dfafc ]
>>
>> The test fails because of a recent fix to the verifier, even though this
>
>That fix is commit b02709587ea3 ("bpf: Fix propagation of 32-bit signed
>bounds from 64-bit bounds.") upstream, which only needed backport to 5.9.
>So although backporting this patch to 5.4 shouldn't break anything, I
>wouldn't bother.
I'll drop it from 5.4, thanks!
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-21 19:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20201220033457.2728519-1-sashal@kernel.org>
2020-12-20 3:34 ` [PATCH AUTOSEL 5.4 08/10] selftests/bpf: Fix array access with signed variable test Sasha Levin
2020-12-20 12:05 ` Jean-Philippe Brucker
2020-12-21 19:09 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox