diff mbox series

[bpf-next] selftests/bpf: Remove two infinite loop bound check cases

Message ID 20230327153538.850440-1-xukuohai@huaweicloud.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [bpf-next] selftests/bpf: Remove two infinite loop bound check cases | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 20 this patch: 20
netdev/cc_maintainers fail 3 blamed authors not CCed: andrii@kernel.org xukuohai@huawei.com john.fastabend@gmail.com; 16 maintainers not CCed: mykolal@fb.com andrii@kernel.org song@kernel.org shuah@kernel.org sdf@google.com haoluo@google.com yhs@fb.com kuba@kernel.org xukuohai@huawei.com john.fastabend@gmail.com kpsingh@kernel.org jolsa@kernel.org hawk@kernel.org netdev@vger.kernel.org martin.lau@linux.dev davem@davemloft.net
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 20 this patch: 20
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 34 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-31 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-33 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-34 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain_full }}
bpf/vmtest-bpf-next-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-8 success Logs for set-matrix

Commit Message

Xu Kuohai March 27, 2023, 3:35 p.m. UTC 
From: Xu Kuohai <xukuohai@huawei.com>

The two infinite loop bound check cases added by commit
1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
take a long time to execute but don't add much value.

Remove them to reduce run time of test_verifier.

Fixes: 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
---
 tools/testing/selftests/bpf/verifier/bounds.c | 50 -------------------
 1 file changed, 50 deletions(-)

Comments

Andrii Nakryiko March 27, 2023, 3:20 a.m. UTC | #1 
On Sun, Mar 26, 2023 at 7:45 PM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
>
> From: Xu Kuohai <xukuohai@huawei.com>
>
> The two infinite loop bound check cases added by commit
> 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
> take a long time to execute but don't add much value.
>
> Remove them to reduce run time of test_verifier.

Summary: 2042 PASSED, 0 SKIPPED, 1 FAILED

real    0m4.780s
user    0m0.458s
sys     0m3.871s


5 seconds isn't such a long time, especially when we compare it to
test_progs (even with parallelization).

>
> Fixes: 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
> ---
>  tools/testing/selftests/bpf/verifier/bounds.c | 50 -------------------
>  1 file changed, 50 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/verifier/bounds.c b/tools/testing/selftests/bpf/verifier/bounds.c
> index 74b1917d4208..515a8222f08f 100644
> --- a/tools/testing/selftests/bpf/verifier/bounds.c
> +++ b/tools/testing/selftests/bpf/verifier/bounds.c
> @@ -777,31 +777,6 @@
>         .result = ACCEPT,
>         .prog_type = BPF_PROG_TYPE_XDP,
>  },
> -{
> -       "bound check with JMP_JSLT for crossing 64-bit signed boundary",
> -       .insns = {
> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 8),
> -
> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
> -       BPF_LD_IMM64(BPF_REG_0, 0x7fffffffffffff10),
> -       BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
> -
> -       BPF_LD_IMM64(BPF_REG_0, 0x8000000000000000),
> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 1),
> -       /* r1 signed range is [S64_MIN, S64_MAX] */
> -       BPF_JMP_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
> -
> -       BPF_MOV64_IMM(BPF_REG_0, 0),
> -       BPF_EXIT_INSN(),
> -       },
> -       .errstr = "BPF program is too large",
> -       .result = REJECT,
> -       .prog_type = BPF_PROG_TYPE_XDP,
> -},
>  {
>         "bound check for loop upper bound greater than U32_MAX",
>         .insns = {
> @@ -849,28 +824,3 @@
>         .result = ACCEPT,
>         .prog_type = BPF_PROG_TYPE_XDP,
>  },
> -{
> -       "bound check with JMP32_JSLT for crossing 32-bit signed boundary",
> -       .insns = {
> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 6),
> -
> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
> -       BPF_MOV32_IMM(BPF_REG_0, 0x7fffff10),
> -       BPF_ALU32_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
> -
> -       BPF_MOV32_IMM(BPF_REG_0, 0x80000000),
> -       BPF_ALU32_IMM(BPF_ADD, BPF_REG_0, 1),
> -       /* r1 signed range is [S32_MIN, S32_MAX] */
> -       BPF_JMP32_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
> -
> -       BPF_MOV64_IMM(BPF_REG_0, 0),
> -       BPF_EXIT_INSN(),
> -       },
> -       .errstr = "BPF program is too large",
> -       .result = REJECT,
> -       .prog_type = BPF_PROG_TYPE_XDP,
> -},
> --
> 2.30.2
>
Xu Kuohai March 27, 2023, 6:20 a.m. UTC | #2 
On 3/27/2023 11:20 AM, Andrii Nakryiko wrote:
> On Sun, Mar 26, 2023 at 7:45 PM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
>>
>> From: Xu Kuohai <xukuohai@huawei.com>
>>
>> The two infinite loop bound check cases added by commit
>> 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
>> take a long time to execute but don't add much value.
>>
>> Remove them to reduce run time of test_verifier.
> 
> Summary: 2042 PASSED, 0 SKIPPED, 1 FAILED
> 
> real    0m4.780s
> user    0m0.458s
> sys     0m3.871s
> 
> 
> 5 seconds isn't such a long time, especially when we compare it to
> test_progs (even with parallelization).
> 

Well, I actually don't know if it is "long time".

This patch was sent to address Alexei's concern about the run time
of test_verifier in mail [1].

[1] https://lore.kernel.org/bpf/20230322213056.2470-1-daniel@iogearbox.net/T/#mb3d6363a693ccd63d416d9d787db17f8fdcb8442

>>
>> Fixes: 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
>> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
>> ---
>>   tools/testing/selftests/bpf/verifier/bounds.c | 50 -------------------
>>   1 file changed, 50 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/verifier/bounds.c b/tools/testing/selftests/bpf/verifier/bounds.c
>> index 74b1917d4208..515a8222f08f 100644
>> --- a/tools/testing/selftests/bpf/verifier/bounds.c
>> +++ b/tools/testing/selftests/bpf/verifier/bounds.c
>> @@ -777,31 +777,6 @@
>>          .result = ACCEPT,
>>          .prog_type = BPF_PROG_TYPE_XDP,
>>   },
>> -{
>> -       "bound check with JMP_JSLT for crossing 64-bit signed boundary",
>> -       .insns = {
>> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
>> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
>> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
>> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
>> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 8),
>> -
>> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
>> -       BPF_LD_IMM64(BPF_REG_0, 0x7fffffffffffff10),
>> -       BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
>> -
>> -       BPF_LD_IMM64(BPF_REG_0, 0x8000000000000000),
>> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 1),
>> -       /* r1 signed range is [S64_MIN, S64_MAX] */
>> -       BPF_JMP_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
>> -
>> -       BPF_MOV64_IMM(BPF_REG_0, 0),
>> -       BPF_EXIT_INSN(),
>> -       },
>> -       .errstr = "BPF program is too large",
>> -       .result = REJECT,
>> -       .prog_type = BPF_PROG_TYPE_XDP,
>> -},
>>   {
>>          "bound check for loop upper bound greater than U32_MAX",
>>          .insns = {
>> @@ -849,28 +824,3 @@
>>          .result = ACCEPT,
>>          .prog_type = BPF_PROG_TYPE_XDP,
>>   },
>> -{
>> -       "bound check with JMP32_JSLT for crossing 32-bit signed boundary",
>> -       .insns = {
>> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
>> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
>> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
>> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
>> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 6),
>> -
>> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
>> -       BPF_MOV32_IMM(BPF_REG_0, 0x7fffff10),
>> -       BPF_ALU32_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
>> -
>> -       BPF_MOV32_IMM(BPF_REG_0, 0x80000000),
>> -       BPF_ALU32_IMM(BPF_ADD, BPF_REG_0, 1),
>> -       /* r1 signed range is [S32_MIN, S32_MAX] */
>> -       BPF_JMP32_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
>> -
>> -       BPF_MOV64_IMM(BPF_REG_0, 0),
>> -       BPF_EXIT_INSN(),
>> -       },
>> -       .errstr = "BPF program is too large",
>> -       .result = REJECT,
>> -       .prog_type = BPF_PROG_TYPE_XDP,
>> -},
>> --
>> 2.30.2
>>
> .
Andrii Nakryiko March 27, 2023, 6:55 p.m. UTC | #3 
On Sun, Mar 26, 2023 at 11:21 PM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
>
> On 3/27/2023 11:20 AM, Andrii Nakryiko wrote:
> > On Sun, Mar 26, 2023 at 7:45 PM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
> >>
> >> From: Xu Kuohai <xukuohai@huawei.com>
> >>
> >> The two infinite loop bound check cases added by commit
> >> 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
> >> take a long time to execute but don't add much value.
> >>
> >> Remove them to reduce run time of test_verifier.
> >
> > Summary: 2042 PASSED, 0 SKIPPED, 1 FAILED
> >
> > real    0m4.780s
> > user    0m0.458s
> > sys     0m3.871s
> >
> >
> > 5 seconds isn't such a long time, especially when we compare it to
> > test_progs (even with parallelization).
> >
>
> Well, I actually don't know if it is "long time".
>
> This patch was sent to address Alexei's concern about the run time
> of test_verifier in mail [1].

  > These infinite loops don't add much value to the actual test.
  > Please rewrite them without infinite loops.

Alexei asked to improve the test, not to just remove it, if I
understand correctly. If the test is there, presumably it's useful. If
it can be implemented better, let's do that. Just removing the test
seems like a wrong move.

>
> [1] https://lore.kernel.org/bpf/20230322213056.2470-1-daniel@iogearbox.net/T/#mb3d6363a693ccd63d416d9d787db17f8fdcb8442
>
> >>
> >> Fixes: 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
> >> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
> >> ---
> >>   tools/testing/selftests/bpf/verifier/bounds.c | 50 -------------------
> >>   1 file changed, 50 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/verifier/bounds.c b/tools/testing/selftests/bpf/verifier/bounds.c
> >> index 74b1917d4208..515a8222f08f 100644
> >> --- a/tools/testing/selftests/bpf/verifier/bounds.c
> >> +++ b/tools/testing/selftests/bpf/verifier/bounds.c
> >> @@ -777,31 +777,6 @@
> >>          .result = ACCEPT,
> >>          .prog_type = BPF_PROG_TYPE_XDP,
> >>   },
> >> -{
> >> -       "bound check with JMP_JSLT for crossing 64-bit signed boundary",
> >> -       .insns = {
> >> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
> >> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
> >> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
> >> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
> >> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 8),
> >> -
> >> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
> >> -       BPF_LD_IMM64(BPF_REG_0, 0x7fffffffffffff10),
> >> -       BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
> >> -
> >> -       BPF_LD_IMM64(BPF_REG_0, 0x8000000000000000),
> >> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 1),
> >> -       /* r1 signed range is [S64_MIN, S64_MAX] */
> >> -       BPF_JMP_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
> >> -
> >> -       BPF_MOV64_IMM(BPF_REG_0, 0),
> >> -       BPF_EXIT_INSN(),
> >> -       },
> >> -       .errstr = "BPF program is too large",
> >> -       .result = REJECT,
> >> -       .prog_type = BPF_PROG_TYPE_XDP,
> >> -},
> >>   {
> >>          "bound check for loop upper bound greater than U32_MAX",
> >>          .insns = {
> >> @@ -849,28 +824,3 @@
> >>          .result = ACCEPT,
> >>          .prog_type = BPF_PROG_TYPE_XDP,
> >>   },
> >> -{
> >> -       "bound check with JMP32_JSLT for crossing 32-bit signed boundary",
> >> -       .insns = {
> >> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
> >> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
> >> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
> >> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
> >> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 6),
> >> -
> >> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
> >> -       BPF_MOV32_IMM(BPF_REG_0, 0x7fffff10),
> >> -       BPF_ALU32_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
> >> -
> >> -       BPF_MOV32_IMM(BPF_REG_0, 0x80000000),
> >> -       BPF_ALU32_IMM(BPF_ADD, BPF_REG_0, 1),
> >> -       /* r1 signed range is [S32_MIN, S32_MAX] */
> >> -       BPF_JMP32_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
> >> -
> >> -       BPF_MOV64_IMM(BPF_REG_0, 0),
> >> -       BPF_EXIT_INSN(),
> >> -       },
> >> -       .errstr = "BPF program is too large",
> >> -       .result = REJECT,
> >> -       .prog_type = BPF_PROG_TYPE_XDP,
> >> -},
> >> --
> >> 2.30.2
> >>
> > .
>
Xu Kuohai March 28, 2023, 12:19 p.m. UTC | #4 
On 3/28/2023 2:55 AM, Andrii Nakryiko wrote:
> On Sun, Mar 26, 2023 at 11:21 PM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
>>
>> On 3/27/2023 11:20 AM, Andrii Nakryiko wrote:
>>> On Sun, Mar 26, 2023 at 7:45 PM Xu Kuohai <xukuohai@huaweicloud.com> wrote:
>>>>
>>>> From: Xu Kuohai <xukuohai@huawei.com>
>>>>
>>>> The two infinite loop bound check cases added by commit
>>>> 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
>>>> take a long time to execute but don't add much value.
>>>>
>>>> Remove them to reduce run time of test_verifier.
>>>
>>> Summary: 2042 PASSED, 0 SKIPPED, 1 FAILED
>>>
>>> real    0m4.780s
>>> user    0m0.458s
>>> sys     0m3.871s
>>>
>>>
>>> 5 seconds isn't such a long time, especially when we compare it to
>>> test_progs (even with parallelization).
>>>
>>
>> Well, I actually don't know if it is "long time".
>>
>> This patch was sent to address Alexei's concern about the run time
>> of test_verifier in mail [1].
> 
>    > These infinite loops don't add much value to the actual test.
>    > Please rewrite them without infinite loops.
> 
> Alexei asked to improve the test, not to just remove it, if I
> understand correctly. If the test is there, presumably it's useful. If
> it can be implemented better, let's do that. Just removing the test
> seems like a wrong move.
>

OK. I added upper bounds to the two loops to make them finite.

>>
>> [1] https://lore.kernel.org/bpf/20230322213056.2470-1-daniel@iogearbox.net/T/#mb3d6363a693ccd63d416d9d787db17f8fdcb8442
>>
>>>>
>>>> Fixes: 1a3148fc171f ("selftests/bpf: Check when bounds are not in the 32-bit range")
>>>> Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
>>>> ---
>>>>    tools/testing/selftests/bpf/verifier/bounds.c | 50 -------------------
>>>>    1 file changed, 50 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/verifier/bounds.c b/tools/testing/selftests/bpf/verifier/bounds.c
>>>> index 74b1917d4208..515a8222f08f 100644
>>>> --- a/tools/testing/selftests/bpf/verifier/bounds.c
>>>> +++ b/tools/testing/selftests/bpf/verifier/bounds.c
>>>> @@ -777,31 +777,6 @@
>>>>           .result = ACCEPT,
>>>>           .prog_type = BPF_PROG_TYPE_XDP,
>>>>    },
>>>> -{
>>>> -       "bound check with JMP_JSLT for crossing 64-bit signed boundary",
>>>> -       .insns = {
>>>> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
>>>> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
>>>> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
>>>> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
>>>> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 8),
>>>> -
>>>> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
>>>> -       BPF_LD_IMM64(BPF_REG_0, 0x7fffffffffffff10),
>>>> -       BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
>>>> -
>>>> -       BPF_LD_IMM64(BPF_REG_0, 0x8000000000000000),
>>>> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 1),
>>>> -       /* r1 signed range is [S64_MIN, S64_MAX] */
>>>> -       BPF_JMP_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
>>>> -
>>>> -       BPF_MOV64_IMM(BPF_REG_0, 0),
>>>> -       BPF_EXIT_INSN(),
>>>> -       },
>>>> -       .errstr = "BPF program is too large",
>>>> -       .result = REJECT,
>>>> -       .prog_type = BPF_PROG_TYPE_XDP,
>>>> -},
>>>>    {
>>>>           "bound check for loop upper bound greater than U32_MAX",
>>>>           .insns = {
>>>> @@ -849,28 +824,3 @@
>>>>           .result = ACCEPT,
>>>>           .prog_type = BPF_PROG_TYPE_XDP,
>>>>    },
>>>> -{
>>>> -       "bound check with JMP32_JSLT for crossing 32-bit signed boundary",
>>>> -       .insns = {
>>>> -       BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
>>>> -       BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
>>>> -       BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
>>>> -       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
>>>> -       BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 6),
>>>> -
>>>> -       BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
>>>> -       BPF_MOV32_IMM(BPF_REG_0, 0x7fffff10),
>>>> -       BPF_ALU32_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
>>>> -
>>>> -       BPF_MOV32_IMM(BPF_REG_0, 0x80000000),
>>>> -       BPF_ALU32_IMM(BPF_ADD, BPF_REG_0, 1),
>>>> -       /* r1 signed range is [S32_MIN, S32_MAX] */
>>>> -       BPF_JMP32_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
>>>> -
>>>> -       BPF_MOV64_IMM(BPF_REG_0, 0),
>>>> -       BPF_EXIT_INSN(),
>>>> -       },
>>>> -       .errstr = "BPF program is too large",
>>>> -       .result = REJECT,
>>>> -       .prog_type = BPF_PROG_TYPE_XDP,
>>>> -},
>>>> --
>>>> 2.30.2
>>>>
>>> .
>>
> .
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/verifier/bounds.c b/tools/testing/selftests/bpf/verifier/bounds.c
index 74b1917d4208..515a8222f08f 100644
--- a/tools/testing/selftests/bpf/verifier/bounds.c
+++ b/tools/testing/selftests/bpf/verifier/bounds.c
@@ -777,31 +777,6 @@ 
 	.result = ACCEPT,
 	.prog_type = BPF_PROG_TYPE_XDP,
 },
-{
-	"bound check with JMP_JSLT for crossing 64-bit signed boundary",
-	.insns = {
-	BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
-	BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
-	BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
-	BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 8),
-
-	BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
-	BPF_LD_IMM64(BPF_REG_0, 0x7fffffffffffff10),
-	BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
-
-	BPF_LD_IMM64(BPF_REG_0, 0x8000000000000000),
-	BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 1),
-	/* r1 signed range is [S64_MIN, S64_MAX] */
-	BPF_JMP_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
-
-	BPF_MOV64_IMM(BPF_REG_0, 0),
-	BPF_EXIT_INSN(),
-	},
-	.errstr = "BPF program is too large",
-	.result = REJECT,
-	.prog_type = BPF_PROG_TYPE_XDP,
-},
 {
 	"bound check for loop upper bound greater than U32_MAX",
 	.insns = {
@@ -849,28 +824,3 @@ 
 	.result = ACCEPT,
 	.prog_type = BPF_PROG_TYPE_XDP,
 },
-{
-	"bound check with JMP32_JSLT for crossing 32-bit signed boundary",
-	.insns = {
-	BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct xdp_md, data)),
-	BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, offsetof(struct xdp_md, data_end)),
-	BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
-	BPF_JMP_REG(BPF_JGT, BPF_REG_1, BPF_REG_3, 6),
-
-	BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_2, 0),
-	BPF_MOV32_IMM(BPF_REG_0, 0x7fffff10),
-	BPF_ALU32_REG(BPF_ADD, BPF_REG_1, BPF_REG_0),
-
-	BPF_MOV32_IMM(BPF_REG_0, 0x80000000),
-	BPF_ALU32_IMM(BPF_ADD, BPF_REG_0, 1),
-	/* r1 signed range is [S32_MIN, S32_MAX] */
-	BPF_JMP32_REG(BPF_JSLT, BPF_REG_0, BPF_REG_1, -2),
-
-	BPF_MOV64_IMM(BPF_REG_0, 0),
-	BPF_EXIT_INSN(),
-	},
-	.errstr = "BPF program is too large",
-	.result = REJECT,
-	.prog_type = BPF_PROG_TYPE_XDP,
-},