BPF List
 help / color / mirror / Atom feed
* [PATCHv2 bpf-next 0/2] bpf: Add bpf_vma_build_id_parse kfunc
@ 2022-11-11 14:33 Jiri Olsa
  2022-11-11 14:33 ` [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc Jiri Olsa
  2022-11-11 14:33 ` [PATCHv2 bpf-next 2/2] selftests/bpf: Add bpf_vma_build_id_parse kfunc test Jiri Olsa
  0 siblings, 2 replies; 6+ messages in thread
From: Jiri Olsa @ 2022-11-11 14:33 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo

hi,
first version of this patchset added helper for this functionality,
but based Alexei's feedback [1], changing it to kfunc.

With the current build_id_parse function as kfunc we can't effectively
check buffer size provided by user. Therefore adding new function as
bpf kfunc:

  int bpf_vma_build_id_parse(struct vm_area_struct *vma,
                             unsigned char *build_id,
                             size_t build_id__sz);

that triggers kfunc's verifier check for build_id/build_id__sz buffer
size and calls build_id_parse.

thanks,
jirka


[1] https://lore.kernel.org/bpf/CAADnVQKyT4Mm4EdTCYK8c070E-BwPZS_FOkWKLJC80riSGmLTg@mail.gmail.com/
---
Jiri Olsa (2):
      bpf: Add bpf_vma_build_id_parse function and kfunc
      selftests/bpf: Add bpf_vma_build_id_parse kfunc test

 include/linux/bpf.h                                             |  5 +++++
 kernel/bpf/helpers.c                                            | 16 ++++++++++++++++
 tools/testing/selftests/bpf/prog_tests/bpf_vma_build_id_parse.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/bpf_vma_build_id_parse.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 149 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_vma_build_id_parse.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_vma_build_id_parse.c

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc
  2022-11-11 14:33 [PATCHv2 bpf-next 0/2] bpf: Add bpf_vma_build_id_parse kfunc Jiri Olsa
