* [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages
@ 2020-07-28 22:18 Yonghong Song
2020-07-28 22:18 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_iter buffer access with negative offset Yonghong Song
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Yonghong Song @ 2020-07-28 22:18 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Newline characters are added in two verifier error messages,
refactored in Commit afbf21dce668 ("bpf: Support readonly/readwrite
buffers in verifier"). This way, they do not mix with
messages afterwards.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/verifier.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 88bb25d08bf8..b6ccfce3bf4c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3069,7 +3069,7 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
{
if (off < 0) {
verbose(env,
- "R%d invalid %s buffer access: off=%d, size=%d",
+ "R%d invalid %s buffer access: off=%d, size=%d\n",
regno, buf_info, off, size);
return -EACCES;
}
@@ -3078,7 +3078,7 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
verbose(env,
- "R%d invalid variable buffer offset: off=%d, var_off=%s",
+ "R%d invalid variable buffer offset: off=%d, var_off=%s\n",
regno, off, tn_buf);
return -EACCES;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH bpf-next 2/2] selftests/bpf: test bpf_iter buffer access with negative offset
2020-07-28 22:18 [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages Yonghong Song
@ 2020-07-28 22:18 ` Yonghong Song
2020-07-28 23:42 ` Song Liu
2020-07-28 23:39 ` [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages Song Liu
2020-07-29 22:44 ` Daniel Borkmann
2 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2020-07-28 22:18 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
Commit afbf21dce668 ("bpf: Support readonly/readwrite buffers
in verifier") added readonly/readwrite buffer support which
is currently used by bpf_iter tracing programs. It has
a bug with incorrect parameter ordering which later fixed
by Commit f6dfbe31e8fa ("bpf: Fix swapped arguments in calls
to check_buffer_access").
This patch added a test case with a negative offset access
which will trigger the error path.
Without Commit f6dfbe31e8fa, running the test case in the patch,
the error message looks like:
R1_w=rdwr_buf(id=0,off=0,imm=0) R10=fp0
; value_sum += *(__u32 *)(value - 4);
2: (61) r1 = *(u32 *)(r1 -4)
R1 invalid (null) buffer access: off=-4, size=4
With the above commit, the error message looks like:
R1_w=rdwr_buf(id=0,off=0,imm=0) R10=fp0
; value_sum += *(__u32 *)(value - 4);
2: (61) r1 = *(u32 *)(r1 -4)
R1 invalid rdwr buffer access: off=-4, size=4
Signed-off-by: Yonghong Song <yhs@fb.com>
---
.../selftests/bpf/prog_tests/bpf_iter.c | 13 ++++++++++++
.../selftests/bpf/progs/bpf_iter_test_kern6.c | 21 +++++++++++++++++++
2 files changed, 34 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index d95de80b1851..4ffefdc1130f 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -21,6 +21,7 @@
#include "bpf_iter_bpf_percpu_array_map.skel.h"
#include "bpf_iter_bpf_sk_storage_map.skel.h"
#include "bpf_iter_test_kern5.skel.h"
+#include "bpf_iter_test_kern6.skel.h"
static int duration;
@@ -885,6 +886,16 @@ static void test_rdonly_buf_out_of_bound(void)
bpf_iter_test_kern5__destroy(skel);
}
+static void test_buf_neg_offset(void)
+{
+ struct bpf_iter_test_kern6 *skel;
+
+ skel = bpf_iter_test_kern6__open_and_load();
+ if (CHECK(skel, "bpf_iter_test_kern6__open_and_load",
+ "skeleton open_and_load unexpected success\n"))
+ bpf_iter_test_kern6__destroy(skel);
+}
+
void test_bpf_iter(void)
{
if (test__start_subtest("btf_id_or_null"))
@@ -933,4 +944,6 @@ void test_bpf_iter(void)
test_bpf_sk_storage_map();
if (test__start_subtest("rdonly-buf-out-of-bound"))
test_rdonly_buf_out_of_bound();
+ if (test__start_subtest("buf-neg-offset"))
+ test_buf_neg_offset();
}
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c
new file mode 100644
index 000000000000..1c7304f56b1e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+#include "bpf_iter.h"
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+__u32 value_sum = 0;
+
+SEC("iter/bpf_map_elem")
+int dump_bpf_hash_map(struct bpf_iter__bpf_map_elem *ctx)
+{
+ void *value = ctx->value;
+
+ if (value == (void *)0)
+ return 0;
+
+ /* negative offset, verifier failure. */
+ value_sum += *(__u32 *)(value - 4);
+ return 0;
+}
--
2.24.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next 2/2] selftests/bpf: test bpf_iter buffer access with negative offset
2020-07-28 22:18 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_iter buffer access with negative offset Yonghong Song
@ 2020-07-28 23:42 ` Song Liu
0 siblings, 0 replies; 5+ messages in thread
From: Song Liu @ 2020-07-28 23:42 UTC (permalink / raw)
To: Yonghong Song; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Tue, Jul 28, 2020 at 3:18 PM Yonghong Song <yhs@fb.com> wrote:
>
> Commit afbf21dce668 ("bpf: Support readonly/readwrite buffers
> in verifier") added readonly/readwrite buffer support which
> is currently used by bpf_iter tracing programs. It has
> a bug with incorrect parameter ordering which later fixed
> by Commit f6dfbe31e8fa ("bpf: Fix swapped arguments in calls
> to check_buffer_access").
>
> This patch added a test case with a negative offset access
> which will trigger the error path.
>
> Without Commit f6dfbe31e8fa, running the test case in the patch,
> the error message looks like:
> R1_w=rdwr_buf(id=0,off=0,imm=0) R10=fp0
> ; value_sum += *(__u32 *)(value - 4);
> 2: (61) r1 = *(u32 *)(r1 -4)
> R1 invalid (null) buffer access: off=-4, size=4
>
> With the above commit, the error message looks like:
> R1_w=rdwr_buf(id=0,off=0,imm=0) R10=fp0
> ; value_sum += *(__u32 *)(value - 4);
> 2: (61) r1 = *(u32 *)(r1 -4)
> R1 invalid rdwr buffer access: off=-4, size=4
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> .../selftests/bpf/prog_tests/bpf_iter.c | 13 ++++++++++++
> .../selftests/bpf/progs/bpf_iter_test_kern6.c | 21 +++++++++++++++++++
> 2 files changed, 34 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> index d95de80b1851..4ffefdc1130f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> @@ -21,6 +21,7 @@
> #include "bpf_iter_bpf_percpu_array_map.skel.h"
> #include "bpf_iter_bpf_sk_storage_map.skel.h"
> #include "bpf_iter_test_kern5.skel.h"
> +#include "bpf_iter_test_kern6.skel.h"
>
> static int duration;
>
> @@ -885,6 +886,16 @@ static void test_rdonly_buf_out_of_bound(void)
> bpf_iter_test_kern5__destroy(skel);
> }
>
> +static void test_buf_neg_offset(void)
> +{
> + struct bpf_iter_test_kern6 *skel;
> +
> + skel = bpf_iter_test_kern6__open_and_load();
> + if (CHECK(skel, "bpf_iter_test_kern6__open_and_load",
> + "skeleton open_and_load unexpected success\n"))
> + bpf_iter_test_kern6__destroy(skel);
> +}
> +
> void test_bpf_iter(void)
> {
> if (test__start_subtest("btf_id_or_null"))
> @@ -933,4 +944,6 @@ void test_bpf_iter(void)
> test_bpf_sk_storage_map();
> if (test__start_subtest("rdonly-buf-out-of-bound"))
> test_rdonly_buf_out_of_bound();
> + if (test__start_subtest("buf-neg-offset"))
> + test_buf_neg_offset();
> }
> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c
> new file mode 100644
> index 000000000000..1c7304f56b1e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern6.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020 Facebook */
> +#include "bpf_iter.h"
> +#include <bpf/bpf_helpers.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +__u32 value_sum = 0;
> +
> +SEC("iter/bpf_map_elem")
> +int dump_bpf_hash_map(struct bpf_iter__bpf_map_elem *ctx)
> +{
> + void *value = ctx->value;
> +
> + if (value == (void *)0)
> + return 0;
> +
> + /* negative offset, verifier failure. */
> + value_sum += *(__u32 *)(value - 4);
> + return 0;
> +}
> --
> 2.24.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages
2020-07-28 22:18 [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages Yonghong Song
2020-07-28 22:18 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_iter buffer access with negative offset Yonghong Song
@ 2020-07-28 23:39 ` Song Liu
2020-07-29 22:44 ` Daniel Borkmann
2 siblings, 0 replies; 5+ messages in thread
From: Song Liu @ 2020-07-28 23:39 UTC (permalink / raw)
To: Yonghong Song; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
On Tue, Jul 28, 2020 at 3:18 PM Yonghong Song <yhs@fb.com> wrote:
>
> Newline characters are added in two verifier error messages,
> refactored in Commit afbf21dce668 ("bpf: Support readonly/readwrite
> buffers in verifier"). This way, they do not mix with
> messages afterwards.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages
2020-07-28 22:18 [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages Yonghong Song
2020-07-28 22:18 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_iter buffer access with negative offset Yonghong Song
2020-07-28 23:39 ` [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages Song Liu
@ 2020-07-29 22:44 ` Daniel Borkmann
2 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2020-07-29 22:44 UTC (permalink / raw)
To: Yonghong Song, bpf; +Cc: Alexei Starovoitov, kernel-team
On 7/29/20 12:18 AM, Yonghong Song wrote:
> Newline characters are added in two verifier error messages,
> refactored in Commit afbf21dce668 ("bpf: Support readonly/readwrite
> buffers in verifier"). This way, they do not mix with
> messages afterwards.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Both applied, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-29 22:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 22:18 [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages Yonghong Song
2020-07-28 22:18 ` [PATCH bpf-next 2/2] selftests/bpf: test bpf_iter buffer access with negative offset Yonghong Song
2020-07-28 23:42 ` Song Liu
2020-07-28 23:39 ` [PATCH bpf-next 1/2] bpf: add missing newline characters in verifier error messages Song Liu
2020-07-29 22:44 ` 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.