* [PATCH bpf-next] bpf: add test cases for non-pointer sanitiation logic
@ 2019-03-01 21:08 Daniel Borkmann
2019-03-01 23:22 ` Song Liu
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2019-03-01 21:08 UTC (permalink / raw)
To: ast; +Cc: afabre, marek, bpf, netdev, Daniel Borkmann
Add two additional tests for further asserting the
BPF_ALU_NON_POINTER logic with cases that were missed
previously.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Marek Majkowski <marek@cloudflare.com>
Cc: Arthur Fabre <afabre@cloudflare.com>
---
.../selftests/bpf/verifier/value_ptr_arith.c | 44 ++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
index 4b721a77bebb..c3de1a2c9dc5 100644
--- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
+++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
@@ -173,7 +173,7 @@
.retval = 0,
},
{
- "sanitation: alu with different scalars",
+ "sanitation: alu with different scalars 1",
.insns = {
BPF_MOV64_IMM(BPF_REG_0, 1),
BPF_LD_MAP_FD(BPF_REG_ARG1, 0),
@@ -198,6 +198,48 @@
.result = ACCEPT,
.retval = 0x100000,
},
+{
+ "sanitation: alu with different scalars 2",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
+ BPF_ST_MEM(BPF_DW, BPF_REG_FP, -16, 0),
+ BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
+ BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
+ BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_8, BPF_REG_6),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_8),
+ BPF_EXIT_INSN(),
+ },
+ .fixup_map_array_48b = { 1 },
+ .result = ACCEPT,
+ .retval = -EINVAL * 2,
+},
+{
+ "sanitation: alu with different scalars 3",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, EINVAL),
+ BPF_ALU64_IMM(BPF_MUL, BPF_REG_0, -1),
+ BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
+ BPF_MOV64_IMM(BPF_REG_0, EINVAL),
+ BPF_ALU64_IMM(BPF_MUL, BPF_REG_0, -1),
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_8, BPF_REG_6),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_8),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = -EINVAL * 2,
+},
{
"map access: value_ptr += known scalar, upper oob arith, test 1",
.insns = {
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: add test cases for non-pointer sanitiation logic
2019-03-01 21:08 [PATCH bpf-next] bpf: add test cases for non-pointer sanitiation logic Daniel Borkmann
@ 2019-03-01 23:22 ` Song Liu
2019-03-01 23:27 ` Daniel Borkmann
0 siblings, 1 reply; 5+ messages in thread
From: Song Liu @ 2019-03-01 23:22 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, afabre, Marek Majkowski, bpf, Networking
On Fri, Mar 1, 2019 at 1:10 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Add two additional tests for further asserting the
> BPF_ALU_NON_POINTER logic with cases that were missed
> previously.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Marek Majkowski <marek@cloudflare.com>
> Cc: Arthur Fabre <afabre@cloudflare.com>
> ---
> .../selftests/bpf/verifier/value_ptr_arith.c | 44 ++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> index 4b721a77bebb..c3de1a2c9dc5 100644
> --- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> +++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> @@ -173,7 +173,7 @@
> .retval = 0,
> },
> {
> - "sanitation: alu with different scalars",
> + "sanitation: alu with different scalars 1",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 1),
> BPF_LD_MAP_FD(BPF_REG_ARG1, 0),
> @@ -198,6 +198,48 @@
> .result = ACCEPT,
> .retval = 0x100000,
> },
> +{
> + "sanitation: alu with different scalars 2",
> + .insns = {
> + BPF_MOV64_IMM(BPF_REG_0, 1),
> + BPF_LD_MAP_FD(BPF_REG_1, 0),
> + BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
> + BPF_ST_MEM(BPF_DW, BPF_REG_FP, -16, 0),
> + BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
> + BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
> + BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
> + BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
> + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> + BPF_MOV64_REG(BPF_REG_8, BPF_REG_6),
> + BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7),
> + BPF_MOV64_REG(BPF_REG_0, BPF_REG_8),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_map_array_48b = { 1 },
> + .result = ACCEPT,
> + .retval = -EINVAL * 2,
Why "-EINVAL * 2" here?
> +},
> +{
> + "sanitation: alu with different scalars 3",
> + .insns = {
> + BPF_MOV64_IMM(BPF_REG_0, EINVAL),
> + BPF_ALU64_IMM(BPF_MUL, BPF_REG_0, -1),
> + BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
> + BPF_MOV64_IMM(BPF_REG_0, EINVAL),
> + BPF_ALU64_IMM(BPF_MUL, BPF_REG_0, -1),
> + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> + BPF_MOV64_REG(BPF_REG_8, BPF_REG_6),
> + BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7),
> + BPF_MOV64_REG(BPF_REG_0, BPF_REG_8),
> + BPF_EXIT_INSN(),
> + },
> + .result = ACCEPT,
> + .retval = -EINVAL * 2,
> +},
> {
> "map access: value_ptr += known scalar, upper oob arith, test 1",
> .insns = {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: add test cases for non-pointer sanitiation logic
2019-03-01 23:22 ` Song Liu
@ 2019-03-01 23:27 ` Daniel Borkmann
2019-03-02 0:04 ` Song Liu
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2019-03-01 23:27 UTC (permalink / raw)
To: Song Liu; +Cc: Alexei Starovoitov, afabre, Marek Majkowski, bpf, Networking
On 03/02/2019 12:22 AM, Song Liu wrote:
> On Fri, Mar 1, 2019 at 1:10 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> Add two additional tests for further asserting the
>> BPF_ALU_NON_POINTER logic with cases that were missed
>> previously.
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Marek Majkowski <marek@cloudflare.com>
>> Cc: Arthur Fabre <afabre@cloudflare.com>
>> ---
>> .../selftests/bpf/verifier/value_ptr_arith.c | 44 ++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
>> index 4b721a77bebb..c3de1a2c9dc5 100644
>> --- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
>> +++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
>> @@ -173,7 +173,7 @@
>> .retval = 0,
>> },
>> {
>> - "sanitation: alu with different scalars",
>> + "sanitation: alu with different scalars 1",
>> .insns = {
>> BPF_MOV64_IMM(BPF_REG_0, 1),
>> BPF_LD_MAP_FD(BPF_REG_ARG1, 0),
>> @@ -198,6 +198,48 @@
>> .result = ACCEPT,
>> .retval = 0x100000,
>> },
>> +{
>> + "sanitation: alu with different scalars 2",
>> + .insns = {
>> + BPF_MOV64_IMM(BPF_REG_0, 1),
>> + BPF_LD_MAP_FD(BPF_REG_1, 0),
>> + BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
>> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
>> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
>> + BPF_ST_MEM(BPF_DW, BPF_REG_FP, -16, 0),
>> + BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
>> + BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
>> + BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
>> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
>> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
>> + BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
>> + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
>> + BPF_MOV64_REG(BPF_REG_8, BPF_REG_6),
>> + BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7),
>> + BPF_MOV64_REG(BPF_REG_0, BPF_REG_8),
>> + BPF_EXIT_INSN(),
>> + },
>> + .fixup_map_array_48b = { 1 },
>> + .result = ACCEPT,
>> + .retval = -EINVAL * 2,
>
> Why "-EINVAL * 2" here?
This is array map so deletion will return -EINVAL, basically I
wanted to get a test case under socket filters where helper
returns a stable value which is then unknown scalar to do some
alu64 op with where sanitation logic triggers. So map_delete_elem()
came in handy for that test.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: add test cases for non-pointer sanitiation logic
2019-03-01 23:27 ` Daniel Borkmann
@ 2019-03-02 0:04 ` Song Liu
2019-03-02 0:09 ` Daniel Borkmann
0 siblings, 1 reply; 5+ messages in thread
From: Song Liu @ 2019-03-02 0:04 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, afabre, Marek Majkowski, bpf, Networking
On Fri, Mar 1, 2019 at 3:27 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 03/02/2019 12:22 AM, Song Liu wrote:
> > On Fri, Mar 1, 2019 at 1:10 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>
> >> Add two additional tests for further asserting the
> >> BPF_ALU_NON_POINTER logic with cases that were missed
> >> previously.
> >>
> >> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> >> Cc: Marek Majkowski <marek@cloudflare.com>
> >> Cc: Arthur Fabre <afabre@cloudflare.com>
> >> ---
> >> .../selftests/bpf/verifier/value_ptr_arith.c | 44 ++++++++++++++++++-
> >> 1 file changed, 43 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> >> index 4b721a77bebb..c3de1a2c9dc5 100644
> >> --- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> >> +++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> >> @@ -173,7 +173,7 @@
> >> .retval = 0,
> >> },
> >> {
> >> - "sanitation: alu with different scalars",
> >> + "sanitation: alu with different scalars 1",
> >> .insns = {
> >> BPF_MOV64_IMM(BPF_REG_0, 1),
> >> BPF_LD_MAP_FD(BPF_REG_ARG1, 0),
> >> @@ -198,6 +198,48 @@
> >> .result = ACCEPT,
> >> .retval = 0x100000,
> >> },
> >> +{
> >> + "sanitation: alu with different scalars 2",
> >> + .insns = {
> >> + BPF_MOV64_IMM(BPF_REG_0, 1),
> >> + BPF_LD_MAP_FD(BPF_REG_1, 0),
> >> + BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
> >> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
> >> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
> >> + BPF_ST_MEM(BPF_DW, BPF_REG_FP, -16, 0),
> >> + BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
> >> + BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
> >> + BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
> >> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_FP),
> >> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
> >> + BPF_EMIT_CALL(BPF_FUNC_map_delete_elem),
> >> + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
> >> + BPF_MOV64_REG(BPF_REG_8, BPF_REG_6),
> >> + BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7),
> >> + BPF_MOV64_REG(BPF_REG_0, BPF_REG_8),
> >> + BPF_EXIT_INSN(),
> >> + },
> >> + .fixup_map_array_48b = { 1 },
> >> + .result = ACCEPT,
> >> + .retval = -EINVAL * 2,
> >
> > Why "-EINVAL * 2" here?
>
> This is array map so deletion will return -EINVAL, basically I
> wanted to get a test case under socket filters where helper
> returns a stable value which is then unknown scalar to do some
> alu64 op with where sanitation logic triggers. So map_delete_elem()
> came in handy for that test.
I see, so the *2 comes from
BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7)
Thanks for the explanation.
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpf: add test cases for non-pointer sanitiation logic
2019-03-02 0:04 ` Song Liu
@ 2019-03-02 0:09 ` Daniel Borkmann
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2019-03-02 0:09 UTC (permalink / raw)
To: Song Liu; +Cc: Alexei Starovoitov, afabre, Marek Majkowski, bpf, Networking
On 03/02/2019 01:04 AM, Song Liu wrote:
> On Fri, Mar 1, 2019 at 3:27 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 03/02/2019 12:22 AM, Song Liu wrote:
>>> On Fri, Mar 1, 2019 at 1:10 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
[...]
>
> I see, so the *2 comes from
>
> BPF_ALU64_REG(BPF_ADD, BPF_REG_8, BPF_REG_7)
Yes, exactly the place we need to test for here.
> Thanks for the explanation.
np
Thanks,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-02 0:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-01 21:08 [PATCH bpf-next] bpf: add test cases for non-pointer sanitiation logic Daniel Borkmann
2019-03-01 23:22 ` Song Liu
2019-03-01 23:27 ` Daniel Borkmann
2019-03-02 0:04 ` Song Liu
2019-03-02 0:09 ` Daniel Borkmann
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.