bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Viktor Malik <vmalik@redhat.com>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Viktor Malik <vmalik@redhat.com>
Subject: [PATCH bpf-next v6 0/2] Fix attaching fentry/fexit/fmod_ret/lsm to modules
Date: Thu, 16 Feb 2023 11:32:40 +0100	[thread overview]
Message-ID: <cover.1676542796.git.vmalik@redhat.com> (raw)

Context: I noticed that the verifier behaves incorrectly when attaching
to fentry of multiple functions of the same name located in different
modules (or in vmlinux). The reason for this is that if the target
program is not specified, the verifier will search kallsyms for the
trampoline address to attach to. The entire kallsyms is always searched,
not respecting the module in which the function to attach to is located.

As Yonghong correctly pointed out, there is yet another issue - the
trampoline acquires the module reference in register_fentry which means
that if the module is unloaded between the place where the address is
found in the verifier and register_fentry, it is possible that another
module is loaded to the same address in the meantime, which may lead to
errors.

This patch fixes the above issues by extracting the module name from the
BTF of the attachment target (which must be specified) and by doing the
search in kallsyms of the correct module. At the same time, the module
reference is acquired right after the address is found and only released
right before the program itself is unloaded.

---
Changes in v6:
- storing the module reference inside bpf_prog_aux instead of
  bpf_trampoline and releasing it when the program is unloaded
  (suggested by Jiri Olsa)

Changes in v5:
- fixed acquiring and releasing of module references by trampolines to
  prevent modules being unloaded between address lookup and trampoline
  allocation

Changes in v4:
- reworked module kallsyms lookup approach using existing functions,
  verifier now calls btf_try_get_module to retrieve the module and
  find_kallsyms_symbol_value to get the symbol address (suggested by
  Alexei)
- included Jiri Olsa's comments
- improved description of the new test and added it as a comment into
  the test source

Changes in v3:
- added trivial implementation for kallsyms_lookup_name_in_module() for
  !CONFIG_MODULES (noticed by test robot, fix suggested by Hao Luo)

Changes in v2:
- introduced and used more space-efficient kallsyms lookup function,
  suggested by Jiri Olsa
- included Hao Luo's comments

Viktor Malik (2):
  bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules
  bpf/selftests: Test fentry attachment to shadowed functions

 include/linux/bpf.h                           |   1 +
 kernel/bpf/syscall.c                          |   2 +
 kernel/bpf/trampoline.c                       |  27 ----
 kernel/bpf/verifier.c                         |  20 ++-
 kernel/module/internal.h                      |   5 +
 net/bpf/test_run.c                            |   5 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |   6 +
 .../bpf/prog_tests/module_attach_shadow.c     | 131 ++++++++++++++++++
 8 files changed, 169 insertions(+), 28 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/module_attach_shadow.c

-- 
2.39.1


             reply	other threads:[~2023-02-16 10:33 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-16 10:32 Viktor Malik [this message]
2023-02-16 10:32 ` [PATCH bpf-next v6 1/2] bpf: Fix attaching fentry/fexit/fmod_ret/lsm to modules Viktor Malik
2023-02-16 13:50   ` Jiri Olsa
2023-02-16 14:45     ` Viktor Malik
2023-02-16 15:50       ` Jiri Olsa
2023-03-22  9:49         ` Artem Savkov
2023-03-22 12:14           ` Jiri Olsa
2023-03-22 16:03             ` Alexei Starovoitov
2023-03-23 14:00               ` Jiri Olsa
2023-03-30  7:29                 ` Jiri Olsa
2023-03-30 12:26                   ` Leizhen (ThunderTown)
2023-03-30 20:59                     ` Jiri Olsa
2023-03-31  8:31                       ` Petr Mladek
2023-03-31  9:15                         ` Leizhen (ThunderTown)
2023-03-31 11:08                           ` Petr Mladek
2023-03-31 21:25                             ` Jiri Olsa
2023-04-03  1:46                               ` Leizhen (ThunderTown)
2023-04-03  8:46                                 ` Petr Mladek
2023-02-16 10:32 ` [PATCH bpf-next v6 2/2] bpf/selftests: Test fentry attachment to shadowed functions Viktor Malik
2023-02-16 23:55   ` Andrii Nakryiko

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=cover.1676542796.git.vmalik@redhat.com \
    --to=vmalik@redhat.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mcgrof@kernel.org \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).