mbox series

[v2,0/4] Fix user stack traces captured from uprobes

Message ID 20240522013845.1631305-1-andrii@kernel.org (mailing list archive)
Headers show
Series Fix user stack traces captured from uprobes | expand

Message

Andrii Nakryiko May 22, 2024, 1:38 a.m. UTC
This patch set reports two issues with captured stack traces.

First issue, fixed in patch #2, deals with fixing up uretprobe trampoline
addresses in captured stack trace. This issue happens when there are pending
return probes, for which kernel hijacks some of the return addresses on user
stacks. The code is matching those special uretprobe trampoline addresses with
the list of pending return probe instances and replaces them with actual
return addresses. This is the same fixup logic that fprobe/kretprobe has for
kernel stack traces.

Second issue, which patch #3 is fixing with the help of heuristic, is having
to do with capturing user stack traces in entry uprobes. At the very entrance
to user function, frame pointer in rbp register is not yet setup, so actual
caller return address is still pointed to by rsp. Patch is using a simple
heuristic, looking for `push %rbp` instruction, to fetch this extra direct
caller return address, before proceeding to unwind the stack using rbp.

Patch #4 adds tests into BPF selftests, that validate that captured stack
traces at various points is what we expect to get. This patch, while being BPF
selftests, is isolated from any other BPF selftests changes and can go in
through non-BPF tree without the risk of merge conflicts.

Patches are based on latest linux-trace/probes/for-next.

v1->v2:
  - fixed GCC aggressively inlining test_uretprobe_stack() function (BPF CI);
  - fixed comments (Peter).

Andrii Nakryiko (4):
  uprobes: rename get_trampoline_vaddr() and make it global
  perf,uprobes: fix user stack traces in the presence of pending
    uretprobes
  perf,x86: avoid missing caller address in stack traces captured in
    uprobe
  selftests/bpf: add test validating uprobe/uretprobe stack traces

 arch/x86/events/core.c                        |  20 ++
 include/linux/uprobes.h                       |   3 +
 kernel/events/callchain.c                     |  43 +++-
 kernel/events/uprobes.c                       |  17 +-
 .../bpf/prog_tests/uretprobe_stack.c          | 186 ++++++++++++++++++
 .../selftests/bpf/progs/uretprobe_stack.c     |  96 +++++++++
 6 files changed, 361 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/uretprobe_stack.c
 create mode 100644 tools/testing/selftests/bpf/progs/uretprobe_stack.c

Comments

Andrii Nakryiko June 3, 2024, 9:09 p.m. UTC | #1
On Tue, May 21, 2024 at 6:38 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> This patch set reports two issues with captured stack traces.
>
> First issue, fixed in patch #2, deals with fixing up uretprobe trampoline
> addresses in captured stack trace. This issue happens when there are pending
> return probes, for which kernel hijacks some of the return addresses on user
> stacks. The code is matching those special uretprobe trampoline addresses with
> the list of pending return probe instances and replaces them with actual
> return addresses. This is the same fixup logic that fprobe/kretprobe has for
> kernel stack traces.
>
> Second issue, which patch #3 is fixing with the help of heuristic, is having
> to do with capturing user stack traces in entry uprobes. At the very entrance
> to user function, frame pointer in rbp register is not yet setup, so actual
> caller return address is still pointed to by rsp. Patch is using a simple
> heuristic, looking for `push %rbp` instruction, to fetch this extra direct
> caller return address, before proceeding to unwind the stack using rbp.
>
> Patch #4 adds tests into BPF selftests, that validate that captured stack
> traces at various points is what we expect to get. This patch, while being BPF
> selftests, is isolated from any other BPF selftests changes and can go in
> through non-BPF tree without the risk of merge conflicts.
>
> Patches are based on latest linux-trace/probes/for-next.
>
> v1->v2:
>   - fixed GCC aggressively inlining test_uretprobe_stack() function (BPF CI);
>   - fixed comments (Peter).
>
> Andrii Nakryiko (4):
>   uprobes: rename get_trampoline_vaddr() and make it global
>   perf,uprobes: fix user stack traces in the presence of pending
>     uretprobes
>   perf,x86: avoid missing caller address in stack traces captured in
>     uprobe
>   selftests/bpf: add test validating uprobe/uretprobe stack traces
>
>  arch/x86/events/core.c                        |  20 ++
>  include/linux/uprobes.h                       |   3 +
>  kernel/events/callchain.c                     |  43 +++-
>  kernel/events/uprobes.c                       |  17 +-
>  .../bpf/prog_tests/uretprobe_stack.c          | 186 ++++++++++++++++++
>  .../selftests/bpf/progs/uretprobe_stack.c     |  96 +++++++++
>  6 files changed, 361 insertions(+), 4 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/uretprobe_stack.c
>  create mode 100644 tools/testing/selftests/bpf/progs/uretprobe_stack.c
>
> --
> 2.43.0
>

Friendly ping. This is a real issue in practice that our production
users are eager to be fixed, please help to get it upstream. Thanks!