On 7/18/22 00:43, Jiri Olsa wrote:
On Fri, Jul 08, 2022 at 12:16:35AM +0200, Jiri Olsa wrote:
On Tue, Jul 05, 2022 at 10:29:17PM -0700, Andrii Nakryiko wrote:
On Tue, Jul 5, 2022 at 12:04 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
The kprobe can be placed anywhere and user must be aware
of the underlying instructions. Therefore fixing just
the bpf program to 'fix' the address to match the actual
function address when CONFIG_X86_KERNEL_IBT is enabled.
Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
---
tools/testing/selftests/bpf/progs/get_func_ip_test.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c
index a587aeca5ae0..220d56b7c1dc 100644
--- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c
+++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c
@@ -2,6 +2,7 @@
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
+#include <stdbool.h>
char _license[] SEC("license") = "GPL";
@@ -13,6 +14,8 @@ extern const void bpf_modify_return_test __ksym;
extern const void bpf_fentry_test6 __ksym;
extern const void bpf_fentry_test7 __ksym;
+extern bool CONFIG_X86_KERNEL_IBT __kconfig __weak;
+
__u64 test1_result = 0;
SEC("fentry/bpf_fentry_test1")
int BPF_PROG(test1, int a)
@@ -37,7 +40,7 @@ __u64 test3_result = 0;
SEC("kprobe/bpf_fentry_test3")
int test3(struct pt_regs *ctx)
{
- __u64 addr = bpf_get_func_ip(ctx);
+ __u64 addr = bpf_get_func_ip(ctx) - (CONFIG_X86_KERNEL_IBT ? 4 : 0);
so for kprobe bpf_get_func_ip() gets an address with 5 byte
compensation for `call __fentry__`, but not for endr? Why can't we
compensate for endbr inside the kernel code as well? I'd imagine we
either do no compensation (and thus we get &bpf_fentry_test3+5 or
&bpf_fentry_test3+9, depending on CONFIG_X86_KERNEL_IBT) or full
compensation (and thus always get &bpf_fentry_test3), but this
in-between solution seems to be the worst of both worlds?...
hm rigth, I guess we should be able to do that in bpf_get_func_ip,
I'll check
sorry for late follow up..
so the problem is that you can place kprobe anywhere in the function
(on instruction boundary) but the IBT adjustment of kprobe address is
made only if it's at the function entry and there's endbr instruction
To add more fun to the issue, not all non-inlined functions get endbr64.
For example "skb_release_head_state()" does, while "skb_free_head()"
doesn't.
and that kprobe address is what we return in helper:
BPF_CALL_1(bpf_get_func_ip_kprobe, struct pt_regs *, regs)
{
struct kprobe *kp = kprobe_running();
return kp ? (uintptr_t)kp->addr : 0;
}
so the adjustment would work only for address at function entry, but
would be wrong for address within the function
perhaps we could add flag to kprobe to indicate the addr adjustment
was done and use it in helper
but that's why I thought I'd keep bpf_get_func_ip_kprobe as it and
leave it up to user
kprobe_multi and trampolines are different, because they can be
only at the function entry, so we can adjust the ip properly
jirka
