On 3/8/23 04:58, Andrii Nakryiko wrote:
On Tue, Feb 28, 2023 at 4:27 AM Viktor Malik <vmalik@xxxxxxxxxx> wrote:
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.
is it expected that your newly added test fails on arm64? See [0]
[0] https://github.com/kernel-patches/bpf/actions/runs/4359596129/jobs/7621687719
I believe so, the test uses fentry and all fentry/fexit tests are
failing on arm64 with the same error (524) and are disabled in the CI.
---
Changes in v9:
- two small changes suggested by Jiri Olsa and Jiri's ack
Changes in v8:
- added module_put to error paths in bpf_check_attach_target after the
module reference is acquired
Changes in v7:
- refactored the module reference manipulation (comments by Jiri Olsa)
- cleaned up the test (comments by Andrii Nakryiko)
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 | 2 +
kernel/bpf/syscall.c | 6 +
kernel/bpf/trampoline.c | 28 ----
kernel/bpf/verifier.c | 18 ++-
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 | 128 ++++++++++++++++++
8 files changed, 169 insertions(+), 29 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/module_attach_shadow.c
--
2.39.1
