Date:   Tue, 28 Feb 2023 10:31:57 +0100
From:   Jiri Olsa <jolsa@...nel.org>
To:     Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Hao Luo <haoluo@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Matthew Wilcox <willy@...radead.org>
Cc:     bpf@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-perf-users@...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>,
        Daniel Borkmann <daniel@...earbox.net>,
        Namhyung Kim <namhyung@...il.com>
Subject: [RFC v2 bpf-next 0/9] mm/bpf/perf: Store build id in inode object

hi,
this is RFC patchset for adding build id under inode's object.

The main change to previous post [1] is to use inode object instead of file
object for build id data.

However.. ;-) while using inode as build id storage place saves some memory
by keeping just one copy of the build id for all file instances, there seems
to be another problem.

The problem is that we read the build id when the file is mmap-ed.

Which is fine for our use case, because we only access build id data through
vma->vm_file->f_inode. But there are possible scenarios/windows where the
build id can be wrong when accessed in another way.

Like when the file is overwritten with another binary version with different
build id. This will result in having wrong build id data in inode until the
new file is mmap-ed.

   - file open                 > inode->i_build_id == NULL
   - file mmap
      -> read build id         > inode->i_build_id == build_id_1

   [ file changed with same inode, inode keeps old i_build_id data ]

   - file open                 > inode->i_build_id == build_id_1
   - file mmap
      -> read build id         > inode->i_build_id == build_id_2


I guess we could release i_build_id when the last file's vma go out?

But I'm not sure how to solve this and still be able to access build id
easily just by accessing the inode->i_build_id field without any lock.

I'm inclined to go back and store build id under the file object, where the
build id would be correct (or missing).

thoughts?

thanks,
jirka


v2 changes:
  - store build id under inode [Matthew Wilcox]
  - use urandom_read and liburandom_read.so for test [Andrii]
  - add libelf-based helper to fetch build ID from elf [Andrii]
  - store build id or error we got when reading it [Andrii]
  - use full name i_build_id [Andrii]
  - several tests fixes [Andrii]


[1] https://lore.kernel.org/bpf/20230201135737.800527-2-jolsa@kernel.org/
---
Jiri Olsa (9):
      mm: Store build id in inode object
      bpf: Use file's inode object build id in stackmap
      perf: Use file object build id in perf_event_mmap_event
      libbpf: Allow to resolve binary path in current directory
      selftests/bpf: Add read_buildid function
      selftests/bpf: Add err.h header
      selftests/bpf: Replace extract_build_id with read_build_id
      selftests/bpf: Add inode_build_id test
      selftests/bpf: Add iter_task_vma_buildid test

 fs/inode.c                                                       | 12 +++++++++++
 include/linux/buildid.h                                          | 15 ++++++++++++++
 include/linux/fs.h                                               |  7 +++++++
 kernel/bpf/stackmap.c                                            | 24 +++++++++++++++++++++-
 kernel/events/core.c                                             | 46 +++++++++++++++++++++++++++++++++++++----
 lib/buildid.c                                                    | 40 ++++++++++++++++++++++++++++++++++++
 mm/Kconfig                                                       |  8 ++++++++
 mm/mmap.c                                                        | 23 +++++++++++++++++++++
 tools/lib/bpf/libbpf.c                                           |  4 +++-
 tools/testing/selftests/bpf/prog_tests/bpf_iter.c                | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/prog_tests/inode_build_id.c          | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c     | 19 +++++++----------
 tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c | 17 ++++++---------
 tools/testing/selftests/bpf/progs/bpf_iter_task_vma_buildid.c    | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/err.h                          | 13 ++++++++++++
 tools/testing/selftests/bpf/progs/inode_build_id.c               | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/profiler.inc.h                 |  3 +--
 tools/testing/selftests/bpf/test_progs.c                         | 25 ----------------------
 tools/testing/selftests/bpf/test_progs.h                         | 11 +++++++++-
 tools/testing/selftests/bpf/trace_helpers.c                      | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/trace_helpers.h                      |  5 +++++
 21 files changed, 581 insertions(+), 57 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/inode_build_id.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_task_vma_buildid.c
 create mode 100644 tools/testing/selftests/bpf/progs/err.h
 create mode 100644 tools/testing/selftests/bpf/progs/inode_build_id.c

Powered by blists - more mailing lists