* [PATCH bpf-next] selftests/bpf: ensure core_reloc_kernel is reading test_progs's data only
@ 2019-11-21 17:59 Andrii Nakryiko
2019-11-21 20:55 ` John Fastabend
2019-11-22 7:26 ` Alexei Starovoitov
0 siblings, 2 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2019-11-21 17:59 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
test_core_reloc_kernel.c selftest is the only CO-RE test that reads and
returns for validation calling thread's information (pid, tgid, comm). Thus it
has to make sure that only test_prog's invocations are honored.
Fixes: df36e621418b ("selftests/bpf: add CO-RE relocs testing setup")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
.../selftests/bpf/prog_tests/core_reloc.c | 16 +++++++++++-----
.../selftests/bpf/progs/test_core_reloc_kernel.c | 4 ++++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index ec9e2fdd6b89..05fe85281ff7 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -2,6 +2,7 @@
#include <test_progs.h>
#include "progs/core_reloc_types.h"
#include <sys/mman.h>
+#include <sys/syscall.h>
#define STRUCT_TO_CHAR_PTR(struct_name) (const char *)&(struct struct_name)
@@ -452,6 +453,7 @@ static struct core_reloc_test_case test_cases[] = {
struct data {
char in[256];
char out[256];
+ uint64_t my_pid_tgid;
};
static size_t roundup_page(size_t sz)
@@ -471,9 +473,12 @@ void test_core_reloc(void)
struct bpf_map *data_map;
struct bpf_program *prog;
struct bpf_object *obj;
+ uint64_t my_pid_tgid;
struct data *data;
void *mmap_data = NULL;
+ my_pid_tgid = getpid() | ((uint64_t)syscall(SYS_gettid) << 32);
+
for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
test_case = &test_cases[i];
if (!test__start_subtest(test_case->case_name))
@@ -517,11 +522,6 @@ void test_core_reloc(void)
goto cleanup;
}
- link = bpf_program__attach_raw_tracepoint(prog, tp_name);
- if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n",
- PTR_ERR(link)))
- goto cleanup;
-
data_map = bpf_object__find_map_by_name(obj, "test_cor.bss");
if (CHECK(!data_map, "find_data_map", "data map not found\n"))
goto cleanup;
@@ -537,6 +537,12 @@ void test_core_reloc(void)
memset(mmap_data, 0, sizeof(*data));
memcpy(data->in, test_case->input, test_case->input_len);
+ data->my_pid_tgid = my_pid_tgid;
+
+ link = bpf_program__attach_raw_tracepoint(prog, tp_name);
+ if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n",
+ PTR_ERR(link)))
+ goto cleanup;
/* trigger test run */
usleep(1);
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
index a4b5e0562ed5..d2fe8f337846 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
@@ -11,6 +11,7 @@ char _license[] SEC("license") = "GPL";
static volatile struct data {
char in[256];
char out[256];
+ uint64_t my_pid_tgid;
} data;
struct core_reloc_kernel_output {
@@ -38,6 +39,9 @@ int test_core_kernel(void *ctx)
uint32_t real_tgid = (uint32_t)pid_tgid;
int pid, tgid;
+ if (data.my_pid_tgid != pid_tgid)
+ return 0;
+
if (CORE_READ(&pid, &task->pid) ||
CORE_READ(&tgid, &task->tgid))
return 1;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH bpf-next] selftests/bpf: ensure core_reloc_kernel is reading test_progs's data only
2019-11-21 17:59 [PATCH bpf-next] selftests/bpf: ensure core_reloc_kernel is reading test_progs's data only Andrii Nakryiko
@ 2019-11-21 20:55 ` John Fastabend
2019-11-22 7:26 ` Alexei Starovoitov
1 sibling, 0 replies; 4+ messages in thread
From: John Fastabend @ 2019-11-21 20:55 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, netdev, ast, daniel
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
Andrii Nakryiko wrote:
> test_core_reloc_kernel.c selftest is the only CO-RE test that reads and
> returns for validation calling thread's information (pid, tgid, comm). Thus it
> has to make sure that only test_prog's invocations are honored.
>
> Fixes: df36e621418b ("selftests/bpf: add CO-RE relocs testing setup")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> .../selftests/bpf/prog_tests/core_reloc.c | 16 +++++++++++-----
> .../selftests/bpf/progs/test_core_reloc_kernel.c | 4 ++++
> 2 files changed, 15 insertions(+), 5 deletions(-)
>
Looks good.
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: ensure core_reloc_kernel is reading test_progs's data only
2019-11-21 17:59 [PATCH bpf-next] selftests/bpf: ensure core_reloc_kernel is reading test_progs's data only Andrii Nakryiko
2019-11-21 20:55 ` John Fastabend
@ 2019-11-22 7:26 ` Alexei Starovoitov
2019-11-22 18:24 ` Andrii Nakryiko
1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2019-11-22 7:26 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Network Development, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kernel Team
On Thu, Nov 21, 2019 at 9:59 AM Andrii Nakryiko <andriin@fb.com> wrote:
>
> test_core_reloc_kernel.c selftest is the only CO-RE test that reads and
> returns for validation calling thread's information (pid, tgid, comm). Thus it
> has to make sure that only test_prog's invocations are honored.
>
> Fixes: df36e621418b ("selftests/bpf: add CO-RE relocs testing setup")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> .../selftests/bpf/prog_tests/core_reloc.c | 16 +++++++++++-----
> .../selftests/bpf/progs/test_core_reloc_kernel.c | 4 ++++
> 2 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> index ec9e2fdd6b89..05fe85281ff7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> @@ -2,6 +2,7 @@
> #include <test_progs.h>
> #include "progs/core_reloc_types.h"
> #include <sys/mman.h>
> +#include <sys/syscall.h>
>
> #define STRUCT_TO_CHAR_PTR(struct_name) (const char *)&(struct struct_name)
>
> @@ -452,6 +453,7 @@ static struct core_reloc_test_case test_cases[] = {
> struct data {
> char in[256];
> char out[256];
> + uint64_t my_pid_tgid;
> };
>
> static size_t roundup_page(size_t sz)
> @@ -471,9 +473,12 @@ void test_core_reloc(void)
> struct bpf_map *data_map;
> struct bpf_program *prog;
> struct bpf_object *obj;
> + uint64_t my_pid_tgid;
> struct data *data;
> void *mmap_data = NULL;
>
> + my_pid_tgid = getpid() | ((uint64_t)syscall(SYS_gettid) << 32);
> +
> for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
> test_case = &test_cases[i];
> if (!test__start_subtest(test_case->case_name))
> @@ -517,11 +522,6 @@ void test_core_reloc(void)
> goto cleanup;
> }
>
> - link = bpf_program__attach_raw_tracepoint(prog, tp_name);
> - if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n",
> - PTR_ERR(link)))
> - goto cleanup;
> -
> data_map = bpf_object__find_map_by_name(obj, "test_cor.bss");
> if (CHECK(!data_map, "find_data_map", "data map not found\n"))
> goto cleanup;
> @@ -537,6 +537,12 @@ void test_core_reloc(void)
>
> memset(mmap_data, 0, sizeof(*data));
> memcpy(data->in, test_case->input, test_case->input_len);
> + data->my_pid_tgid = my_pid_tgid;
> +
> + link = bpf_program__attach_raw_tracepoint(prog, tp_name);
> + if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n",
> + PTR_ERR(link)))
> + goto cleanup;
>
> /* trigger test run */
> usleep(1);
> diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> index a4b5e0562ed5..d2fe8f337846 100644
> --- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> +++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> @@ -11,6 +11,7 @@ char _license[] SEC("license") = "GPL";
> static volatile struct data {
> char in[256];
> char out[256];
> + uint64_t my_pid_tgid;
> } data;
There was a conflict here, since global data support patchset was
already applied.
I resolved it and applied to bpf-next.
Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: ensure core_reloc_kernel is reading test_progs's data only
2019-11-22 7:26 ` Alexei Starovoitov
@ 2019-11-22 18:24 ` Andrii Nakryiko
0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2019-11-22 18:24 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Thu, Nov 21, 2019 at 11:27 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Nov 21, 2019 at 9:59 AM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > test_core_reloc_kernel.c selftest is the only CO-RE test that reads and
> > returns for validation calling thread's information (pid, tgid, comm). Thus it
> > has to make sure that only test_prog's invocations are honored.
> >
> > Fixes: df36e621418b ("selftests/bpf: add CO-RE relocs testing setup")
> > Reported-by: Alexei Starovoitov <ast@kernel.org>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > .../selftests/bpf/prog_tests/core_reloc.c | 16 +++++++++++-----
> > .../selftests/bpf/progs/test_core_reloc_kernel.c | 4 ++++
> > 2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > index ec9e2fdd6b89..05fe85281ff7 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > @@ -2,6 +2,7 @@
> > #include <test_progs.h>
> > #include "progs/core_reloc_types.h"
> > #include <sys/mman.h>
> > +#include <sys/syscall.h>
> >
> > #define STRUCT_TO_CHAR_PTR(struct_name) (const char *)&(struct struct_name)
> >
> > @@ -452,6 +453,7 @@ static struct core_reloc_test_case test_cases[] = {
> > struct data {
> > char in[256];
> > char out[256];
> > + uint64_t my_pid_tgid;
> > };
> >
> > static size_t roundup_page(size_t sz)
> > @@ -471,9 +473,12 @@ void test_core_reloc(void)
> > struct bpf_map *data_map;
> > struct bpf_program *prog;
> > struct bpf_object *obj;
> > + uint64_t my_pid_tgid;
> > struct data *data;
> > void *mmap_data = NULL;
> >
> > + my_pid_tgid = getpid() | ((uint64_t)syscall(SYS_gettid) << 32);
> > +
> > for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
> > test_case = &test_cases[i];
> > if (!test__start_subtest(test_case->case_name))
> > @@ -517,11 +522,6 @@ void test_core_reloc(void)
> > goto cleanup;
> > }
> >
> > - link = bpf_program__attach_raw_tracepoint(prog, tp_name);
> > - if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n",
> > - PTR_ERR(link)))
> > - goto cleanup;
> > -
> > data_map = bpf_object__find_map_by_name(obj, "test_cor.bss");
> > if (CHECK(!data_map, "find_data_map", "data map not found\n"))
> > goto cleanup;
> > @@ -537,6 +537,12 @@ void test_core_reloc(void)
> >
> > memset(mmap_data, 0, sizeof(*data));
> > memcpy(data->in, test_case->input, test_case->input_len);
> > + data->my_pid_tgid = my_pid_tgid;
> > +
> > + link = bpf_program__attach_raw_tracepoint(prog, tp_name);
> > + if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n",
> > + PTR_ERR(link)))
> > + goto cleanup;
> >
> > /* trigger test run */
> > usleep(1);
> > diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > index a4b5e0562ed5..d2fe8f337846 100644
> > --- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > +++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
> > @@ -11,6 +11,7 @@ char _license[] SEC("license") = "GPL";
> > static volatile struct data {
> > char in[256];
> > char out[256];
> > + uint64_t my_pid_tgid;
> > } data;
>
> There was a conflict here, since global data support patchset was
> already applied.
> I resolved it and applied to bpf-next.
> Thanks
Ah, right, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-22 18:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-21 17:59 [PATCH bpf-next] selftests/bpf: ensure core_reloc_kernel is reading test_progs's data only Andrii Nakryiko
2019-11-21 20:55 ` John Fastabend
2019-11-22 7:26 ` Alexei Starovoitov
2019-11-22 18:24 ` Andrii Nakryiko
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).