@ 2022-11-11 14:33 ` Jiri Olsa
  2022-11-14 17:40   ` Alexei Starovoitov
  2022-11-11 14:33 ` [PATCHv2 bpf-next 2/2] selftests/bpf: Add bpf_vma_build_id_parse kfunc test Jiri Olsa
  1 sibling, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2022-11-11 14:33 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo

Adding bpf_vma_build_id_parse function to retrieve build id from
passed vma object and making it available as bpf kfunc.

We can't use build_id_parse directly as kfunc, because we would
not have control over the build id buffer size provided by user.

Instead we are adding new bpf_vma_build_id_parse function with
'build_id__sz' argument that instructs verifier to check for the
available space in build_id buffer.

This way  we check that there's  always available memory space
behind build_id pointer. We also check that the build_id__sz is
at least BUILD_ID_SIZE_MAX so we can place any buildid in.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/bpf.h  |  5 +++++
 kernel/bpf/helpers.c | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 798aec816970..5e7c4c50da8e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2779,4 +2779,9 @@ struct bpf_key {
 	bool has_ref;
 };
 #endif /* CONFIG_KEYS */
+
+extern int bpf_vma_build_id_parse(struct vm_area_struct *vma,
+				  unsigned char *build_id,
+				  size_t build_id__sz);
+
 #endif /* _LINUX_BPF_H */
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 283f55bbeb70..af7a30dafff3 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -19,6 +19,7 @@
 #include <linux/proc_ns.h>
 #include <linux/security.h>
 #include <linux/btf_ids.h>
+#include <linux/buildid.h>
 
 #include "../../lib/kstrtox.h"
 
@@ -1706,10 +1707,25 @@ bpf_base_func_proto(enum bpf_func_id func_id)
 	}
 }
 
+int bpf_vma_build_id_parse(struct vm_area_struct *vma,
+			   unsigned char *build_id,
+			   size_t build_id__sz)
+{
+	__u32 size;
+	int err;
+
+	if (build_id__sz < BUILD_ID_SIZE_MAX)
+		return -EINVAL;
+
+	err = build_id_parse(vma, build_id, &size);
+	return err ?: (int) size;
+}
+
 BTF_SET8_START(tracing_btf_ids)
 #ifdef CONFIG_KEXEC_CORE
 BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
 #endif
+BTF_ID_FLAGS(func, bpf_vma_build_id_parse)
 BTF_SET8_END(tracing_btf_ids)
 
 static const struct btf_kfunc_id_set tracing_kfunc_set = {
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 bpf-next 2/2] selftests/bpf: Add bpf_vma_build_id_parse kfunc test
  2022-11-11 14:33 [PATCHv2 bpf-next 0/2] bpf: Add bpf_vma_build_id_parse kfunc Jiri Olsa
  2022-11-11 14:33 ` [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc Jiri Olsa
@ 2022-11-11 14:33 ` Jiri Olsa
  1 sibling, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2022-11-11 14:33 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo

Adding test for bpf_vma_build_id_parse kfunc.

On bpf side the test finds the vma of the test_progs text through the
test function pointer and reads its build id with the new kfunc.

On user side the test uses readelf to get test_progs build id and
compares it with the one from bpf side.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../bpf/prog_tests/bpf_vma_build_id_parse.c   | 88 +++++++++++++++++++
 .../bpf/progs/bpf_vma_build_id_parse.c        | 40 +++++++++
 2 files changed, 128 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_vma_build_id_parse.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_vma_build_id_parse.c

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_vma_build_id_parse.c b/tools/testing/selftests/bpf/prog_tests/bpf_vma_build_id_parse.c
new file mode 100644
index 000000000000..83030a3b2c42
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_vma_build_id_parse.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include "bpf_vma_build_id_parse.skel.h"
+
+#define BUILDID_STR_SIZE (BPF_BUILD_ID_SIZE*2 + 1)
+
+static int read_buildid(char **build_id)
+{
+	char tmp[] = "/tmp/dataXXXXXX";
+	char buf[200];
+	int err, fd;
+	FILE *f;
+
+	fd = mkstemp(tmp);
+	if (fd == -1)
+		return -1;
+	close(fd);
+
+	snprintf(buf, sizeof(buf),
+		"readelf -n ./test_progs 2>/dev/null | grep 'Build ID' | awk '{print $3}' > %s",
+		tmp);
+
+	err = system(buf);
+	if (err)
+		goto out;
+
+	f = fopen(tmp, "r");
+	if (f) {
+		if (fscanf(f, "%ms$*\n", build_id) != 1) {
+			*build_id = NULL;
+			err = -1;
+		}
+		fclose(f);
+	}
+
+out:
+	unlink(tmp);
+	return err;
+}
+
+void test_bpf_vma_build_id_parse(void)
+{
+	char bpf_build_id[BUILDID_STR_SIZE] = {}, *build_id;
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct bpf_vma_build_id_parse *skel;
+	int i, err, prog_fd;
+
+	skel = bpf_vma_build_id_parse__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "bpf_vma_build_id_parse__open_and_load"))
+		return;
+
+	skel->bss->target_pid = getpid();
+	skel->bss->addr = (__u64)(uintptr_t)test_bpf_vma_build_id_parse;
+
+	err = bpf_vma_build_id_parse__attach(skel);
+	if (!ASSERT_OK(err, "bpf_vma_build_id_parse__attach"))
+		goto out;
+
+	prog_fd = bpf_program__fd(skel->progs.test1);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "test_run_err");
+	ASSERT_EQ(topts.retval, 0, "test_run_retval");
+
+	ASSERT_EQ(skel->data->ret, 0, "ret");
+
+	ASSERT_GT(skel->data->size_pass, 0, "size_pass");
+	ASSERT_EQ(skel->data->size_fail, -EINVAL, "size_fail");
+
+	/* Read build id via readelf to compare with build_id. */
+	if (!ASSERT_OK(read_buildid(&build_id), "read_buildid"))
+		goto out;
+
+	ASSERT_EQ(skel->data->size_pass, strlen(build_id)/2, "build_id_size");
+
+	/* Convert bpf build id to string, so we can compare it later. */
+	for (i = 0; i < skel->data->size_pass; i++) {
+		sprintf(bpf_build_id + i*2, "%02x",
+			(unsigned char) skel->bss->build_id[i]);
+	}
+	ASSERT_STREQ(bpf_build_id, build_id, "build_id_match");
+
+	free(build_id);
+out:
+	bpf_vma_build_id_parse__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/bpf_vma_build_id_parse.c b/tools/testing/selftests/bpf/progs/bpf_vma_build_id_parse.c
new file mode 100644
index 000000000000..8937212207db
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_vma_build_id_parse.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+#define BPF_BUILD_ID_SIZE 20
+
+extern int bpf_vma_build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
+				  size_t build_id__sz) __ksym;
+
+pid_t target_pid = 0;
+__u64 addr = 0;
+
+int ret = -1;
+int size_pass = -1;
+int size_fail = -1;
+
+unsigned char build_id[BPF_BUILD_ID_SIZE];
+
+static long check_vma(struct task_struct *task, struct vm_area_struct *vma,
+		      void *data)
+{
+	size_fail = bpf_vma_build_id_parse(vma, build_id, sizeof(build_id)/2);
+	size_pass = bpf_vma_build_id_parse(vma, build_id, sizeof(build_id));
+	return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(test1, int a)
+{
+	struct task_struct *task = bpf_get_current_task_btf();
+
+	if (task->pid != target_pid)
+		return 0;
+
+	ret = bpf_find_vma(task, addr, check_vma, NULL, 0);
+	return 0;
+}
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc
  2022-11-11 14:33 ` [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc Jiri Olsa
@ 2022-11-14 17:40   ` Alexei Starovoitov
  2022-11-14 18:42     ` Song Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2022-11-14 17:40 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo

On Fri, Nov 11, 2022 at 6:33 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding bpf_vma_build_id_parse function to retrieve build id from
> passed vma object and making it available as bpf kfunc.
>
> We can't use build_id_parse directly as kfunc, because we would
> not have control over the build id buffer size provided by user.
>
> Instead we are adding new bpf_vma_build_id_parse function with
> 'build_id__sz' argument that instructs verifier to check for the
> available space in build_id buffer.
>
> This way  we check that there's  always available memory space
> behind build_id pointer. We also check that the build_id__sz is
> at least BUILD_ID_SIZE_MAX so we can place any buildid in.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/linux/bpf.h  |  5 +++++
>  kernel/bpf/helpers.c | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 798aec816970..5e7c4c50da8e 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2779,4 +2779,9 @@ struct bpf_key {
>         bool has_ref;
>  };
>  #endif /* CONFIG_KEYS */
> +
> +extern int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> +                                 unsigned char *build_id,
> +                                 size_t build_id__sz);
> +
>  #endif /* _LINUX_BPF_H */
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 283f55bbeb70..af7a30dafff3 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -19,6 +19,7 @@
>  #include <linux/proc_ns.h>
>  #include <linux/security.h>
>  #include <linux/btf_ids.h>
> +#include <linux/buildid.h>
>
>  #include "../../lib/kstrtox.h"
>
> @@ -1706,10 +1707,25 @@ bpf_base_func_proto(enum bpf_func_id func_id)
>         }
>  }
>
> +int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> +                          unsigned char *build_id,
> +                          size_t build_id__sz)
> +{
> +       __u32 size;
> +       int err;
> +
> +       if (build_id__sz < BUILD_ID_SIZE_MAX)
> +               return -EINVAL;
> +
> +       err = build_id_parse(vma, build_id, &size);
> +       return err ?: (int) size;
> +}

