All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hao Luo <haoluo@google.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	Stanislav Fomichev <sdf@google.com>
Subject: Re: [PATCHv4 bpf-next 2/4] bpf: Add bpf_vma_build_id_parse function and kfunc
Date: Mon, 28 Nov 2022 22:20:33 -0800	[thread overview]
Message-ID: <CA+khW7gAYHmoUkq0UqTiZjdOqARLG256USj3uFwi6z_FyZf31w@mail.gmail.com> (raw)
In-Reply-To: <CAEf4BzaZCUoxN_X2ALXwQeFTCwtL17R4P_B_-hUCcidfyO2xyQ@mail.gmail.com>

On Mon, Nov 28, 2022 at 5:15 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Nov 28, 2022 at 5:29 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.
>
> As a completely different way of solving this problem of retrieving
> build_id for tracing needs, can we teach kernel itself to parse and
> store build_id (probably gated behind Kconfig option) in struct file
> (ideally)? On exec() and when mmap()'ing with executable permissions,
> Linux kernel will try to fetch build_id from ELF file and if
> successful store it in struct file. Given build_id can be up to 20
> bytes (currently) and not each struct file points to executable, we
> might want to only add a pointer field to `struct file` itself, which,
> if build_id is present, will point to
>
> struct build_id {
>     char sz;
>     char data[];
> };
>
> This way we don't increase `struct file` by much.
>
> And then any BPF program would be able to easily probe_read_kernel
> such build_id from vma_area_struct->vm_file->build_id?
>
> I'm sure I'm oversimplifying, but this seems like a good solution for
> all kinds of profiling BPF programs without the need to maintain all
> these allowlists and adding new helpers/kfuncs?
>
> I know Hao was looking at the problem of getting build_id, I'm curious
> if something like this would work for their use cases as well?
>

This helps a little. We would like to get build_id reliably. There are
two problems we encountered.

First, sometimes we need to get build_id from an atomic context. We
fail to get build_id if the page that contains the build_id has been
evicted from pagecache. Storing the build_id in `struct file` or
`struct inode` is a good and natural solution. But, this problem can
also be solved by using mlock to pin the page in memory. We are using
mlock, it seems to be working well right now.

The other problem we encountered may be very specific to our own use
case. Sometimes we execute code that is mapped in an anonymous page
(not backed by file). In that case, we can't get build_id either. What
we are doing right now is writing the build_id into the
vm_area_struct->anon_name field and teach build_id_parse to try
parsing from there, when seeing an anonymous page. I can send this
with upstream if there are other users who have the same problem.

>
> >
> > 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.
> >
> > The bpf_vma_build_id_parse kfunc is marked as KF_TRUSTED_ARGS,
> > so it can be only called with trusted vma objects. These are
> > currently provided only by find_vma callback function and
> > task_vma iterator program.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/linux/bpf.h      |  4 ++++
> >  kernel/trace/bpf_trace.c | 31 +++++++++++++++++++++++++++++++
> >  2 files changed, 35 insertions(+)
> >
>
> [...]

  reply	other threads:[~2022-11-29  6:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 13:29 [PATCHv4 bpf-next 0/4] bpf: Add bpf_vma_build_id_parse kfunc Jiri Olsa
2022-11-28 13:29 ` [PATCHv4 bpf-next 1/4] bpf: Mark vma objects as trusted for task_vma iter and find_vma callback Jiri Olsa
2022-11-28 18:43   ` Alexei Starovoitov
2022-11-28 19:04     ` Yonghong Song
2022-11-28 20:25       ` Alexei Starovoitov
2022-11-28 13:29 ` [PATCHv4 bpf-next 2/4] bpf: Add bpf_vma_build_id_parse function and kfunc Jiri Olsa
2022-11-28 21:36   ` Alexei Starovoitov
2022-11-29  8:45     ` Jiri Olsa
2022-11-29  1:15   ` Andrii Nakryiko
2022-11-29  6:20     ` Hao Luo [this message]
2022-11-30  0:35       ` Andrii Nakryiko
2022-11-30  1:27         ` Hao Luo
2022-12-01  1:11           ` Andrii Nakryiko
2022-11-29  8:52     ` Jiri Olsa
2022-11-28 13:29 ` [PATCHv4 bpf-next 3/4] selftests/bpf: Add bpf_vma_build_id_parse find_vma callback test Jiri Olsa
2022-11-28 19:25   ` Hao Luo
2022-11-29  8:32     ` Jiri Olsa
2022-11-28 13:29 ` [PATCHv4 bpf-next 4/4] selftests/bpf: Add bpf_vma_build_id_parse task vma iterator test Jiri Olsa
2022-11-28 19:21   ` Hao Luo
2022-11-29  8:29     ` Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+khW7gAYHmoUkq0UqTiZjdOqARLG256USj3uFwi6z_FyZf31w@mail.gmail.com \
    --to=haoluo@google.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=sdf@google.com \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.