lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 2 Jul 2024 15:10:55 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jiri Olsa <jolsa@...nel.org>
Cc: Oleg Nesterov <oleg@...hat.com>, Peter Zijlstra <peterz@...radead.org>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org, Martin KaFai Lau <kafai@...com>, 
	Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, 
	John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...omium.org>, 
	Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Steven Rostedt <rostedt@...dmis.org>, 
	Masami Hiramatsu <mhiramat@...nel.org>, linux-kernel@...r.kernel.org, 
	linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCHv2 bpf-next 9/9] selftests/bpf: Add uprobe session
 consumers test

On Mon, Jul 1, 2024 at 9:44 AM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Adding test that attached/detaches multiple consumers on
> single uprobe and verifies all were hit as expected.
>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
>  .../bpf/prog_tests/uprobe_multi_test.c        | 203 ++++++++++++++++++
>  .../progs/uprobe_multi_session_consumers.c    |  53 +++++
>  2 files changed, 256 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_session_consumers.c
>

This is clever, though bit notation obscures the meaning of the code a
bit. But thanks for the long comment explaining the overall idea.

> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> index b521590fdbb9..83eac954cf00 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> @@ -9,6 +9,7 @@
>  #include "uprobe_multi_session.skel.h"
>  #include "uprobe_multi_session_cookie.skel.h"
>  #include "uprobe_multi_session_recursive.skel.h"
> +#include "uprobe_multi_session_consumers.skel.h"
>  #include "bpf/libbpf_internal.h"
>  #include "testing_helpers.h"
>  #include "../sdt.h"
> @@ -739,6 +740,206 @@ static void test_session_recursive_skel_api(void)
>         uprobe_multi_session_recursive__destroy(skel);
>  }
>
> +static int uprobe_attach(struct uprobe_multi_session_consumers *skel, int bit)
> +{
> +       struct bpf_program **prog = &skel->progs.uprobe_0 + bit;
> +       struct bpf_link **link = &skel->links.uprobe_0 + bit;
> +       LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
> +
> +       /*
> +        * bit: 0,1 uprobe session
> +        * bit: 2,3 uprobe entry
> +        * bit: 4,5 uprobe return
> +        */
> +       opts.session = bit < 2;
> +       opts.retprobe = bit == 4 || bit == 5;
> +
> +       *link = bpf_program__attach_uprobe_multi(*prog, 0, "/proc/self/exe",
> +                                                "uprobe_session_consumer_test",
> +                                                &opts);
> +       if (!ASSERT_OK_PTR(*link, "bpf_program__attach_uprobe_multi"))
> +               return -1;
> +       return 0;
> +}
> +
> +static void uprobe_detach(struct uprobe_multi_session_consumers *skel, int bit)
> +{
> +       struct bpf_link **link = &skel->links.uprobe_0 + bit;

ok, this is nasty, no one guarantees this should keep working,
explicit switch would be preferable

> +
> +       bpf_link__destroy(*link);
> +       *link = NULL;
> +}
> +
> +static bool test_bit(int bit, unsigned long val)
> +{
> +       return val & (1 << bit);
> +}
> +
> +noinline int
> +uprobe_session_consumer_test(struct uprobe_multi_session_consumers *skel,
> +                            unsigned long before, unsigned long after)
> +{
> +       int bit;
> +
> +       /* detach uprobe for each unset bit in 'before' state ... */
> +       for (bit = 0; bit < 6; bit++) {

Does "bit" correspond to the uprobe_X program? Maybe call it an uprobe
index or something, if that's the case? bits are just representations,
but semantically meaningful is identifier of an uprobe program, right?

> +               if (test_bit(bit, before) && !test_bit(bit, after))
> +                       uprobe_detach(skel, bit);
> +       }
> +
> +       /* ... and attach all new bits in 'after' state */
> +       for (bit = 0; bit < 6; bit++) {
> +               if (!test_bit(bit, before) && test_bit(bit, after)) {
> +                       if (!ASSERT_OK(uprobe_attach(skel, bit), "uprobe_attach_after"))
> +                               return -1;
> +               }
> +       }
> +       return 0;
> +}
> +

[...]

> +
> +static void test_session_consumers(void)
> +{
> +       struct uprobe_multi_session_consumers *skel;
> +       int before, after;
> +
> +       skel = uprobe_multi_session_consumers__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "uprobe_multi_session_consumers__open_and_load"))
> +               return;
> +
> +       /*
> +        * The idea of this test is to try all possible combinations of
> +        * uprobes consumers attached on single function.
> +        *
> +        *  - 1 uprobe session with return handler called
> +        *  - 1 uprobe session without return handler called
> +        *  - 2 uprobe entry consumer
> +        *  - 2 uprobe exit consumers
> +        *
> +        * The test uses 6 uprobes attached on single function, but that
> +        * translates into single uprobe with 6 consumers in kernel.
> +        *
> +        * The before/after values present the state of attached consumers
> +        * before and after the probed function:
> +        *
> +        *  bit 0   : uprobe session with return
> +        *  bit 1   : uprobe session with no return
> +        *  bit 2,3 : uprobe entry
> +        *  bit 4,5 : uprobe return
> +        *
> +        * For example for:
> +        *
> +        *   before = 0b10101
> +        *   after  = 0b00110
> +        *
> +        * it means that before we call 'uprobe_session_consumer_test' we
> +        * attach uprobes defined in 'before' value:
> +        *
> +        *   - bit 0: uprobe session with return
> +        *   - bit 2: uprobe entry
> +        *   - bit 4: uprobe return
> +        *
> +        * uprobe_session_consumer_test is called and inside it we attach
> +        * and detach * uprobes based on 'after' value:
> +        *
> +        *   - bit 0: uprobe session with return is detached
> +        *   - bit 1: uprobe session without return is attached
> +        *   - bit 2: stays untouched
> +        *   - bit 4: uprobe return is detached
> +        *
> +        * uprobe_session_consumer_test returs and we check counters values
> +        * increased by bpf programs on each uprobe to match the expected
> +        * count based on before/after bits.
> +        */
> +       for (before = 0; before < 64; before++) {
> +               for (after = 0; after < 64; after++)
> +                       session_consumer_test(skel, before, after);
> +       }
> +
> +       uprobe_multi_session_consumers__destroy(skel);
> +}
> +

[...]

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