Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce two enhancements for func IP #468

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Asphaltt
Copy link
Contributor

@Asphaltt Asphaltt commented Dec 14, 2024

I encountered the following error days ago.

failed to trace TC progs: failed to trace bpf progs: failed to find address for function pod_XXX of bpf prog SchedCLS(pod_XXX)#108

Then, I realize that we have to improve getting func IP when trace bpf progs.

With commit 55bdaac ("Fix tracing tc-bpf with --filter-track-skb-by-stackid") and my blog Debug a tailcall BUG with fentry, I think it's able to get tracee IP when trace bpf progs.

Then, let me introduce our own get_func_ip() helper for tracing to get tracee IP.

Next, in 'output.go', it's incorrect to get func name when output in JSON format.

Why not correct func IP in bpf in order to make sure event.addr is correct always?

To achieve it, let us do - 1 for kprobe and - 4 for endbr in 'kprobe_pwru.c'. As a result, introduce get_addr() function for kprobe, kprobe-multi, fentry and fexit.

BTW, clean some Go code that was used for tracing bpf progs.


  • get_func_ip() must be compatible on arm64

Test on arm64:

# ./pwru --backend kprobe --filter-trace-tc --filter-trace-xdp --output-meta --output-tuple --filter-func '.*icmp.*' icmp
0xffff80008000b938 1   ping:363         4026531840 0              lo:1       0x0000 65536 98    [0x00000000,0x00000000,0x00000000,0x00000000,0x00000000] 127.0.0.1:0->127.0.0.1:0(icmp) bpf_prog_3b185187f1855c4c_dummy[bpf](xdp)
0xffff0000c1ee5200 1   <empty>:363      4026531840 0              lo:1       0x0800 65536 64    [0x00000000,0x00000000,0x00000000,0x00000100,0x00000000] 127.0.0.1:0->127.0.0.1:0(icmp) icmp_rcv
0xffff0000c1ee5200 1   <empty>:363      4026531840 0              lo:1       0x0800 65536 56    [0x00000000,0x00000000,0x00000000,0x00000100,0x00000000] 127.0.0.1:0->127.0.0.1:0(icmp) icmp_echo
0xffff0000c1ee5200 1   <empty>:363      4026531840 0              lo:1       0x0800 65536 56    [0x00000000,0x00000000,0x00000000,0x00000100,0x00000000] 127.0.0.1:0->127.0.0.1:0(icmp) icmp_reply
0xffff80008000b938 1   <empty>:363      4026531840 0              lo:1       0x0000 65536 98    [0x00000000,0x00000000,0x00000000,0x00000000,0x00000000] 127.0.0.1:0->127.0.0.1:0(icmp) bpf_prog_3b185187f1855c4c_dummy[bpf](xdp)
0xffff0000c1ee5300 1   <empty>:363      4026531840 0              lo:1       0x0800 65536 64    [0x00000000,0x00000000,0x00000000,0x00000100,0x00000000] 127.0.0.1:0->127.0.0.1:0(icmp) icmp_rcv

Test on amd64:

# ./pwru --backend kprobe --filter-trace-tc --filter-trace-xdp --output-meta --output-tuple --filter-func '.*icmp.*' icmp
0xffffaab500600b00 6   <empty>:0        4026531840 0            ens33:2      0x0000 1500  98    [0x00000000,0x00000000,0x00000000,0x00000000,0x00000000] 192.168.241.1:0->192.168.241.133:0(icmp) bpf_prog_3b185187f1855c4c_dummy[bpf](xdp)
0xffff9097a49d1a00 6   <empty>:0        4026531840 0            ens33:2      0x0800 65536 64    [0x00000000,0x00000000,0x00000000,0x00000000,0x00000000] 192.168.241.1:0->192.168.241.133:0(icmp) icmp_rcv
0xffff9097a49d1a00 6   <empty>:0        4026531840 0            ens33:2      0x0800 65536 56    [0x00000000,0x00000000,0x00000000,0x00000000,0x00000000] 192.168.241.1:0->192.168.241.133:0(icmp) icmp_echo
0xffff9097a49d1a00 6   <empty>:0        4026531840 0            ens33:2      0x0800 65536 56    [0x00000000,0x00000000,0x00000000,0x00000000,0x00000000] 192.168.241.1:0->192.168.241.133:0(icmp) icmp_reply

@Asphaltt Asphaltt marked this pull request as ready for review December 14, 2024 09:15
@Asphaltt Asphaltt requested a review from a team as a code owner December 14, 2024 09:15
@Asphaltt Asphaltt requested review from brb and removed request for a team December 14, 2024 09:15
@Asphaltt Asphaltt marked this pull request as draft December 14, 2024 09:56
Copy link
Member

@jschwinger233 jschwinger233 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment regarding platform specific bpf code, other than that I love the way you fetched rip in fentry.

Some minor(?) concerns:

  1. I fully understand the context because I read Chinese and we talked about this in private. Martynas (brb) please feel free to ask for more details if commit messages don't elaborate enough.
  2. Lack of test is constantly hurting us. I'm not 100% confident in whether this PR won't break anything, considering we already suffered certain symbol-address mapping issue pwru fails to parse helper symbols when --filter-track-bpf-helpers #462. Again, this is not the problem of this PR's author, I'm just saying we (pwru maintainers) might want to prioritize some tasks like "pwru hardening"

bpf/kprobe_pwru.c Outdated Show resolved Hide resolved
@Asphaltt Asphaltt force-pushed the feat/get_func_ip branch 4 times, most recently from 2c6c2d8 to ba8ccdc Compare December 15, 2024 15:10
Since commit 55bdaac ("Fix tracing tc-bpf with --filter-track-skb-by-stackid"),
we are able to get FP of current bpf prog.

Then, from blog [Debug a tailcall BUG with fentry](https://asphaltt.github.io/post/ebpf-talk-138-debug-tailcall-bug-with-fentry/),
we are able to get IP of tracee bpf prog like the way of
`bpf_get_func_ip()` helper.

Therefore, implement our own `get_func_ip()` helper to get IP of tracee
bpf prog.

Signed-off-by: Leon Hwang <[email protected]>
As we're able to get IP of tracee bpf prog at runtime, remove those code
related to name2progName preparation.

Signed-off-by: Leon Hwang <[email protected]>
It's better to correct func IP in bpf then in user space, because in bpf
it is able to distinguish every case, include endbr case.

As a result, it gets `addr` by this `get_addr()` function for **kprobe**,
**kprobe-multi**, **fentry** and **fexit**.

Signed-off-by: Leon Hwang <[email protected]>
On arm64, R10 of bpf is not FP, aka A64_FP register.

It should reuse `detect_tramp_fp()` function to get a valid FP.

Signed-off-by: Leon Hwang <[email protected]>
@Asphaltt Asphaltt marked this pull request as ready for review December 16, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants