[PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

[PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

[Daniel Xu]

This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
On top of being generally useful for unit testing kprobe progs, this
feature more specifically helps solve a relability problem with bpftrace
BEGIN and END probes.

BEGIN and END probes are run exactly once at the beginning and end of a
bpftrace tracing session, respectively. bpftrace currently implements
the probes by creating two dummy functions and attaching the BEGIN and
END progs, if defined, to those functions and calling the dummy
functions as appropriate. This works pretty well most of the time except
for when distros strip symbols from bpftrace. Every now and then this
happens and users get confused. Having PROG_TEST_RUN support will help
solve this issue by allowing us to directly trigger uprobes from
userspace.

Admittedly, this is a pretty specific problem and could probably be
solved other ways. However, PROG_TEST_RUN also makes unit testing more
convenient, especially as users start building more complex tracing
applications. So I see this as killing two birds with one stone.

Daniel Xu (2):
  bpf, test_run: Add PROG_TEST_RUN support to kprobe
  Add PROG_TEST_RUN selftest for BPF_PROG_TYPE_KPROBE

 include/linux/bpf.h                           | 10 ++++
 kernel/trace/bpf_trace.c                      |  1 +
 net/bpf/test_run.c                            | 36 ++++++++++++
 .../selftests/bpf/prog_tests/kprobe_ctx.c     | 57 +++++++++++++++++++
 .../testing/selftests/bpf/progs/kprobe_ctx.c  | 33 +++++++++++
 5 files changed, 137 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
 create mode 100644 tools/testing/selftests/bpf/progs/kprobe_ctx.c

-- 
2.36.1

[PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

[Daniel Xu]

This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
top of being generally useful for unit testing kprobe progs, this commit
more specifically helps solve a relability problem with bpftrace BEGIN
and END probes.

BEGIN and END probes are run exactly once at the beginning and end of a
bpftrace tracing session, respectively. bpftrace currently implements
the probes by creating two dummy functions and attaching the BEGIN and
END progs, if defined, to those functions and calling the dummy
functions as appropriate. This works pretty well most of the time except
for when distros strip symbols from bpftrace. Every now and then this
happens and users get confused. Having PROG_TEST_RUN support will help
solve this issue by allowing us to directly trigger uprobes from
userspace.

Admittedly, this is a pretty specific problem and could probably be
solved other ways. However, PROG_TEST_RUN also makes unit testing more
convenient, especially as users start building more complex tracing
applications. So I see this as killing two birds with one stone.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 include/linux/bpf.h      | 10 ++++++++++
 kernel/trace/bpf_trace.c |  1 +
 net/bpf/test_run.c       | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2b914a56a2c5..dec3082ee158 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1751,6 +1751,9 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
 int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
 				const union bpf_attr *kattr,
 				union bpf_attr __user *uattr);
+int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
+			     const union bpf_attr *kattr,
+			     union bpf_attr __user *uattr);
 bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		    const struct bpf_prog *prog,
 		    struct bpf_insn_access_aux *info);
@@ -1998,6 +2001,13 @@ static inline int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
 	return -ENOTSUPP;
 }
 
+static inline int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
+					   const union bpf_attr *kattr,
+					   union bpf_attr __user *uattr)
+{
+	return -ENOTSUPP;
+}
+
 static inline void bpf_map_put(struct bpf_map *map)
 {
 }
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 10b157a6d73e..b452e84b9ba4 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1363,6 +1363,7 @@ const struct bpf_verifier_ops kprobe_verifier_ops = {
 };
 
 const struct bpf_prog_ops kprobe_prog_ops = {
+	.test_run = bpf_prog_test_run_kprobe,
 };
 
 BPF_CALL_5(bpf_perf_event_output_tp, void *, tp_buff, struct bpf_map *, map,
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 56f059b3c242..0b6fc17ce901 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1622,6 +1622,42 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
 	return err;
 }
 
+int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
+			     const union bpf_attr *kattr,
+			     union bpf_attr __user *uattr)
+{
+	void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
+	__u32 ctx_size_in = kattr->test.ctx_size_in;
+	u32 repeat = kattr->test.repeat;
+	struct pt_regs *ctx = NULL;
+	u32 retval, duration;
+	int err = 0;
+
+	if (kattr->test.data_in || kattr->test.data_out ||
+	    kattr->test.ctx_out || kattr->test.flags ||
+	    kattr->test.cpu || kattr->test.batch_size)
+		return -EINVAL;
+
+	if (ctx_size_in != sizeof(struct pt_regs))
+		return -EINVAL;
+
+	ctx = memdup_user(ctx_in, ctx_size_in);
+	if (IS_ERR(ctx))
+		return PTR_ERR(ctx);
+
+	err = bpf_test_run(prog, ctx, repeat, &retval, &duration, false);
+	if (err)
+		goto out;
+
+	if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)) ||
+	    copy_to_user(&uattr->test.duration, &duration, sizeof(duration))) {
+		err = -EFAULT;
+	}
+out:
+	kfree(ctx);
+	return err;
+}
+
 static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
 	.owner        = THIS_MODULE,
 	.check_set        = &test_sk_check_kfunc_ids,
-- 
2.36.1

[PATCH bpf-next 2/2] selftests/bpf: Add PROG_TEST_RUN selftest for BPF_PROG_TYPE_KPROBE

[Daniel Xu]

This commit adds a selftest to test that we can both PROG_TEST_RUN a
kprobe prog and set its context.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 .../selftests/bpf/prog_tests/kprobe_ctx.c     | 57 +++++++++++++++++++
 .../testing/selftests/bpf/progs/kprobe_ctx.c  | 33 +++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
 create mode 100644 tools/testing/selftests/bpf/progs/kprobe_ctx.c

diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c b/tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
new file mode 100644
index 000000000000..260966fd4506
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <linux/ptrace.h>
+#include "kprobe_ctx.skel.h"
+
+/*
+ * x86_64 happens to be one of the architectures that exports the
+ * kernel `struct pt_regs` to userspace ABI. For the architectures
+ * that don't, users will have to extract `struct pt_regs` from vmlinux
+ * BTF in order to use BPF_PROG_TYPE_KPROBE's BPF_PROG_RUN functionality.
+ *
+ * We choose to only test x86 here to keep the test simple.
+ */
+void test_kprobe_ctx(void)
+{
+#ifdef __x86_64__
+	struct pt_regs regs = {
+		.rdi = 1,
+		.rsi = 2,
+		.rdx = 3,
+		.rcx = 4,
+		.r8 = 5,
+	};
+
+	LIBBPF_OPTS(bpf_test_run_opts, tattr,
+		.ctx_in = &regs,
+		.ctx_size_in = sizeof(regs),
+	);
+
+	struct kprobe_ctx *skel = NULL;
+	int prog_fd;
+	int err;
+
+	skel = kprobe_ctx__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	skel->bss->expected_p1 = (void *)1;
+	skel->bss->expected_p2 = (void *)2;
+	skel->bss->expected_p3 = (void *)3;
+	skel->bss->expected_p4 = (void *)4;
+	skel->bss->expected_p5 = (void *)5;
+
+	prog_fd = bpf_program__fd(skel->progs.prog);
+	err = bpf_prog_test_run_opts(prog_fd, &tattr);
+	if (!ASSERT_OK(err, "bpf_prog_test_run"))
+		goto cleanup;
+
+	if (!ASSERT_TRUE(skel->bss->ret, "ret"))
+		goto cleanup;
+
+	if (!ASSERT_GT(tattr.duration, 0, "duration"))
+		goto cleanup;
+cleanup:
+	kprobe_ctx__destroy(skel);
+#endif
+}
diff --git a/tools/testing/selftests/bpf/progs/kprobe_ctx.c b/tools/testing/selftests/bpf/progs/kprobe_ctx.c
new file mode 100644
index 000000000000..98063c549930
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kprobe_ctx.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+volatile void *expected_p1;
+volatile void *expected_p2;
+volatile void *expected_p3;
+volatile void *expected_p4;
+volatile void *expected_p5;
+volatile bool ret = false;
+
+SEC("kprobe/this_function_does_not_exist")
+int prog(struct pt_regs *ctx)
+{
+	void *p1, *p2, *p3, *p4, *p5;
+
+	p1 = (void *)PT_REGS_PARM1(ctx);
+	p2 = (void *)PT_REGS_PARM2(ctx);
+	p3 = (void *)PT_REGS_PARM3(ctx);
+	p4 = (void *)PT_REGS_PARM4(ctx);
+	p5 = (void *)PT_REGS_PARM5(ctx);
+
+	if (p1 != expected_p1 || p2 != expected_p2 || p3 != expected_p3 ||
+	    p4 != expected_p4 || p5 != expected_p5)
+		return 0;
+
+	ret = true;
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.36.1

Re: [PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

[Song Liu]

On Sun, May 29, 2022 at 3:06 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
> On top of being generally useful for unit testing kprobe progs, this
> feature more specifically helps solve a relability problem with bpftrace
> BEGIN and END probes.
>
> BEGIN and END probes are run exactly once at the beginning and end of a
> bpftrace tracing session, respectively. bpftrace currently implements
> the probes by creating two dummy functions and attaching the BEGIN and
> END progs, if defined, to those functions and calling the dummy
> functions as appropriate. This works pretty well most of the time except
> for when distros strip symbols from bpftrace. Every now and then this
> happens and users get confused. Having PROG_TEST_RUN support will help
> solve this issue by allowing us to directly trigger uprobes from
> userspace.
>
> Admittedly, this is a pretty specific problem and could probably be
> solved other ways. However, PROG_TEST_RUN also makes unit testing more
> convenient, especially as users start building more complex tracing
> applications. So I see this as killing two birds with one stone.

We have BPF_PROG_RUN which is an alias of BPF_PROG_TEST_RUN. I guess
that's a better name for the BEGIN/END use case.

Have you checked out bpf_prog_test_run_raw_tp()? AFAICT, it works as good as
kprobe for this use case.

Thanks,
Song

Re: [PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

[Daniel Xu]

Hi Song,

On Sun, May 29, 2022 at 11:00:48PM -0700, Song Liu wrote:
> On Sun, May 29, 2022 at 3:06 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> >
> > This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
> > On top of being generally useful for unit testing kprobe progs, this
> > feature more specifically helps solve a relability problem with bpftrace
> > BEGIN and END probes.
> >
> > BEGIN and END probes are run exactly once at the beginning and end of a
> > bpftrace tracing session, respectively. bpftrace currently implements
> > the probes by creating two dummy functions and attaching the BEGIN and
> > END progs, if defined, to those functions and calling the dummy
> > functions as appropriate. This works pretty well most of the time except
> > for when distros strip symbols from bpftrace. Every now and then this
> > happens and users get confused. Having PROG_TEST_RUN support will help
> > solve this issue by allowing us to directly trigger uprobes from
> > userspace.
> >
> > Admittedly, this is a pretty specific problem and could probably be
> > solved other ways. However, PROG_TEST_RUN also makes unit testing more
> > convenient, especially as users start building more complex tracing
> > applications. So I see this as killing two birds with one stone.
> 
> We have BPF_PROG_RUN which is an alias of BPF_PROG_TEST_RUN. I guess
> that's a better name for the BEGIN/END use case.

Right, sorry. Was getting names mixed up.

> 
> Have you checked out bpf_prog_test_run_raw_tp()? AFAICT, it works as good as
> kprobe for this use case.

I just took a look -- I think it'll work for BEGIN/END use case. But
also like I mentioned, BPF_PROG_RUN/BPF_PROG_TEST_RUN support for
kprobes is probably still useful. For example if kprobe accesses 13th
register. I suppose the raw_tp 12 arg limit could be lifted but it might
be tricky to capture that logic in the absence of something like `struct
pt_regs` to check the ctx_size_in against.

Thanks,
Daniel

Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

[kernel test robot]

Hi Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: s390-randconfig-r026-20220531 (https://download.01.org/0day-ci/archive/20220531/202205312315.3VC5jz4T-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/a547a4c795103fd002d3bbb5ee4d7141113716c0
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
        git checkout a547a4c795103fd002d3bbb5ee4d7141113716c0
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> s390-linux-ld: kernel/trace/bpf_trace.o:(.data.rel.ro+0x100): undefined reference to `bpf_prog_test_run_kprobe'
   s390-linux-ld: drivers/dma/qcom/hidma.o: in function `hidma_probe':
   hidma.c:(.text+0x313e): undefined reference to `devm_ioremap_resource'
   s390-linux-ld: hidma.c:(.text+0x3192): undefined reference to `devm_ioremap_resource'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH bpf-next 0/2] Add PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE

[Song Liu]

On Mon, May 30, 2022 at 7:56 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> Hi Song,
>
> On Sun, May 29, 2022 at 11:00:48PM -0700, Song Liu wrote:
> > On Sun, May 29, 2022 at 3:06 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> > >
> > > This patchset adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs.
> > > On top of being generally useful for unit testing kprobe progs, this
> > > feature more specifically helps solve a relability problem with bpftrace
> > > BEGIN and END probes.
> > >
> > > BEGIN and END probes are run exactly once at the beginning and end of a
> > > bpftrace tracing session, respectively. bpftrace currently implements
> > > the probes by creating two dummy functions and attaching the BEGIN and
> > > END progs, if defined, to those functions and calling the dummy
> > > functions as appropriate. This works pretty well most of the time except
> > > for when distros strip symbols from bpftrace. Every now and then this
> > > happens and users get confused. Having PROG_TEST_RUN support will help
> > > solve this issue by allowing us to directly trigger uprobes from
> > > userspace.
> > >
> > > Admittedly, this is a pretty specific problem and could probably be
> > > solved other ways. However, PROG_TEST_RUN also makes unit testing more
> > > convenient, especially as users start building more complex tracing
> > > applications. So I see this as killing two birds with one stone.
> >
> > We have BPF_PROG_RUN which is an alias of BPF_PROG_TEST_RUN. I guess
> > that's a better name for the BEGIN/END use case.
>
> Right, sorry. Was getting names mixed up.
>
> >
> > Have you checked out bpf_prog_test_run_raw_tp()? AFAICT, it works as good as
> > kprobe for this use case.
>
> I just took a look -- I think it'll work for BEGIN/END use case. But
> also like I mentioned, BPF_PROG_RUN/BPF_PROG_TEST_RUN support for
> kprobes is probably still useful. For example if kprobe accesses 13th
> register. I suppose the raw_tp 12 arg limit could be lifted but it might
> be tricky to capture that logic in the absence of something like `struct
> pt_regs` to check the ctx_size_in against.

Agreed that unit tests support for kprobe programs is great.

Thanks,
Song

Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

[Song Liu]

On Sun, May 29, 2022 at 3:06 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
> top of being generally useful for unit testing kprobe progs, this commit
> more specifically helps solve a relability problem with bpftrace BEGIN
> and END probes.
>
> BEGIN and END probes are run exactly once at the beginning and end of a
> bpftrace tracing session, respectively. bpftrace currently implements
> the probes by creating two dummy functions and attaching the BEGIN and
> END progs, if defined, to those functions and calling the dummy
> functions as appropriate. This works pretty well most of the time except
> for when distros strip symbols from bpftrace. Every now and then this
> happens and users get confused. Having PROG_TEST_RUN support will help
> solve this issue by allowing us to directly trigger uprobes from
> userspace.
>
> Admittedly, this is a pretty specific problem and could probably be
> solved other ways. However, PROG_TEST_RUN also makes unit testing more
> convenient, especially as users start building more complex tracing
> applications. So I see this as killing two birds with one stone.
>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
>  include/linux/bpf.h      | 10 ++++++++++
>  kernel/trace/bpf_trace.c |  1 +
>  net/bpf/test_run.c       | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 2b914a56a2c5..dec3082ee158 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1751,6 +1751,9 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
>  int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
>                                 const union bpf_attr *kattr,
>                                 union bpf_attr __user *uattr);
> +int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
> +                            const union bpf_attr *kattr,
> +                            union bpf_attr __user *uattr);
>  bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>                     const struct bpf_prog *prog,
>                     struct bpf_insn_access_aux *info);
> @@ -1998,6 +2001,13 @@ static inline int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
>         return -ENOTSUPP;
>  }
>
> +static inline int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
> +                                          const union bpf_attr *kattr,
> +                                          union bpf_attr __user *uattr)
> +{
> +       return -ENOTSUPP;
> +}

As the kernel test bot reported, this is not enough to cover all
different configs. We can
follow the pattern with bpf_prog_test_run_tracing().

Otherwise, this looks good to me.

Song

> +
>  static inline void bpf_map_put(struct bpf_map *map)
>  {
>  }
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 10b157a6d73e..b452e84b9ba4 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1363,6 +1363,7 @@ const struct bpf_verifier_ops kprobe_verifier_ops = {
>  };
>
>  const struct bpf_prog_ops kprobe_prog_ops = {
> +       .test_run = bpf_prog_test_run_kprobe,
>  };
>
>  BPF_CALL_5(bpf_perf_event_output_tp, void *, tp_buff, struct bpf_map *, map,
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 56f059b3c242..0b6fc17ce901 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -1622,6 +1622,42 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
>         return err;
>  }
>
> +int bpf_prog_test_run_kprobe(struct bpf_prog *prog,
> +                            const union bpf_attr *kattr,
> +                            union bpf_attr __user *uattr)
> +{
> +       void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
> +       __u32 ctx_size_in = kattr->test.ctx_size_in;
> +       u32 repeat = kattr->test.repeat;
> +       struct pt_regs *ctx = NULL;
> +       u32 retval, duration;
> +       int err = 0;
> +
> +       if (kattr->test.data_in || kattr->test.data_out ||
> +           kattr->test.ctx_out || kattr->test.flags ||
> +           kattr->test.cpu || kattr->test.batch_size)
> +               return -EINVAL;
> +
> +       if (ctx_size_in != sizeof(struct pt_regs))
> +               return -EINVAL;
> +
> +       ctx = memdup_user(ctx_in, ctx_size_in);
> +       if (IS_ERR(ctx))
> +               return PTR_ERR(ctx);
> +
> +       err = bpf_test_run(prog, ctx, repeat, &retval, &duration, false);
> +       if (err)
> +               goto out;
> +
> +       if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)) ||
> +           copy_to_user(&uattr->test.duration, &duration, sizeof(duration))) {
> +               err = -EFAULT;
> +       }
> +out:
> +       kfree(ctx);
> +       return err;
> +}
> +
>  static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
>         .owner        = THIS_MODULE,
>         .check_set        = &test_sk_check_kfunc_ids,
> --
> 2.36.1
>

Re: [PATCH bpf-next 2/2] selftests/bpf: Add PROG_TEST_RUN selftest for BPF_PROG_TYPE_KPROBE

[Song Liu]

On Sun, May 29, 2022 at 3:06 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> This commit adds a selftest to test that we can both PROG_TEST_RUN a
> kprobe prog and set its context.

nit: per Documentation/process/submitting-patches.rst:

Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour.

>
> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>

Other than that,

Acked-by: Song Liu <songliubraving@fb.com>


> ---
>  .../selftests/bpf/prog_tests/kprobe_ctx.c     | 57 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/kprobe_ctx.c  | 33 +++++++++++
>  2 files changed, 90 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
>  create mode 100644 tools/testing/selftests/bpf/progs/kprobe_ctx.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c b/tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
> new file mode 100644
> index 000000000000..260966fd4506
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_ctx.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include <linux/ptrace.h>
> +#include "kprobe_ctx.skel.h"
> +
> +/*
> + * x86_64 happens to be one of the architectures that exports the
> + * kernel `struct pt_regs` to userspace ABI. For the architectures
> + * that don't, users will have to extract `struct pt_regs` from vmlinux
> + * BTF in order to use BPF_PROG_TYPE_KPROBE's BPF_PROG_RUN functionality.
> + *
> + * We choose to only test x86 here to keep the test simple.
> + */
> +void test_kprobe_ctx(void)
> +{
> +#ifdef __x86_64__
> +       struct pt_regs regs = {
> +               .rdi = 1,
> +               .rsi = 2,
> +               .rdx = 3,
> +               .rcx = 4,
> +               .r8 = 5,
> +       };
> +
> +       LIBBPF_OPTS(bpf_test_run_opts, tattr,
> +               .ctx_in = &regs,
> +               .ctx_size_in = sizeof(regs),
> +       );
> +
> +       struct kprobe_ctx *skel = NULL;
> +       int prog_fd;
> +       int err;
> +
> +       skel = kprobe_ctx__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "skel_open"))
> +               return;
> +
> +       skel->bss->expected_p1 = (void *)1;
> +       skel->bss->expected_p2 = (void *)2;
> +       skel->bss->expected_p3 = (void *)3;
> +       skel->bss->expected_p4 = (void *)4;
> +       skel->bss->expected_p5 = (void *)5;
> +
> +       prog_fd = bpf_program__fd(skel->progs.prog);
> +       err = bpf_prog_test_run_opts(prog_fd, &tattr);
> +       if (!ASSERT_OK(err, "bpf_prog_test_run"))
> +               goto cleanup;
> +
> +       if (!ASSERT_TRUE(skel->bss->ret, "ret"))
> +               goto cleanup;
> +
> +       if (!ASSERT_GT(tattr.duration, 0, "duration"))
> +               goto cleanup;
> +cleanup:
> +       kprobe_ctx__destroy(skel);
> +#endif
> +}
> diff --git a/tools/testing/selftests/bpf/progs/kprobe_ctx.c b/tools/testing/selftests/bpf/progs/kprobe_ctx.c
> new file mode 100644
> index 000000000000..98063c549930
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/kprobe_ctx.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +volatile void *expected_p1;
> +volatile void *expected_p2;
> +volatile void *expected_p3;
> +volatile void *expected_p4;
> +volatile void *expected_p5;
> +volatile bool ret = false;
> +
> +SEC("kprobe/this_function_does_not_exist")
> +int prog(struct pt_regs *ctx)
> +{
> +       void *p1, *p2, *p3, *p4, *p5;
> +
> +       p1 = (void *)PT_REGS_PARM1(ctx);
> +       p2 = (void *)PT_REGS_PARM2(ctx);
> +       p3 = (void *)PT_REGS_PARM3(ctx);
> +       p4 = (void *)PT_REGS_PARM4(ctx);
> +       p5 = (void *)PT_REGS_PARM5(ctx);
> +
> +       if (p1 != expected_p1 || p2 != expected_p2 || p3 != expected_p3 ||
> +           p4 != expected_p4 || p5 != expected_p5)
> +               return 0;
> +
> +       ret = true;
> +       return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.36.1
>

Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

[Alexei Starovoitov]

On Sun, May 29, 2022 at 3:06 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
> top of being generally useful for unit testing kprobe progs, this commit
> more specifically helps solve a relability problem with bpftrace BEGIN
> and END probes.
>
> BEGIN and END probes are run exactly once at the beginning and end of a
> bpftrace tracing session, respectively. bpftrace currently implements
> the probes by creating two dummy functions and attaching the BEGIN and
> END progs, if defined, to those functions and calling the dummy
> functions as appropriate. This works pretty well most of the time except
> for when distros strip symbols from bpftrace. Every now and then this
> happens and users get confused. Having PROG_TEST_RUN support will help
> solve this issue by allowing us to directly trigger uprobes from
> userspace.
>
> Admittedly, this is a pretty specific problem and could probably be
> solved other ways. However, PROG_TEST_RUN also makes unit testing more
> convenient, especially as users start building more complex tracing
> applications. So I see this as killing two birds with one stone.

bpftrace approach of uprobe-ing into BEGIN_trigger can
be solved with raw_tp prog.
"BEGIN" bpftrace's program doesn't have to be uprobe.
I can be raw_tp and prog_test_run command is
already implemented for raw_tp.

kprobe prog has pt_regs as arguments,
raw_tp has tracepoint args.
Both progs expect kernel addresses in args.
Passing 'struct pt_regs' filled with integers and non-kernel addresses
won't help to unit test bpf-kprobe programs.
There is little use in creating a dummy kprobe-bpf prog
that expects RDI to be 1 and RSI to be 2
(like selftest from patch 2 does) and running it.
We already have raw_tp with similar args and such
progs can be executed already.
Whether SEC() part says kprobe/ or raw_tp/ doesn't change
much in the prog itself.
More so raw_tp prog will work on all architectures,
whereas kprobe's pt_regs are arch specific.
So even if kprobe progs were runnable, bpftrace
should probably be using raw_tp to be arch independent.

Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

[kernel test robot]

Hi Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: s390-randconfig-r002-20220531 (https://download.01.org/0day-ci/archive/20220601/202206011814.9lsKlbB0-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c825abd6b0198fb088d9752f556a70705bc99dfd)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/a547a4c795103fd002d3bbb5ee4d7141113716c0
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Daniel-Xu/Add-PROG_TEST_RUN-support-to-BPF_PROG_TYPE_KPROBE/20220530-060742
        git checkout a547a4c795103fd002d3bbb5ee4d7141113716c0
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> /opt/cross/gcc-11.3.0-nolibc/s390x-linux/bin/s390x-linux-ld: kernel/trace/bpf_trace.o:kernel/trace/bpf_trace.c:1365: undefined reference to `bpf_prog_test_run_kprobe'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH bpf-next 1/2] bpf, test_run: Add PROG_TEST_RUN support to kprobe

[Daniel Xu]

On Tue, May 31, 2022 at 11:07:31AM -0700, Alexei Starovoitov wrote:
> On Sun, May 29, 2022 at 3:06 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> >
> > This commit adds PROG_TEST_RUN support to BPF_PROG_TYPE_KPROBE progs. On
> > top of being generally useful for unit testing kprobe progs, this commit
> > more specifically helps solve a relability problem with bpftrace BEGIN
> > and END probes.
> >
> > BEGIN and END probes are run exactly once at the beginning and end of a
> > bpftrace tracing session, respectively. bpftrace currently implements
> > the probes by creating two dummy functions and attaching the BEGIN and
> > END progs, if defined, to those functions and calling the dummy
> > functions as appropriate. This works pretty well most of the time except
> > for when distros strip symbols from bpftrace. Every now and then this
> > happens and users get confused. Having PROG_TEST_RUN support will help
> > solve this issue by allowing us to directly trigger uprobes from
> > userspace.
> >
> > Admittedly, this is a pretty specific problem and could probably be
> > solved other ways. However, PROG_TEST_RUN also makes unit testing more
> > convenient, especially as users start building more complex tracing
> > applications. So I see this as killing two birds with one stone.
> 
> bpftrace approach of uprobe-ing into BEGIN_trigger can
> be solved with raw_tp prog.
> "BEGIN" bpftrace's program doesn't have to be uprobe.
> I can be raw_tp and prog_test_run command is
> already implemented for raw_tp.
> 
> kprobe prog has pt_regs as arguments,
> raw_tp has tracepoint args.
> Both progs expect kernel addresses in args.
> Passing 'struct pt_regs' filled with integers and non-kernel addresses
> won't help to unit test bpf-kprobe programs.
> There is little use in creating a dummy kprobe-bpf prog
> that expects RDI to be 1 and RSI to be 2
> (like selftest from patch 2 does) and running it.

Yeah that's a good point about the kprobe case. But AFAICT uprobes are
implemented using a kprobe prog in which case it would be both possible
and useful to insert userspace args and userspace addrspace addrs.

> We already have raw_tp with similar args and such
> progs can be executed already.
> Whether SEC() part says kprobe/ or raw_tp/ doesn't change
> much in the prog itself.

I suppose so, and I guess you could always bpf_program__set_type(..).

> More so raw_tp prog will work on all architectures,
> whereas kprobe's pt_regs are arch specific.
> So even if kprobe progs were runnable, bpftrace
> should probably be using raw_tp to be arch independent.

bpftrace has all the infra to pull arbitrary structs out of vmlinux BTF
already. It should be fairly simple to get the arch-specific struct
pt_regs size and construct a buffer of all 0s. And fall back to old
logic (that'll be necessary for a while) if kprobe BPF_PROG_RUN or
vmlinux BTF is missing.

That being said, I didn't realize that when I put up the patch, so
thanks for the hint. It sounds like it's probably simpler to just use
raw_tp then.

FWIW I still think this feature is useful, but since I probably won't
use it in bpftrace I'm fine with dropping the patchset. If anyone still
wants it in I'm also fine with continuing on it.

Thanks,
Daniel