And you'll allow any tracing prog to call it like this?
Feels obviously unsafe unless I'm missing something big here.
See the amount of safety checks that
stack_map_get_build_id_offset() does.
Why can we get away without them here?

The use case is not clear to me as well.
Do you alwasy expect to call this kfunc from bpf_find_vma callback ?


> +
>  BTF_SET8_START(tracing_btf_ids)
>  #ifdef CONFIG_KEXEC_CORE
>  BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
>  #endif
> +BTF_ID_FLAGS(func, bpf_vma_build_id_parse)
>  BTF_SET8_END(tracing_btf_ids)
>
>  static const struct btf_kfunc_id_set tracing_kfunc_set = {
> --
> 2.38.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc
  2022-11-14 17:40   ` Alexei Starovoitov
@ 2022-11-14 18:42     ` Song Liu
  2022-11-14 21:47       ` Jiri Olsa
  0 siblings, 1 reply; 6+ messages in thread
From: Song Liu @ 2022-11-14 18:42 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo

On Mon, Nov 14, 2022 at 9:41 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Nov 11, 2022 at 6:33 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding bpf_vma_build_id_parse function to retrieve build id from
> > passed vma object and making it available as bpf kfunc.
> >
> > We can't use build_id_parse directly as kfunc, because we would
> > not have control over the build id buffer size provided by user.
> >
> > Instead we are adding new bpf_vma_build_id_parse function with
> > 'build_id__sz' argument that instructs verifier to check for the
> > available space in build_id buffer.
> >
> > This way  we check that there's  always available memory space
> > behind build_id pointer. We also check that the build_id__sz is
> > at least BUILD_ID_SIZE_MAX so we can place any buildid in.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/linux/bpf.h  |  5 +++++
> >  kernel/bpf/helpers.c | 16 ++++++++++++++++
> >  2 files changed, 21 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 798aec816970..5e7c4c50da8e 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -2779,4 +2779,9 @@ struct bpf_key {
> >         bool has_ref;
> >  };
> >  #endif /* CONFIG_KEYS */
> > +
> > +extern int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> > +                                 unsigned char *build_id,
> > +                                 size_t build_id__sz);
> > +
> >  #endif /* _LINUX_BPF_H */
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index 283f55bbeb70..af7a30dafff3 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/proc_ns.h>
> >  #include <linux/security.h>
> >  #include <linux/btf_ids.h>
> > +#include <linux/buildid.h>
> >
> >  #include "../../lib/kstrtox.h"
> >
> > @@ -1706,10 +1707,25 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> >         }
> >  }
> >
> > +int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> > +                          unsigned char *build_id,
> > +                          size_t build_id__sz)
> > +{
> > +       __u32 size;
> > +       int err;
> > +
> > +       if (build_id__sz < BUILD_ID_SIZE_MAX)
> > +               return -EINVAL;
> > +
> > +       err = build_id_parse(vma, build_id, &size);
> > +       return err ?: (int) size;
> > +}
>
> And you'll allow any tracing prog to call it like this?
> Feels obviously unsafe unless I'm missing something big here.
> See the amount of safety checks that
> stack_map_get_build_id_offset() does.
> Why can we get away without them here?
>
> The use case is not clear to me as well.
> Do you alwasy expect to call this kfunc from bpf_find_vma callback ?

It is also safe to call it from iter/task_vma programs. Some allow list
seems necessary here.

Thanks,
Song

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc
  2022-11-14 18:42     ` Song Liu
@ 2022-11-14 21:47       ` Jiri Olsa
  0 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2022-11-14 21:47 UTC (permalink / raw)
  To: Song Liu
  Cc: Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo

On Mon, Nov 14, 2022 at 10:42:15AM -0800, Song Liu wrote:
> On Mon, Nov 14, 2022 at 9:41 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Nov 11, 2022 at 6:33 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding bpf_vma_build_id_parse function to retrieve build id from
> > > passed vma object and making it available as bpf kfunc.
> > >
> > > We can't use build_id_parse directly as kfunc, because we would
> > > not have control over the build id buffer size provided by user.
> > >
> > > Instead we are adding new bpf_vma_build_id_parse function with
> > > 'build_id__sz' argument that instructs verifier to check for the
> > > available space in build_id buffer.
> > >
> > > This way  we check that there's  always available memory space
> > > behind build_id pointer. We also check that the build_id__sz is
> > > at least BUILD_ID_SIZE_MAX so we can place any buildid in.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  include/linux/bpf.h  |  5 +++++
> > >  kernel/bpf/helpers.c | 16 ++++++++++++++++
> > >  2 files changed, 21 insertions(+)
> > >
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index 798aec816970..5e7c4c50da8e 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -2779,4 +2779,9 @@ struct bpf_key {
> > >         bool has_ref;
> > >  };
> > >  #endif /* CONFIG_KEYS */
> > > +
> > > +extern int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> > > +                                 unsigned char *build_id,
> > > +                                 size_t build_id__sz);
> > > +
> > >  #endif /* _LINUX_BPF_H */
> > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > > index 283f55bbeb70..af7a30dafff3 100644
> > > --- a/kernel/bpf/helpers.c
> > > +++ b/kernel/bpf/helpers.c
> > > @@ -19,6 +19,7 @@
> > >  #include <linux/proc_ns.h>
> > >  #include <linux/security.h>
> > >  #include <linux/btf_ids.h>
> > > +#include <linux/buildid.h>
> > >
> > >  #include "../../lib/kstrtox.h"
> > >
> > > @@ -1706,10 +1707,25 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> > >         }
> > >  }
> > >
> > > +int bpf_vma_build_id_parse(struct vm_area_struct *vma,
> > > +                          unsigned char *build_id,
> > > +                          size_t build_id__sz)
> > > +{
> > > +       __u32 size;
> > > +       int err;
> > > +
> > > +       if (build_id__sz < BUILD_ID_SIZE_MAX)
> > > +               return -EINVAL;
> > > +
> > > +       err = build_id_parse(vma, build_id, &size);
> > > +       return err ?: (int) size;
> > > +}
> >
> > And you'll allow any tracing prog to call it like this?
> > Feels obviously unsafe unless I'm missing something big here.
> > See the amount of safety checks that
> > stack_map_get_build_id_offset() does.
> > Why can we get away without them here?
> >

ugh right.. I use it always from bpf_find_vma callback, that's probably
why I did not realize that, because it's already locked there

> > The use case is not clear to me as well.
> > Do you alwasy expect to call this kfunc from bpf_find_vma callback ?
> 
> It is also safe to call it from iter/task_vma programs. Some allow list
> seems necessary here.

I have 2 places I call it from: sched_process_exec tp_btf and process iterator
both through bpf_find_vma callback.. so I think we can allow it only from
bpf_find_vma callback.. that should keep it simple for start

thanks,
jirka

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-14 21:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-11 14:33 [PATCHv2 bpf-next 0/2] bpf: Add bpf_vma_build_id_parse kfunc Jiri Olsa
2022-11-11 14:33 ` [PATCHv2 bpf-next 1/2] bpf: Add bpf_vma_build_id_parse function and kfunc Jiri Olsa
2022-11-14 17:40   ` Alexei Starovoitov
2022-11-14 18:42     ` Song Liu
2022-11-14 21:47       ` Jiri Olsa
2022-11-11 14:33 ` [PATCHv2 bpf-next 2/2] selftests/bpf: Add bpf_vma_build_id_parse kfunc test Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox