* [PATCH v2] selftests/bpf: Fix test_verifier after introducing resolve_pseudo_ldimm64
@ 2020-10-07 1:23 Hao Luo
2020-10-07 2:04 ` Alexei Starovoitov
0 siblings, 1 reply; 3+ messages in thread
From: Hao Luo @ 2020-10-07 1:23 UTC (permalink / raw)
To: netdev, bpf, linux-kernel
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Hao Luo
Commit 4976b718c355 ("bpf: Introduce pseudo_btf_id") switched
the order of check_subprogs() and resolve_pseudo_ldimm() in
the verifier. Now an empty prog expects to see the error "last
insn is not an the prog of a single invalid ldimm exit or jmp"
instead, because the check for subprogs comes first. It's now
pointless to validate that half of ldimm64 won't be the last
instruction.
Tested:
# ./test_verifier
Summary: 1129 PASSED, 537 SKIPPED, 0 FAILED
and the full set of bpf selftests.
Fixes: 4976b718c355 ("bpf: Introduce pseudo_btf_id")
Signed-off-by: Hao Luo <haoluo@google.com>
---
Changelog in v2:
- Remove the original test_verifier ld_imm64 test4
- Updated commit message.
tools/testing/selftests/bpf/verifier/basic.c | 2 +-
.../testing/selftests/bpf/verifier/ld_imm64.c | 24 +++++++------------
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/bpf/verifier/basic.c b/tools/testing/selftests/bpf/verifier/basic.c
index b8d18642653a..de84f0d57082 100644
--- a/tools/testing/selftests/bpf/verifier/basic.c
+++ b/tools/testing/selftests/bpf/verifier/basic.c
@@ -2,7 +2,7 @@
"empty prog",
.insns = {
},
- .errstr = "unknown opcode 00",
+ .errstr = "last insn is not an exit or jmp",
.result = REJECT,
},
{
diff --git a/tools/testing/selftests/bpf/verifier/ld_imm64.c b/tools/testing/selftests/bpf/verifier/ld_imm64.c
index 3856dba733e9..ed6a34991216 100644
--- a/tools/testing/selftests/bpf/verifier/ld_imm64.c
+++ b/tools/testing/selftests/bpf/verifier/ld_imm64.c
@@ -54,21 +54,13 @@
"test5 ld_imm64",
.insns = {
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
- },
- .errstr = "invalid bpf_ld_imm64 insn",
- .result = REJECT,
-},
-{
- "test6 ld_imm64",
- .insns = {
- BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
BPF_RAW_INSN(0, 0, 0, 0, 0),
BPF_EXIT_INSN(),
},
.result = ACCEPT,
},
{
- "test7 ld_imm64",
+ "test6 ld_imm64",
.insns = {
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
BPF_RAW_INSN(0, 0, 0, 0, 1),
@@ -78,7 +70,7 @@
.retval = 1,
},
{
- "test8 ld_imm64",
+ "test7 ld_imm64",
.insns = {
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 1, 1),
BPF_RAW_INSN(0, 0, 0, 0, 1),
@@ -88,7 +80,7 @@
.result = REJECT,
},
{
- "test9 ld_imm64",
+ "test8 ld_imm64",
.insns = {
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
BPF_RAW_INSN(0, 0, 0, 1, 1),
@@ -98,7 +90,7 @@
.result = REJECT,
},
{
- "test10 ld_imm64",
+ "test9 ld_imm64",
.insns = {
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
BPF_RAW_INSN(0, BPF_REG_1, 0, 0, 1),
@@ -108,7 +100,7 @@
.result = REJECT,
},
{
- "test11 ld_imm64",
+ "test10 ld_imm64",
.insns = {
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
BPF_RAW_INSN(0, 0, BPF_REG_1, 0, 1),
@@ -118,7 +110,7 @@
.result = REJECT,
},
{
- "test12 ld_imm64",
+ "test11 ld_imm64",
.insns = {
BPF_MOV64_IMM(BPF_REG_1, 0),
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, BPF_REG_1, 0, 1),
@@ -129,7 +121,7 @@
.result = REJECT,
},
{
- "test13 ld_imm64",
+ "test12 ld_imm64",
.insns = {
BPF_MOV64_IMM(BPF_REG_1, 0),
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, BPF_REG_1, 0, 1),
@@ -140,7 +132,7 @@
.result = REJECT,
},
{
- "test14 ld_imm64: reject 2nd imm != 0",
+ "test13 ld_imm64: reject 2nd imm != 0",
.insns = {
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, BPF_REG_1,
--
2.28.0.806.g8561365e88-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] selftests/bpf: Fix test_verifier after introducing resolve_pseudo_ldimm64
2020-10-07 1:23 [PATCH v2] selftests/bpf: Fix test_verifier after introducing resolve_pseudo_ldimm64 Hao Luo
@ 2020-10-07 2:04 ` Alexei Starovoitov
2020-10-07 2:29 ` Hao Luo
0 siblings, 1 reply; 3+ messages in thread
From: Alexei Starovoitov @ 2020-10-07 2:04 UTC (permalink / raw)
To: Hao Luo
Cc: netdev, bpf, linux-kernel, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh
On Tue, Oct 06, 2020 at 06:23:13PM -0700, Hao Luo wrote:
> Commit 4976b718c355 ("bpf: Introduce pseudo_btf_id") switched
> the order of check_subprogs() and resolve_pseudo_ldimm() in
> the verifier. Now an empty prog expects to see the error "last
> insn is not an the prog of a single invalid ldimm exit or jmp"
> instead, because the check for subprogs comes first. It's now
> pointless to validate that half of ldimm64 won't be the last
> instruction.
>
> Tested:
> # ./test_verifier
> Summary: 1129 PASSED, 537 SKIPPED, 0 FAILED
> and the full set of bpf selftests.
>
> Fixes: 4976b718c355 ("bpf: Introduce pseudo_btf_id")
> Signed-off-by: Hao Luo <haoluo@google.com>
> ---
> Changelog in v2:
> - Remove the original test_verifier ld_imm64 test4
> - Updated commit message.
>
> tools/testing/selftests/bpf/verifier/basic.c | 2 +-
> .../testing/selftests/bpf/verifier/ld_imm64.c | 24 +++++++------------
> 2 files changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/verifier/basic.c b/tools/testing/selftests/bpf/verifier/basic.c
> index b8d18642653a..de84f0d57082 100644
> --- a/tools/testing/selftests/bpf/verifier/basic.c
> +++ b/tools/testing/selftests/bpf/verifier/basic.c
> @@ -2,7 +2,7 @@
> "empty prog",
> .insns = {
> },
> - .errstr = "unknown opcode 00",
> + .errstr = "last insn is not an exit or jmp",
> .result = REJECT,
> },
> {
> diff --git a/tools/testing/selftests/bpf/verifier/ld_imm64.c b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> index 3856dba733e9..ed6a34991216 100644
> --- a/tools/testing/selftests/bpf/verifier/ld_imm64.c
> +++ b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> @@ -54,21 +54,13 @@
> "test5 ld_imm64",
> .insns = {
> BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
> - },
> - .errstr = "invalid bpf_ld_imm64 insn",
> - .result = REJECT,
> -},
> -{
> - "test6 ld_imm64",
> - .insns = {
> - BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
> BPF_RAW_INSN(0, 0, 0, 0, 0),
> BPF_EXIT_INSN(),
> },
> .result = ACCEPT,
> },
> {
> - "test7 ld_imm64",
> + "test6 ld_imm64",
> .insns = {
> BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
> BPF_RAW_INSN(0, 0, 0, 0, 1),
> @@ -78,7 +70,7 @@
> .retval = 1,
> },
> {
> - "test8 ld_imm64",
> + "test7 ld_imm64",
imo that's too much churn to rename all of them.
Just delete one.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] selftests/bpf: Fix test_verifier after introducing resolve_pseudo_ldimm64
2020-10-07 2:04 ` Alexei Starovoitov
@ 2020-10-07 2:29 ` Hao Luo
0 siblings, 0 replies; 3+ messages in thread
From: Hao Luo @ 2020-10-07 2:29 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Networking, bpf, open list, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh
Ack. Sent one with just deletion.
Hao
On Tue, Oct 6, 2020 at 7:04 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Oct 06, 2020 at 06:23:13PM -0700, Hao Luo wrote:
> > Commit 4976b718c355 ("bpf: Introduce pseudo_btf_id") switched
> > the order of check_subprogs() and resolve_pseudo_ldimm() in
> > the verifier. Now an empty prog expects to see the error "last
> > insn is not an the prog of a single invalid ldimm exit or jmp"
> > instead, because the check for subprogs comes first. It's now
> > pointless to validate that half of ldimm64 won't be the last
> > instruction.
> >
> > Tested:
> > # ./test_verifier
> > Summary: 1129 PASSED, 537 SKIPPED, 0 FAILED
> > and the full set of bpf selftests.
> >
> > Fixes: 4976b718c355 ("bpf: Introduce pseudo_btf_id")
> > Signed-off-by: Hao Luo <haoluo@google.com>
> > ---
> > Changelog in v2:
> > - Remove the original test_verifier ld_imm64 test4
> > - Updated commit message.
> >
> > tools/testing/selftests/bpf/verifier/basic.c | 2 +-
> > .../testing/selftests/bpf/verifier/ld_imm64.c | 24 +++++++------------
> > 2 files changed, 9 insertions(+), 17 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/verifier/basic.c b/tools/testing/selftests/bpf/verifier/basic.c
> > index b8d18642653a..de84f0d57082 100644
> > --- a/tools/testing/selftests/bpf/verifier/basic.c
> > +++ b/tools/testing/selftests/bpf/verifier/basic.c
> > @@ -2,7 +2,7 @@
> > "empty prog",
> > .insns = {
> > },
> > - .errstr = "unknown opcode 00",
> > + .errstr = "last insn is not an exit or jmp",
> > .result = REJECT,
> > },
> > {
> > diff --git a/tools/testing/selftests/bpf/verifier/ld_imm64.c b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> > index 3856dba733e9..ed6a34991216 100644
> > --- a/tools/testing/selftests/bpf/verifier/ld_imm64.c
> > +++ b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> > @@ -54,21 +54,13 @@
> > "test5 ld_imm64",
> > .insns = {
> > BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
> > - },
> > - .errstr = "invalid bpf_ld_imm64 insn",
> > - .result = REJECT,
> > -},
> > -{
> > - "test6 ld_imm64",
> > - .insns = {
> > - BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
> > BPF_RAW_INSN(0, 0, 0, 0, 0),
> > BPF_EXIT_INSN(),
> > },
> > .result = ACCEPT,
> > },
> > {
> > - "test7 ld_imm64",
> > + "test6 ld_imm64",
> > .insns = {
> > BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
> > BPF_RAW_INSN(0, 0, 0, 0, 1),
> > @@ -78,7 +70,7 @@
> > .retval = 1,
> > },
> > {
> > - "test8 ld_imm64",
> > + "test7 ld_imm64",
>
> imo that's too much churn to rename all of them.
> Just delete one.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-07 2:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 1:23 [PATCH v2] selftests/bpf: Fix test_verifier after introducing resolve_pseudo_ldimm64 Hao Luo
2020-10-07 2:04 ` Alexei Starovoitov
2020-10-07 2:29 ` Hao Luo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).