* [PATCHv2 bpf-next 1/5] bpf: Allow trampoline re-attach for tracing and lsm programs
2021-04-06 21:29 [PATCHv2 bpf-next 0/5] bpf: Tracing and lsm programs re-attach Jiri Olsa
@ 2021-04-06 21:29 ` Jiri Olsa
2021-04-06 21:35 ` Toke Høiland-Jørgensen
2021-04-06 21:29 ` [PATCHv2 bpf-next 2/5] selftests/bpf: Add re-attach test to fentry_test Jiri Olsa
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2021-04-06 21:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
Currently we don't allow re-attaching of trampolines. Once
it's detached, it can't be re-attach even when the program
is still loaded.
Adding the possibility to re-attach the loaded tracing and
lsm programs.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/bpf/syscall.c | 23 +++++++++++++++++------
kernel/bpf/trampoline.c | 2 +-
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6428634da57e..f416735b20b3 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2645,14 +2645,25 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
* target_btf_id using the link_create API.
*
* - if tgt_prog == NULL when this function was called using the old
- * raw_tracepoint_open API, and we need a target from prog->aux
- *
- * The combination of no saved target in prog->aux, and no target
- * specified on load is illegal, and we reject that here.
+ * raw_tracepoint_open API, and we need a target from prog->aux
+ *
+ * - if prog->aux->dst_trampoline and tgt_prog is NULL, the program
+ * was detached and is going for re-attachment.
*/
if (!prog->aux->dst_trampoline && !tgt_prog) {
- err = -ENOENT;
- goto out_unlock;
+ /*
+ * Allow re-attach for TRACING and LSM programs. If it's
+ * currently linked, bpf_trampoline_link_prog will fail.
+ * EXT programs need to specify tgt_prog_fd, so they
+ * re-attach in separate code path.
+ */
+ if (prog->type != BPF_PROG_TYPE_TRACING &&
+ prog->type != BPF_PROG_TYPE_LSM) {
+ err = -EINVAL;
+ goto out_put_prog;
+ }
+ btf_id = prog->aux->attach_btf_id;
+ key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf, btf_id);
}
if (!prog->aux->dst_trampoline ||
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 4aa8b52adf25..0d937c63fc22 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -467,7 +467,7 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
tr->extension_prog = NULL;
goto out;
}
- hlist_del(&prog->aux->tramp_hlist);
+ hlist_del_init(&prog->aux->tramp_hlist);
tr->progs_cnt[kind]--;
err = bpf_trampoline_update(tr);
out:
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCHv2 bpf-next 1/5] bpf: Allow trampoline re-attach for tracing and lsm programs
2021-04-06 21:29 ` [PATCHv2 bpf-next 1/5] bpf: Allow trampoline re-attach for tracing and lsm programs Jiri Olsa
@ 2021-04-06 21:35 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 13+ messages in thread
From: Toke Høiland-Jørgensen @ 2021-04-06 21:35 UTC (permalink / raw)
To: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh
Jiri Olsa <jolsa@kernel.org> writes:
> Currently we don't allow re-attaching of trampolines. Once
> it's detached, it can't be re-attach even when the program
> is still loaded.
>
> Adding the possibility to re-attach the loaded tracing and
> lsm programs.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 bpf-next 2/5] selftests/bpf: Add re-attach test to fentry_test
2021-04-06 21:29 [PATCHv2 bpf-next 0/5] bpf: Tracing and lsm programs re-attach Jiri Olsa
2021-04-06 21:29 ` [PATCHv2 bpf-next 1/5] bpf: Allow trampoline re-attach for tracing and lsm programs Jiri Olsa
@ 2021-04-06 21:29 ` Jiri Olsa
2021-04-07 22:47 ` Andrii Nakryiko
2021-04-06 21:29 ` [PATCHv2 bpf-next 3/5] selftests/bpf: Add re-attach test to fexit_test Jiri Olsa
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2021-04-06 21:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
Adding the test to re-attach (detach/attach again) tracing
fentry programs, plus check that already linked program can't
be attached again.
Fixing the number of check-ed results, which should be 8.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/fentry_test.c | 48 +++++++++++++++----
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
index 04ebbf1cb390..1f7566e772e9 100644
--- a/tools/testing/selftests/bpf/prog_tests/fentry_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
@@ -3,20 +3,24 @@
#include <test_progs.h>
#include "fentry_test.skel.h"
-void test_fentry_test(void)
+static __u32 duration;
+
+static int fentry_test(struct fentry_test *fentry_skel)
{
- struct fentry_test *fentry_skel = NULL;
+ struct bpf_link *link;
int err, prog_fd, i;
- __u32 duration = 0, retval;
__u64 *result;
-
- fentry_skel = fentry_test__open_and_load();
- if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
- goto cleanup;
+ __u32 retval;
err = fentry_test__attach(fentry_skel);
if (CHECK(err, "fentry_attach", "fentry attach failed: %d\n", err))
- goto cleanup;
+ return err;
+
+ /* Check that already linked program can't be attached again. */
+ link = bpf_program__attach(fentry_skel->progs.test1);
+ if (CHECK(!IS_ERR(link), "fentry_attach_link",
+ "re-attach without detach should not succeed"))
+ return -1;
prog_fd = bpf_program__fd(fentry_skel->progs.test1);
err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
@@ -26,12 +30,36 @@ void test_fentry_test(void)
err, errno, retval, duration);
result = (__u64 *)fentry_skel->bss;
- for (i = 0; i < 6; i++) {
+ for (i = 0; i < 8; i++) {
if (CHECK(result[i] != 1, "result",
"fentry_test%d failed err %lld\n", i + 1, result[i]))
- goto cleanup;
+ return -1;
}
+ fentry_test__detach(fentry_skel);
+
+ /* zero results for re-attach test */
+ for (i = 0; i < 8; i++)
+ result[i] = 0;
+ return 0;
+}
+
+void test_fentry_test(void)
+{
+ struct fentry_test *fentry_skel = NULL;
+ int err;
+
+ fentry_skel = fentry_test__open_and_load();
+ if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
+ goto cleanup;
+
+ err = fentry_test(fentry_skel);
+ if (CHECK(err, "fentry_test", "first attach failed\n"))
+ goto cleanup;
+
+ err = fentry_test(fentry_skel);
+ CHECK(err, "fentry_test", "second attach failed\n");
+
cleanup:
fentry_test__destroy(fentry_skel);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCHv2 bpf-next 2/5] selftests/bpf: Add re-attach test to fentry_test
2021-04-06 21:29 ` [PATCHv2 bpf-next 2/5] selftests/bpf: Add re-attach test to fentry_test Jiri Olsa
@ 2021-04-07 22:47 ` Andrii Nakryiko
2021-04-08 11:52 ` Jiri Olsa
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-04-07 22:47 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Toke Høiland-Jørgensen
On Wed, Apr 7, 2021 at 4:21 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding the test to re-attach (detach/attach again) tracing
> fentry programs, plus check that already linked program can't
> be attached again.
>
> Fixing the number of check-ed results, which should be 8.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> .../selftests/bpf/prog_tests/fentry_test.c | 48 +++++++++++++++----
> 1 file changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
> index 04ebbf1cb390..1f7566e772e9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fentry_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
> @@ -3,20 +3,24 @@
> #include <test_progs.h>
> #include "fentry_test.skel.h"
>
> -void test_fentry_test(void)
> +static __u32 duration;
> +
> +static int fentry_test(struct fentry_test *fentry_skel)
> {
> - struct fentry_test *fentry_skel = NULL;
> + struct bpf_link *link;
> int err, prog_fd, i;
> - __u32 duration = 0, retval;
> __u64 *result;
> -
> - fentry_skel = fentry_test__open_and_load();
> - if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
> - goto cleanup;
> + __u32 retval;
>
> err = fentry_test__attach(fentry_skel);
> if (CHECK(err, "fentry_attach", "fentry attach failed: %d\n", err))
> - goto cleanup;
> + return err;
> +
> + /* Check that already linked program can't be attached again. */
> + link = bpf_program__attach(fentry_skel->progs.test1);
> + if (CHECK(!IS_ERR(link), "fentry_attach_link",
if (!ASSERT_ERR_PTR(link, "fentry_attach_link")) ?
> + "re-attach without detach should not succeed"))
> + return -1;
>
> prog_fd = bpf_program__fd(fentry_skel->progs.test1);
> err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
> @@ -26,12 +30,36 @@ void test_fentry_test(void)
> err, errno, retval, duration);
>
> result = (__u64 *)fentry_skel->bss;
> - for (i = 0; i < 6; i++) {
> + for (i = 0; i < 8; i++) {
how about using sizeof(*fentry_skel->bss) / sizeof(__u64) ?
> if (CHECK(result[i] != 1, "result",
> "fentry_test%d failed err %lld\n", i + 1, result[i]))
> - goto cleanup;
> + return -1;
> }
>
> + fentry_test__detach(fentry_skel);
> +
> + /* zero results for re-attach test */
> + for (i = 0; i < 8; i++)
> + result[i] = 0;
> + return 0;
> +}
> +
> +void test_fentry_test(void)
> +{
> + struct fentry_test *fentry_skel = NULL;
> + int err;
> +
> + fentry_skel = fentry_test__open_and_load();
> + if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
> + goto cleanup;
> +
> + err = fentry_test(fentry_skel);
> + if (CHECK(err, "fentry_test", "first attach failed\n"))
> + goto cleanup;
> +
> + err = fentry_test(fentry_skel);
> + CHECK(err, "fentry_test", "second attach failed\n");
overall: please try to use ASSERT_xxx macros, they are easier to
follow and require less typing
> +
> cleanup:
> fentry_test__destroy(fentry_skel);
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCHv2 bpf-next 2/5] selftests/bpf: Add re-attach test to fentry_test
2021-04-07 22:47 ` Andrii Nakryiko
@ 2021-04-08 11:52 ` Jiri Olsa
0 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2021-04-08 11:52 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
On Wed, Apr 07, 2021 at 03:47:30PM -0700, Andrii Nakryiko wrote:
> On Wed, Apr 7, 2021 at 4:21 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding the test to re-attach (detach/attach again) tracing
> > fentry programs, plus check that already linked program can't
> > be attached again.
> >
> > Fixing the number of check-ed results, which should be 8.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > .../selftests/bpf/prog_tests/fentry_test.c | 48 +++++++++++++++----
> > 1 file changed, 38 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_test.c b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
> > index 04ebbf1cb390..1f7566e772e9 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/fentry_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/fentry_test.c
> > @@ -3,20 +3,24 @@
> > #include <test_progs.h>
> > #include "fentry_test.skel.h"
> >
> > -void test_fentry_test(void)
> > +static __u32 duration;
> > +
> > +static int fentry_test(struct fentry_test *fentry_skel)
> > {
> > - struct fentry_test *fentry_skel = NULL;
> > + struct bpf_link *link;
> > int err, prog_fd, i;
> > - __u32 duration = 0, retval;
> > __u64 *result;
> > -
> > - fentry_skel = fentry_test__open_and_load();
> > - if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
> > - goto cleanup;
> > + __u32 retval;
> >
> > err = fentry_test__attach(fentry_skel);
> > if (CHECK(err, "fentry_attach", "fentry attach failed: %d\n", err))
> > - goto cleanup;
> > + return err;
> > +
> > + /* Check that already linked program can't be attached again. */
> > + link = bpf_program__attach(fentry_skel->progs.test1);
> > + if (CHECK(!IS_ERR(link), "fentry_attach_link",
>
> if (!ASSERT_ERR_PTR(link, "fentry_attach_link")) ?
ok
>
> > + "re-attach without detach should not succeed"))
> > + return -1;
> >
> > prog_fd = bpf_program__fd(fentry_skel->progs.test1);
> > err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
> > @@ -26,12 +30,36 @@ void test_fentry_test(void)
> > err, errno, retval, duration);
> >
> > result = (__u64 *)fentry_skel->bss;
> > - for (i = 0; i < 6; i++) {
> > + for (i = 0; i < 8; i++) {
>
> how about using sizeof(*fentry_skel->bss) / sizeof(__u64) ?
ok
>
> > if (CHECK(result[i] != 1, "result",
> > "fentry_test%d failed err %lld\n", i + 1, result[i]))
> > - goto cleanup;
> > + return -1;
> > }
> >
> > + fentry_test__detach(fentry_skel);
> > +
> > + /* zero results for re-attach test */
> > + for (i = 0; i < 8; i++)
> > + result[i] = 0;
> > + return 0;
> > +}
> > +
> > +void test_fentry_test(void)
> > +{
> > + struct fentry_test *fentry_skel = NULL;
> > + int err;
> > +
> > + fentry_skel = fentry_test__open_and_load();
> > + if (CHECK(!fentry_skel, "fentry_skel_load", "fentry skeleton failed\n"))
> > + goto cleanup;
> > +
> > + err = fentry_test(fentry_skel);
> > + if (CHECK(err, "fentry_test", "first attach failed\n"))
> > + goto cleanup;
> > +
> > + err = fentry_test(fentry_skel);
> > + CHECK(err, "fentry_test", "second attach failed\n");
>
> overall: please try to use ASSERT_xxx macros, they are easier to
> follow and require less typing
ok, will check
thanks,
jirka
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 bpf-next 3/5] selftests/bpf: Add re-attach test to fexit_test
2021-04-06 21:29 [PATCHv2 bpf-next 0/5] bpf: Tracing and lsm programs re-attach Jiri Olsa
2021-04-06 21:29 ` [PATCHv2 bpf-next 1/5] bpf: Allow trampoline re-attach for tracing and lsm programs Jiri Olsa
2021-04-06 21:29 ` [PATCHv2 bpf-next 2/5] selftests/bpf: Add re-attach test to fentry_test Jiri Olsa
@ 2021-04-06 21:29 ` Jiri Olsa
2021-04-07 22:51 ` Andrii Nakryiko
2021-04-06 21:29 ` [PATCHv2 bpf-next 4/5] selftests/bpf: Add re-attach test to lsm test Jiri Olsa
2021-04-06 21:29 ` [PATCHv2 bpf-next 5/5] selftests/bpf: Test that module can't be unloaded with attached trampoline Jiri Olsa
4 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2021-04-06 21:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
Adding the test to re-attach (detach/attach again) tracing
fexit programs, plus check that already linked program can't
be attached again.
Fixing the number of check-ed results, which should be 8.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/fexit_test.c | 48 +++++++++++++++----
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_test.c b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
index 78d7a2765c27..579e620e6612 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
@@ -3,20 +3,24 @@
#include <test_progs.h>
#include "fexit_test.skel.h"
-void test_fexit_test(void)
+static __u32 duration;
+
+static int fexit_test(struct fexit_test *fexit_skel)
{
- struct fexit_test *fexit_skel = NULL;
+ struct bpf_link *link;
int err, prog_fd, i;
- __u32 duration = 0, retval;
__u64 *result;
-
- fexit_skel = fexit_test__open_and_load();
- if (CHECK(!fexit_skel, "fexit_skel_load", "fexit skeleton failed\n"))
- goto cleanup;
+ __u32 retval;
err = fexit_test__attach(fexit_skel);
if (CHECK(err, "fexit_attach", "fexit attach failed: %d\n", err))
- goto cleanup;
+ return err;
+
+ /* Check that already linked program can't be attached again. */
+ link = bpf_program__attach(fexit_skel->progs.test1);
+ if (CHECK(!IS_ERR(link), "fexit_attach_link",
+ "re-attach without detach should not succeed"))
+ return -1;
prog_fd = bpf_program__fd(fexit_skel->progs.test1);
err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
@@ -26,12 +30,36 @@ void test_fexit_test(void)
err, errno, retval, duration);
result = (__u64 *)fexit_skel->bss;
- for (i = 0; i < 6; i++) {
+ for (i = 0; i < 8; i++) {
if (CHECK(result[i] != 1, "result",
"fexit_test%d failed err %lld\n", i + 1, result[i]))
- goto cleanup;
+ return -1;
}
+ fexit_test__detach(fexit_skel);
+
+ /* zero results for re-attach test */
+ for (i = 0; i < 8; i++)
+ result[i] = 0;
+ return 0;
+}
+
+void test_fexit_test(void)
+{
+ struct fexit_test *fexit_skel = NULL;
+ int err;
+
+ fexit_skel = fexit_test__open_and_load();
+ if (CHECK(!fexit_skel, "fexit_skel_load", "fexit skeleton failed\n"))
+ goto cleanup;
+
+ err = fexit_test(fexit_skel);
+ if (CHECK(err, "fexit_test", "first attach failed\n"))
+ goto cleanup;
+
+ err = fexit_test(fexit_skel);
+ CHECK(err, "fexit_test", "second attach failed\n");
+
cleanup:
fexit_test__destroy(fexit_skel);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCHv2 bpf-next 3/5] selftests/bpf: Add re-attach test to fexit_test
2021-04-06 21:29 ` [PATCHv2 bpf-next 3/5] selftests/bpf: Add re-attach test to fexit_test Jiri Olsa
@ 2021-04-07 22:51 ` Andrii Nakryiko
2021-04-08 11:51 ` Jiri Olsa
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-04-07 22:51 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Toke Høiland-Jørgensen
On Wed, Apr 7, 2021 at 4:21 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding the test to re-attach (detach/attach again) tracing
> fexit programs, plus check that already linked program can't
> be attached again.
>
> Fixing the number of check-ed results, which should be 8.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> .../selftests/bpf/prog_tests/fexit_test.c | 48 +++++++++++++++----
> 1 file changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_test.c b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
> index 78d7a2765c27..579e620e6612 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
> @@ -3,20 +3,24 @@
> #include <test_progs.h>
> #include "fexit_test.skel.h"
>
> -void test_fexit_test(void)
> +static __u32 duration;
> +
> +static int fexit_test(struct fexit_test *fexit_skel)
> {
> - struct fexit_test *fexit_skel = NULL;
> + struct bpf_link *link;
> int err, prog_fd, i;
> - __u32 duration = 0, retval;
> __u64 *result;
> -
> - fexit_skel = fexit_test__open_and_load();
> - if (CHECK(!fexit_skel, "fexit_skel_load", "fexit skeleton failed\n"))
> - goto cleanup;
> + __u32 retval;
>
> err = fexit_test__attach(fexit_skel);
> if (CHECK(err, "fexit_attach", "fexit attach failed: %d\n", err))
> - goto cleanup;
> + return err;
> +
> + /* Check that already linked program can't be attached again. */
> + link = bpf_program__attach(fexit_skel->progs.test1);
> + if (CHECK(!IS_ERR(link), "fexit_attach_link",
> + "re-attach without detach should not succeed"))
> + return -1;
>
> prog_fd = bpf_program__fd(fexit_skel->progs.test1);
> err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
> @@ -26,12 +30,36 @@ void test_fexit_test(void)
> err, errno, retval, duration);
>
> result = (__u64 *)fexit_skel->bss;
> - for (i = 0; i < 6; i++) {
> + for (i = 0; i < 8; i++) {
> if (CHECK(result[i] != 1, "result",
> "fexit_test%d failed err %lld\n", i + 1, result[i]))
> - goto cleanup;
> + return -1;
> }
>
> + fexit_test__detach(fexit_skel);
> +
> + /* zero results for re-attach test */
> + for (i = 0; i < 8; i++)
> + result[i] = 0;
memset(fexit_skel->bss, 0, sizeof(*fexit_skel->bss)) ? ;)
and see my nits in previous patch about ASSERT over CHECK
> + return 0;
> +}
> +
> +void test_fexit_test(void)
> +{
> + struct fexit_test *fexit_skel = NULL;
> + int err;
> +
> + fexit_skel = fexit_test__open_and_load();
> + if (CHECK(!fexit_skel, "fexit_skel_load", "fexit skeleton failed\n"))
> + goto cleanup;
> +
> + err = fexit_test(fexit_skel);
> + if (CHECK(err, "fexit_test", "first attach failed\n"))
> + goto cleanup;
> +
> + err = fexit_test(fexit_skel);
> + CHECK(err, "fexit_test", "second attach failed\n");
> +
> cleanup:
> fexit_test__destroy(fexit_skel);
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCHv2 bpf-next 3/5] selftests/bpf: Add re-attach test to fexit_test
2021-04-07 22:51 ` Andrii Nakryiko
@ 2021-04-08 11:51 ` Jiri Olsa
0 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2021-04-08 11:51 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
On Wed, Apr 07, 2021 at 03:51:46PM -0700, Andrii Nakryiko wrote:
> On Wed, Apr 7, 2021 at 4:21 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding the test to re-attach (detach/attach again) tracing
> > fexit programs, plus check that already linked program can't
> > be attached again.
> >
> > Fixing the number of check-ed results, which should be 8.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > .../selftests/bpf/prog_tests/fexit_test.c | 48 +++++++++++++++----
> > 1 file changed, 38 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_test.c b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
> > index 78d7a2765c27..579e620e6612 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/fexit_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/fexit_test.c
> > @@ -3,20 +3,24 @@
> > #include <test_progs.h>
> > #include "fexit_test.skel.h"
> >
> > -void test_fexit_test(void)
> > +static __u32 duration;
> > +
> > +static int fexit_test(struct fexit_test *fexit_skel)
> > {
> > - struct fexit_test *fexit_skel = NULL;
> > + struct bpf_link *link;
> > int err, prog_fd, i;
> > - __u32 duration = 0, retval;
> > __u64 *result;
> > -
> > - fexit_skel = fexit_test__open_and_load();
> > - if (CHECK(!fexit_skel, "fexit_skel_load", "fexit skeleton failed\n"))
> > - goto cleanup;
> > + __u32 retval;
> >
> > err = fexit_test__attach(fexit_skel);
> > if (CHECK(err, "fexit_attach", "fexit attach failed: %d\n", err))
> > - goto cleanup;
> > + return err;
> > +
> > + /* Check that already linked program can't be attached again. */
> > + link = bpf_program__attach(fexit_skel->progs.test1);
> > + if (CHECK(!IS_ERR(link), "fexit_attach_link",
> > + "re-attach without detach should not succeed"))
> > + return -1;
> >
> > prog_fd = bpf_program__fd(fexit_skel->progs.test1);
> > err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
> > @@ -26,12 +30,36 @@ void test_fexit_test(void)
> > err, errno, retval, duration);
> >
> > result = (__u64 *)fexit_skel->bss;
> > - for (i = 0; i < 6; i++) {
> > + for (i = 0; i < 8; i++) {
> > if (CHECK(result[i] != 1, "result",
> > "fexit_test%d failed err %lld\n", i + 1, result[i]))
> > - goto cleanup;
> > + return -1;
> > }
> >
> > + fexit_test__detach(fexit_skel);
> > +
> > + /* zero results for re-attach test */
> > + for (i = 0; i < 8; i++)
> > + result[i] = 0;
>
> memset(fexit_skel->bss, 0, sizeof(*fexit_skel->bss)) ? ;)
>
> and see my nits in previous patch about ASSERT over CHECK
sure ;-) thanks
jirka
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv2 bpf-next 4/5] selftests/bpf: Add re-attach test to lsm test
2021-04-06 21:29 [PATCHv2 bpf-next 0/5] bpf: Tracing and lsm programs re-attach Jiri Olsa
` (2 preceding siblings ...)
2021-04-06 21:29 ` [PATCHv2 bpf-next 3/5] selftests/bpf: Add re-attach test to fexit_test Jiri Olsa
@ 2021-04-06 21:29 ` Jiri Olsa
2021-04-06 21:29 ` [PATCHv2 bpf-next 5/5] selftests/bpf: Test that module can't be unloaded with attached trampoline Jiri Olsa
4 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2021-04-06 21:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
Adding the test to re-attach (detach/attach again) lsm programs,
plus check that already linked program can't be attached again.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/test_lsm.c | 48 +++++++++++++++----
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_lsm.c b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
index 2755e4f81499..d492e76e01cf 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_lsm.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
@@ -18,6 +18,8 @@ char *CMD_ARGS[] = {"true", NULL};
#define GET_PAGE_ADDR(ADDR, PAGE_SIZE) \
(char *)(((unsigned long) (ADDR + PAGE_SIZE)) & ~(PAGE_SIZE-1))
+static int duration = 0;
+
int stack_mprotect(void)
{
void *buf;
@@ -51,23 +53,25 @@ int exec_cmd(int *monitored_pid)
return -EINVAL;
}
-void test_test_lsm(void)
+static int test_lsm(struct lsm *skel)
{
- struct lsm *skel = NULL;
- int err, duration = 0;
+ struct bpf_link *link;
int buf = 1234;
-
- skel = lsm__open_and_load();
- if (CHECK(!skel, "skel_load", "lsm skeleton failed\n"))
- goto close_prog;
+ int err;
err = lsm__attach(skel);
if (CHECK(err, "attach", "lsm attach failed: %d\n", err))
- goto close_prog;
+ return err;
+
+ /* Check that already linked program can't be attached again. */
+ link = bpf_program__attach(skel->progs.test_int_hook);
+ if (CHECK(!IS_ERR(link), "attach_link",
+ "re-attach without detach should not succeed"))
+ return -1;
err = exec_cmd(&skel->bss->monitored_pid);
if (CHECK(err < 0, "exec_cmd", "err %d errno %d\n", err, errno))
- goto close_prog;
+ return err;
CHECK(skel->bss->bprm_count != 1, "bprm_count", "bprm_count = %d\n",
skel->bss->bprm_count);
@@ -77,7 +81,7 @@ void test_test_lsm(void)
err = stack_mprotect();
if (CHECK(errno != EPERM, "stack_mprotect", "want err=EPERM, got %d\n",
errno))
- goto close_prog;
+ return err;
CHECK(skel->bss->mprotect_count != 1, "mprotect_count",
"mprotect_count = %d\n", skel->bss->mprotect_count);
@@ -89,6 +93,30 @@ void test_test_lsm(void)
CHECK(skel->bss->copy_test != 3, "copy_test",
"copy_test = %d\n", skel->bss->copy_test);
+ lsm__detach(skel);
+
+ skel->bss->copy_test = 0;
+ skel->bss->bprm_count = 0;
+ skel->bss->mprotect_count = 0;
+ return 0;
+}
+
+void test_test_lsm(void)
+{
+ struct lsm *skel = NULL;
+ int err;
+
+ skel = lsm__open_and_load();
+ if (CHECK(!skel, "lsm_skel_load", "lsm skeleton failed\n"))
+ goto close_prog;
+
+ err = test_lsm(skel);
+ if (CHECK(err, "test_lsm", "first attach failed\n"))
+ goto close_prog;
+
+ err = test_lsm(skel);
+ CHECK(err, "test_lsm", "second attach failed\n");
+
close_prog:
lsm__destroy(skel);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCHv2 bpf-next 5/5] selftests/bpf: Test that module can't be unloaded with attached trampoline
2021-04-06 21:29 [PATCHv2 bpf-next 0/5] bpf: Tracing and lsm programs re-attach Jiri Olsa
` (3 preceding siblings ...)
2021-04-06 21:29 ` [PATCHv2 bpf-next 4/5] selftests/bpf: Add re-attach test to lsm test Jiri Olsa
@ 2021-04-06 21:29 ` Jiri Olsa
2021-04-07 23:04 ` Andrii Nakryiko
4 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2021-04-06 21:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
Adding test to verify that once we attach module's trampoline,
the module can't be unloaded.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/module_attach.c | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
index 5bc53d53d86e..d180b8c28287 100644
--- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
@@ -45,12 +45,18 @@ static int trigger_module_test_write(int write_sz)
return 0;
}
+static int delete_module(const char *name, int flags)
+{
+ return syscall(__NR_delete_module, name, flags);
+}
+
void test_module_attach(void)
{
const int READ_SZ = 456;
const int WRITE_SZ = 457;
struct test_module_attach* skel;
struct test_module_attach__bss *bss;
+ struct bpf_link *link;
int err;
skel = test_module_attach__open();
@@ -84,6 +90,23 @@ void test_module_attach(void)
ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
+ test_module_attach__detach(skel);
+
+ /* attach fentry/fexit and make sure it get's module reference */
+ link = bpf_program__attach(skel->progs.handle_fentry);
+ if (CHECK(IS_ERR(link), "attach_fentry", "err: %ld\n", PTR_ERR(link)))
+ goto cleanup;
+
+ ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
+ bpf_link__destroy(link);
+
+ link = bpf_program__attach(skel->progs.handle_fexit);
+ if (CHECK(IS_ERR(link), "attach_fexit", "err: %ld\n", PTR_ERR(link)))
+ goto cleanup;
+
+ ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
+ bpf_link__destroy(link);
+
cleanup:
test_module_attach__destroy(skel);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCHv2 bpf-next 5/5] selftests/bpf: Test that module can't be unloaded with attached trampoline
2021-04-06 21:29 ` [PATCHv2 bpf-next 5/5] selftests/bpf: Test that module can't be unloaded with attached trampoline Jiri Olsa
@ 2021-04-07 23:04 ` Andrii Nakryiko
2021-04-08 11:59 ` Jiri Olsa
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-04-07 23:04 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Toke Høiland-Jørgensen
On Wed, Apr 7, 2021 at 4:22 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test to verify that once we attach module's trampoline,
> the module can't be unloaded.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
To be fair, to test that you are actually testing what you think you
are testing, you'd have to prove that you *can* detach when no program
is attached to bpf_testmod ;) You'd also need kern_sync_rcu() to wait
for all the async clean up to complete inside the kernel. But that
doesn't interact with other tests well, so I think it's fine.
grumpily due to CHECK() usage (please do consider updating to ASSERT):
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> .../selftests/bpf/prog_tests/module_attach.c | 23 +++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> index 5bc53d53d86e..d180b8c28287 100644
> --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> @@ -45,12 +45,18 @@ static int trigger_module_test_write(int write_sz)
> return 0;
> }
>
> +static int delete_module(const char *name, int flags)
> +{
> + return syscall(__NR_delete_module, name, flags);
> +}
> +
> void test_module_attach(void)
> {
> const int READ_SZ = 456;
> const int WRITE_SZ = 457;
> struct test_module_attach* skel;
> struct test_module_attach__bss *bss;
> + struct bpf_link *link;
> int err;
>
> skel = test_module_attach__open();
> @@ -84,6 +90,23 @@ void test_module_attach(void)
> ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
> ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
>
> + test_module_attach__detach(skel);
> +
> + /* attach fentry/fexit and make sure it get's module reference */
> + link = bpf_program__attach(skel->progs.handle_fentry);
> + if (CHECK(IS_ERR(link), "attach_fentry", "err: %ld\n", PTR_ERR(link)))
> + goto cleanup;
> +
> + ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
> + bpf_link__destroy(link);
> +
> + link = bpf_program__attach(skel->progs.handle_fexit);
> + if (CHECK(IS_ERR(link), "attach_fexit", "err: %ld\n", PTR_ERR(link)))
> + goto cleanup;
> +
> + ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
> + bpf_link__destroy(link);
> +
> cleanup:
> test_module_attach__destroy(skel);
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCHv2 bpf-next 5/5] selftests/bpf: Test that module can't be unloaded with attached trampoline
2021-04-07 23:04 ` Andrii Nakryiko
@ 2021-04-08 11:59 ` Jiri Olsa
0 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2021-04-08 11:59 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Toke Høiland-Jørgensen
On Wed, Apr 07, 2021 at 04:04:48PM -0700, Andrii Nakryiko wrote:
> On Wed, Apr 7, 2021 at 4:22 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding test to verify that once we attach module's trampoline,
> > the module can't be unloaded.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
>
> To be fair, to test that you are actually testing what you think you
> are testing, you'd have to prove that you *can* detach when no program
> is attached to bpf_testmod ;) You'd also need kern_sync_rcu() to wait
> for all the async clean up to complete inside the kernel. But that
> doesn't interact with other tests well, so I think it's fine.
well without the kernel change the module gets unloaded
and the test below fails.. we could add module unload
test, but as you described it could probably interfere
with other tests
>
> grumpily due to CHECK() usage (please do consider updating to ASSERT):
ok, will check
thanks,
jirka
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > .../selftests/bpf/prog_tests/module_attach.c | 23 +++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> > index 5bc53d53d86e..d180b8c28287 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
> > @@ -45,12 +45,18 @@ static int trigger_module_test_write(int write_sz)
> > return 0;
> > }
> >
> > +static int delete_module(const char *name, int flags)
> > +{
> > + return syscall(__NR_delete_module, name, flags);
> > +}
> > +
> > void test_module_attach(void)
> > {
> > const int READ_SZ = 456;
> > const int WRITE_SZ = 457;
> > struct test_module_attach* skel;
> > struct test_module_attach__bss *bss;
> > + struct bpf_link *link;
> > int err;
> >
> > skel = test_module_attach__open();
> > @@ -84,6 +90,23 @@ void test_module_attach(void)
> > ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
> > ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
> >
> > + test_module_attach__detach(skel);
> > +
> > + /* attach fentry/fexit and make sure it get's module reference */
> > + link = bpf_program__attach(skel->progs.handle_fentry);
> > + if (CHECK(IS_ERR(link), "attach_fentry", "err: %ld\n", PTR_ERR(link)))
> > + goto cleanup;
> > +
> > + ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
> > + bpf_link__destroy(link);
> > +
> > + link = bpf_program__attach(skel->progs.handle_fexit);
> > + if (CHECK(IS_ERR(link), "attach_fexit", "err: %ld\n", PTR_ERR(link)))
> > + goto cleanup;
> > +
> > + ASSERT_ERR(delete_module("bpf_testmod", 0), "delete_module");
> > + bpf_link__destroy(link);
> > +
> > cleanup:
> > test_module_attach__destroy(skel);
> > }
> > --
> > 2.30.2
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread