* [PATCH] BUG: demonstration of uprobe/uretprobe corrupted stack traces
@ 2022-05-24 5:57 Andrii Nakryiko
2022-06-09 20:34 ` Andrii Nakryiko
2022-06-12 14:11 ` Masami Hiramatsu
0 siblings, 2 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-05-24 5:57 UTC (permalink / raw)
To: mhiramat, bpf, ast; +Cc: rihams, jolsa, Andrii Nakryiko
Hi Masami,
We've got reports about partially corrupt stack traces when being captured from
uretprobes. Trying the simplest repro seems to confirm that something is not
quite right here. I'll try to debug it a bit more this week, but I was hoping
for you to take a look as well, if you get a chance.
Simple repro built on top of BPF selftests.
$ sudo ./test_progs -a uprobe_autoattach -v
...
FN ADDR 0x55fde0 - 0x55fdef
UPROBE SZ 40 (CNT 5) URETPROBE SZ 40 (CNT 5)
UPROBE 0x55fde0 URETPROBE 0x55ffd4
UPROBE 0x584653 URETPROBE 0x584653
UPROBE 0x585cc9 URETPROBE 0x585cc9
UPROBE 0x7fa9a31eaca3 URETPROBE 0x7fa9a31eaca3
UPROBE 0x5541f689495641d7 URETPROBE 0x5541f689495641d7
...
#203 uprobe_autoattach:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
There seem to be two distinct problems.
1. Last two entries for both uprobe and uretprobe stacks are not user-space
addressed (0x7fa9a31eaca3) and the very last one doesn't even look like a valid
address (0x5541f689495641d7).
2. Looking at first entry for UPROBE vs URETPROBE, you can see that uprobe
one's is correct and points exactly to the beginning of autoattach_trigger_func
(0x55fde0) as expected, but uretprobe entry (0x55ffd4) is way out of
autoattach_trigger_func (which is just 15 bytes long and ends at 0x55fdef).
Using addr2line it shows that it points to:
0x000000000055ffd4: test_uprobe_autoattach at /data/users/andriin/linux/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c:33
Which is a valid function and location to which autoattach_trigger_func()
should return (see objdump snippet below), but from uretprobe I'd imagine that
we are going to get address within traced user function (that is 0x55fde0 -
0x55fdef range), not the return address in a parent function.
55ffc4: 89 83 3c 08 00 00 mov %eax,0x83c(%rbx)
55ffca: 8b 45 e8 mov -0x18(%rbp),%eax
55ffcd: 89 c7 mov %eax,%edi
55ffcf: e8 0c fe ff ff call 55fde0 <autoattach_trigger_func>
--> 55ffd4: 89 45 a8 mov %eax,-0x58(%rbp)
55ffd7: ba ef fd 55 00 mov $0x55fdef,%edx
Both issues above seem unexpected, can you please see if I have some wrong
assumptions here?
Thanks in advance for taking a look!
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Riham Selim <rihams@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/Makefile | 3 ++-
.../selftests/bpf/prog_tests/uprobe_autoattach.c | 14 ++++++++++++++
.../selftests/bpf/progs/test_uprobe_autoattach.c | 11 +++++++++++
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 2d3c8c8f558a..0d3109c8d8d5 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -23,7 +23,8 @@ BPF_GCC ?= $(shell command -v bpf-gcc;)
SAN_CFLAGS ?=
CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \
-I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
- -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
+ -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) -fno-omit-frame-pointer
+
LDFLAGS += $(SAN_CFLAGS)
LDLIBS += -lelf -lz -lrt -lpthread
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
index 35b87c7ba5be..c0fbe4d240be 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
@@ -10,6 +10,7 @@ static noinline int autoattach_trigger_func(int arg)
asm volatile ("");
return arg + 1;
}
+static noinline int autoattach_trigger_func_post(int arg) { return 0; }
void test_uprobe_autoattach(void)
{
@@ -17,6 +18,7 @@ void test_uprobe_autoattach(void)
int trigger_val = 100, trigger_ret;
size_t malloc_sz = 1;
char *mem;
+ int i;
skel = test_uprobe_autoattach__open_and_load();
if (!ASSERT_OK_PTR(skel, "skel_open"))
@@ -30,6 +32,18 @@ void test_uprobe_autoattach(void)
/* trigger & validate uprobe & uretprobe */
trigger_ret = autoattach_trigger_func(trigger_val);
+ printf("FN ADDR %p - %p\n", autoattach_trigger_func, autoattach_trigger_func_post);
+ printf("UPROBE SZ %d (CNT %d) URETPROBE SZ %d (CNT %d)\n",
+ skel->bss->uprobe_stack_sz,
+ skel->bss->uprobe_stack_sz / 8,
+ skel->bss->uretprobe_stack_sz,
+ skel->bss->uretprobe_stack_sz / 8);
+ for (i = 0; i < skel->bss->uprobe_stack_sz / 8; i++) {
+ printf("UPROBE %-18p URETPROBE %-18p\n",
+ (void *)skel->bss->uprobe_stack[i],
+ (void *)skel->bss->uretprobe_stack[i]);
+ }
+
skel->bss->test_pid = getpid();
/* trigger & validate shared library u[ret]probes attached by name */
diff --git a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
index ab75522e2eeb..f630f83b4426 100644
--- a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
+++ b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
@@ -27,11 +27,19 @@ int handle_uprobe_noautoattach(struct pt_regs *ctx)
return 0;
}
+__u64 uprobe_stack[128];
+__u64 uretprobe_stack[128];
+int uprobe_stack_sz, uretprobe_stack_sz;
+
SEC("uprobe//proc/self/exe:autoattach_trigger_func")
int handle_uprobe_byname(struct pt_regs *ctx)
{
uprobe_byname_parm1 = PT_REGS_PARM1_CORE(ctx);
uprobe_byname_ran = 1;
+
+ uprobe_stack_sz = bpf_get_stack(ctx,
+ uprobe_stack, sizeof(uprobe_stack),
+ BPF_F_USER_STACK);
return 0;
}
@@ -40,6 +48,9 @@ int handle_uretprobe_byname(struct pt_regs *ctx)
{
uretprobe_byname_rc = PT_REGS_RC_CORE(ctx);
uretprobe_byname_ran = 2;
+ uretprobe_stack_sz = bpf_get_stack(ctx,
+ uretprobe_stack, sizeof(uretprobe_stack),
+ BPF_F_USER_STACK);
return 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] BUG: demonstration of uprobe/uretprobe corrupted stack traces
2022-05-24 5:57 [PATCH] BUG: demonstration of uprobe/uretprobe corrupted stack traces Andrii Nakryiko
@ 2022-06-09 20:34 ` Andrii Nakryiko
2022-06-12 14:11 ` Masami Hiramatsu
1 sibling, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-06-09 20:34 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Masami Hiramatsu, bpf, Alexei Starovoitov, rihams, Jiri Olsa,
Mykola Lysenko
On Mon, May 23, 2022 at 10:58 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Hi Masami,
>
> We've got reports about partially corrupt stack traces when being captured from
> uretprobes. Trying the simplest repro seems to confirm that something is not
> quite right here. I'll try to debug it a bit more this week, but I was hoping
> for you to take a look as well, if you get a chance.
>
A friendly ping! See below, at this point I'm mostly concerned about #2.
> Simple repro built on top of BPF selftests.
>
> $ sudo ./test_progs -a uprobe_autoattach -v
> ...
> FN ADDR 0x55fde0 - 0x55fdef
> UPROBE SZ 40 (CNT 5) URETPROBE SZ 40 (CNT 5)
> UPROBE 0x55fde0 URETPROBE 0x55ffd4
> UPROBE 0x584653 URETPROBE 0x584653
> UPROBE 0x585cc9 URETPROBE 0x585cc9
> UPROBE 0x7fa9a31eaca3 URETPROBE 0x7fa9a31eaca3
> UPROBE 0x5541f689495641d7 URETPROBE 0x5541f689495641d7
> ...
> #203 uprobe_autoattach:OK
> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>
> There seem to be two distinct problems.
>
> 1. Last two entries for both uprobe and uretprobe stacks are not user-space
> addressed (0x7fa9a31eaca3) and the very last one doesn't even look like a valid
> address (0x5541f689495641d7).
We can probably ignore this one, because I suspect it's due to my
system-wide libc was built without frame-pointers. So once we got into
some API inside libc stack traces got corrupted. But the one below
about not getting an address of a probed function for uretprobe still
stands. Can you please take a look? Thank you!
>
> 2. Looking at first entry for UPROBE vs URETPROBE, you can see that uprobe
> one's is correct and points exactly to the beginning of autoattach_trigger_func
> (0x55fde0) as expected, but uretprobe entry (0x55ffd4) is way out of
> autoattach_trigger_func (which is just 15 bytes long and ends at 0x55fdef).
> Using addr2line it shows that it points to:
>
> 0x000000000055ffd4: test_uprobe_autoattach at /data/users/andriin/linux/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c:33
>
> Which is a valid function and location to which autoattach_trigger_func()
> should return (see objdump snippet below), but from uretprobe I'd imagine that
> we are going to get address within traced user function (that is 0x55fde0 -
> 0x55fdef range), not the return address in a parent function.
>
> 55ffc4: 89 83 3c 08 00 00 mov %eax,0x83c(%rbx)
> 55ffca: 8b 45 e8 mov -0x18(%rbp),%eax
> 55ffcd: 89 c7 mov %eax,%edi
> 55ffcf: e8 0c fe ff ff call 55fde0 <autoattach_trigger_func>
> --> 55ffd4: 89 45 a8 mov %eax,-0x58(%rbp)
> 55ffd7: ba ef fd 55 00 mov $0x55fdef,%edx
>
> Both issues above seem unexpected, can you please see if I have some wrong
> assumptions here?
>
> Thanks in advance for taking a look!
>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Riham Selim <rihams@fb.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/testing/selftests/bpf/Makefile | 3 ++-
> .../selftests/bpf/prog_tests/uprobe_autoattach.c | 14 ++++++++++++++
> .../selftests/bpf/progs/test_uprobe_autoattach.c | 11 +++++++++++
> 3 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 2d3c8c8f558a..0d3109c8d8d5 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -23,7 +23,8 @@ BPF_GCC ?= $(shell command -v bpf-gcc;)
> SAN_CFLAGS ?=
> CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \
> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> - -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
> + -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) -fno-omit-frame-pointer
> +
> LDFLAGS += $(SAN_CFLAGS)
> LDLIBS += -lelf -lz -lrt -lpthread
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> index 35b87c7ba5be..c0fbe4d240be 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> @@ -10,6 +10,7 @@ static noinline int autoattach_trigger_func(int arg)
> asm volatile ("");
> return arg + 1;
> }
> +static noinline int autoattach_trigger_func_post(int arg) { return 0; }
>
> void test_uprobe_autoattach(void)
> {
> @@ -17,6 +18,7 @@ void test_uprobe_autoattach(void)
> int trigger_val = 100, trigger_ret;
> size_t malloc_sz = 1;
> char *mem;
> + int i;
>
> skel = test_uprobe_autoattach__open_and_load();
> if (!ASSERT_OK_PTR(skel, "skel_open"))
> @@ -30,6 +32,18 @@ void test_uprobe_autoattach(void)
> /* trigger & validate uprobe & uretprobe */
> trigger_ret = autoattach_trigger_func(trigger_val);
>
> + printf("FN ADDR %p - %p\n", autoattach_trigger_func, autoattach_trigger_func_post);
> + printf("UPROBE SZ %d (CNT %d) URETPROBE SZ %d (CNT %d)\n",
> + skel->bss->uprobe_stack_sz,
> + skel->bss->uprobe_stack_sz / 8,
> + skel->bss->uretprobe_stack_sz,
> + skel->bss->uretprobe_stack_sz / 8);
> + for (i = 0; i < skel->bss->uprobe_stack_sz / 8; i++) {
> + printf("UPROBE %-18p URETPROBE %-18p\n",
> + (void *)skel->bss->uprobe_stack[i],
> + (void *)skel->bss->uretprobe_stack[i]);
> + }
> +
> skel->bss->test_pid = getpid();
>
> /* trigger & validate shared library u[ret]probes attached by name */
> diff --git a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> index ab75522e2eeb..f630f83b4426 100644
> --- a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> +++ b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> @@ -27,11 +27,19 @@ int handle_uprobe_noautoattach(struct pt_regs *ctx)
> return 0;
> }
>
> +__u64 uprobe_stack[128];
> +__u64 uretprobe_stack[128];
> +int uprobe_stack_sz, uretprobe_stack_sz;
> +
> SEC("uprobe//proc/self/exe:autoattach_trigger_func")
> int handle_uprobe_byname(struct pt_regs *ctx)
> {
> uprobe_byname_parm1 = PT_REGS_PARM1_CORE(ctx);
> uprobe_byname_ran = 1;
> +
> + uprobe_stack_sz = bpf_get_stack(ctx,
> + uprobe_stack, sizeof(uprobe_stack),
> + BPF_F_USER_STACK);
> return 0;
> }
>
> @@ -40,6 +48,9 @@ int handle_uretprobe_byname(struct pt_regs *ctx)
> {
> uretprobe_byname_rc = PT_REGS_RC_CORE(ctx);
> uretprobe_byname_ran = 2;
> + uretprobe_stack_sz = bpf_get_stack(ctx,
> + uretprobe_stack, sizeof(uretprobe_stack),
> + BPF_F_USER_STACK);
> return 0;
> }
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] BUG: demonstration of uprobe/uretprobe corrupted stack traces
2022-05-24 5:57 [PATCH] BUG: demonstration of uprobe/uretprobe corrupted stack traces Andrii Nakryiko
2022-06-09 20:34 ` Andrii Nakryiko
@ 2022-06-12 14:11 ` Masami Hiramatsu
2022-06-16 0:33 ` Andrii Nakryiko
1 sibling, 1 reply; 4+ messages in thread
From: Masami Hiramatsu @ 2022-06-12 14:11 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, rihams, jolsa, oleg
Hi Andrii,
Sorry for waiting.
On Mon, 23 May 2022 22:57:48 -0700
Andrii Nakryiko <andrii@kernel.org> wrote:
> Hi Masami,
>
> We've got reports about partially corrupt stack traces when being captured from
> uretprobes. Trying the simplest repro seems to confirm that something is not
> quite right here. I'll try to debug it a bit more this week, but I was hoping
> for you to take a look as well, if you get a chance.
Ah, uprobes/uretprobes will be handled by Oleg. But I'll try to explain it
as far as I know. Oleg, please correct me if I'm wrong.
>
> Simple repro built on top of BPF selftests.
>
> $ sudo ./test_progs -a uprobe_autoattach -v
> ...
> FN ADDR 0x55fde0 - 0x55fdef
> UPROBE SZ 40 (CNT 5) URETPROBE SZ 40 (CNT 5)
> UPROBE 0x55fde0 URETPROBE 0x55ffd4
> UPROBE 0x584653 URETPROBE 0x584653
> UPROBE 0x585cc9 URETPROBE 0x585cc9
> UPROBE 0x7fa9a31eaca3 URETPROBE 0x7fa9a31eaca3
> UPROBE 0x5541f689495641d7 URETPROBE 0x5541f689495641d7
> ...
> #203 uprobe_autoattach:OK
> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>
> There seem to be two distinct problems.
>
> 1. Last two entries for both uprobe and uretprobe stacks are not user-space
> addressed (0x7fa9a31eaca3) and the very last one doesn't even look like a valid
> address (0x5541f689495641d7).
Hmm, what user-space stack unwinder are you using? I guess it comes
from the user-space stack unwinder routine.
>
> 2. Looking at first entry for UPROBE vs URETPROBE, you can see that uprobe
> one's is correct and points exactly to the beginning of autoattach_trigger_func
> (0x55fde0) as expected, but uretprobe entry (0x55ffd4) is way out of
> autoattach_trigger_func (which is just 15 bytes long and ends at 0x55fdef).
> Using addr2line it shows that it points to:
>
> 0x000000000055ffd4: test_uprobe_autoattach at /data/users/andriin/linux/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c:33
>
> Which is a valid function and location to which autoattach_trigger_func()
> should return (see objdump snippet below), but from uretprobe I'd imagine that
> we are going to get address within traced user function (that is 0x55fde0 -
> 0x55fdef range), not the return address in a parent function.
No, it is expected behavior. Since the "return probe" probes the function
returning, the first entry of the stack address is the address where the
function is returned (IOW, the IP address when the probe hit). Not the
address of the return instruction. As long as it uses the stack to hook
the return of the function, it's impossible in principle.
(if it decodes the function to find the return instruction and replace it
with a software breakpoint, it may be possible. But this will not work
if the function does tail call etc.)
>
> 55ffc4: 89 83 3c 08 00 00 mov %eax,0x83c(%rbx)
> 55ffca: 8b 45 e8 mov -0x18(%rbp),%eax
> 55ffcd: 89 c7 mov %eax,%edi
> 55ffcf: e8 0c fe ff ff call 55fde0 <autoattach_trigger_func>
> --> 55ffd4: 89 45 a8 mov %eax,-0x58(%rbp)
> 55ffd7: ba ef fd 55 00 mov $0x55fdef,%edx
>
> Both issues above seem unexpected, can you please see if I have some wrong
> assumptions here?
So, #1 maybe we need to look into the stack unwinder (bpf_get_stack()?)
Does this happen if you unwind user-stack from kprobe event?
Oleg, would you know anything about this issue?
And #2 is the expected behavior.
Thank you,
>
> Thanks in advance for taking a look!
>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Riham Selim <rihams@fb.com>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/testing/selftests/bpf/Makefile | 3 ++-
> .../selftests/bpf/prog_tests/uprobe_autoattach.c | 14 ++++++++++++++
> .../selftests/bpf/progs/test_uprobe_autoattach.c | 11 +++++++++++
> 3 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 2d3c8c8f558a..0d3109c8d8d5 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -23,7 +23,8 @@ BPF_GCC ?= $(shell command -v bpf-gcc;)
> SAN_CFLAGS ?=
> CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \
> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> - -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
> + -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) -fno-omit-frame-pointer
> +
> LDFLAGS += $(SAN_CFLAGS)
> LDLIBS += -lelf -lz -lrt -lpthread
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> index 35b87c7ba5be..c0fbe4d240be 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> @@ -10,6 +10,7 @@ static noinline int autoattach_trigger_func(int arg)
> asm volatile ("");
> return arg + 1;
> }
> +static noinline int autoattach_trigger_func_post(int arg) { return 0; }
>
> void test_uprobe_autoattach(void)
> {
> @@ -17,6 +18,7 @@ void test_uprobe_autoattach(void)
> int trigger_val = 100, trigger_ret;
> size_t malloc_sz = 1;
> char *mem;
> + int i;
>
> skel = test_uprobe_autoattach__open_and_load();
> if (!ASSERT_OK_PTR(skel, "skel_open"))
> @@ -30,6 +32,18 @@ void test_uprobe_autoattach(void)
> /* trigger & validate uprobe & uretprobe */
> trigger_ret = autoattach_trigger_func(trigger_val);
>
> + printf("FN ADDR %p - %p\n", autoattach_trigger_func, autoattach_trigger_func_post);
> + printf("UPROBE SZ %d (CNT %d) URETPROBE SZ %d (CNT %d)\n",
> + skel->bss->uprobe_stack_sz,
> + skel->bss->uprobe_stack_sz / 8,
> + skel->bss->uretprobe_stack_sz,
> + skel->bss->uretprobe_stack_sz / 8);
> + for (i = 0; i < skel->bss->uprobe_stack_sz / 8; i++) {
> + printf("UPROBE %-18p URETPROBE %-18p\n",
> + (void *)skel->bss->uprobe_stack[i],
> + (void *)skel->bss->uretprobe_stack[i]);
> + }
> +
> skel->bss->test_pid = getpid();
>
> /* trigger & validate shared library u[ret]probes attached by name */
> diff --git a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> index ab75522e2eeb..f630f83b4426 100644
> --- a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> +++ b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> @@ -27,11 +27,19 @@ int handle_uprobe_noautoattach(struct pt_regs *ctx)
> return 0;
> }
>
> +__u64 uprobe_stack[128];
> +__u64 uretprobe_stack[128];
> +int uprobe_stack_sz, uretprobe_stack_sz;
> +
> SEC("uprobe//proc/self/exe:autoattach_trigger_func")
> int handle_uprobe_byname(struct pt_regs *ctx)
> {
> uprobe_byname_parm1 = PT_REGS_PARM1_CORE(ctx);
> uprobe_byname_ran = 1;
> +
> + uprobe_stack_sz = bpf_get_stack(ctx,
> + uprobe_stack, sizeof(uprobe_stack),
> + BPF_F_USER_STACK);
> return 0;
> }
>
> @@ -40,6 +48,9 @@ int handle_uretprobe_byname(struct pt_regs *ctx)
> {
> uretprobe_byname_rc = PT_REGS_RC_CORE(ctx);
> uretprobe_byname_ran = 2;
> + uretprobe_stack_sz = bpf_get_stack(ctx,
> + uretprobe_stack, sizeof(uretprobe_stack),
> + BPF_F_USER_STACK);
> return 0;
> }
>
> --
> 2.30.2
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] BUG: demonstration of uprobe/uretprobe corrupted stack traces
2022-06-12 14:11 ` Masami Hiramatsu
@ 2022-06-16 0:33 ` Andrii Nakryiko
0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-06-16 0:33 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, rihams, Jiri Olsa,
Oleg Nesterov
On Sun, Jun 12, 2022 at 7:11 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi Andrii,
>
> Sorry for waiting.
>
> On Mon, 23 May 2022 22:57:48 -0700
> Andrii Nakryiko <andrii@kernel.org> wrote:
>
> > Hi Masami,
> >
> > We've got reports about partially corrupt stack traces when being captured from
> > uretprobes. Trying the simplest repro seems to confirm that something is not
> > quite right here. I'll try to debug it a bit more this week, but I was hoping
> > for you to take a look as well, if you get a chance.
>
> Ah, uprobes/uretprobes will be handled by Oleg. But I'll try to explain it
> as far as I know. Oleg, please correct me if I'm wrong.
>
> >
> > Simple repro built on top of BPF selftests.
> >
> > $ sudo ./test_progs -a uprobe_autoattach -v
> > ...
> > FN ADDR 0x55fde0 - 0x55fdef
> > UPROBE SZ 40 (CNT 5) URETPROBE SZ 40 (CNT 5)
> > UPROBE 0x55fde0 URETPROBE 0x55ffd4
> > UPROBE 0x584653 URETPROBE 0x584653
> > UPROBE 0x585cc9 URETPROBE 0x585cc9
> > UPROBE 0x7fa9a31eaca3 URETPROBE 0x7fa9a31eaca3
> > UPROBE 0x5541f689495641d7 URETPROBE 0x5541f689495641d7
> > ...
> > #203 uprobe_autoattach:OK
> > Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> >
> > There seem to be two distinct problems.
> >
> > 1. Last two entries for both uprobe and uretprobe stacks are not user-space
> > addressed (0x7fa9a31eaca3) and the very last one doesn't even look like a valid
> > address (0x5541f689495641d7).
>
> Hmm, what user-space stack unwinder are you using? I guess it comes
> from the user-space stack unwinder routine.
>
> >
> > 2. Looking at first entry for UPROBE vs URETPROBE, you can see that uprobe
> > one's is correct and points exactly to the beginning of autoattach_trigger_func
> > (0x55fde0) as expected, but uretprobe entry (0x55ffd4) is way out of
> > autoattach_trigger_func (which is just 15 bytes long and ends at 0x55fdef).
> > Using addr2line it shows that it points to:
> >
> > 0x000000000055ffd4: test_uprobe_autoattach at /data/users/andriin/linux/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c:33
> >
> > Which is a valid function and location to which autoattach_trigger_func()
> > should return (see objdump snippet below), but from uretprobe I'd imagine that
> > we are going to get address within traced user function (that is 0x55fde0 -
> > 0x55fdef range), not the return address in a parent function.
>
> No, it is expected behavior. Since the "return probe" probes the function
> returning, the first entry of the stack address is the address where the
> function is returned (IOW, the IP address when the probe hit). Not the
> address of the return instruction. As long as it uses the stack to hook
> the return of the function, it's impossible in principle.
> (if it decodes the function to find the return instruction and replace it
> with a software breakpoint, it may be possible. But this will not work
> if the function does tail call etc.)
>
> >
> > 55ffc4: 89 83 3c 08 00 00 mov %eax,0x83c(%rbx)
> > 55ffca: 8b 45 e8 mov -0x18(%rbp),%eax
> > 55ffcd: 89 c7 mov %eax,%edi
> > 55ffcf: e8 0c fe ff ff call 55fde0 <autoattach_trigger_func>
> > --> 55ffd4: 89 45 a8 mov %eax,-0x58(%rbp)
> > 55ffd7: ba ef fd 55 00 mov $0x55fdef,%edx
> >
> > Both issues above seem unexpected, can you please see if I have some wrong
> > assumptions here?
>
> So, #1 maybe we need to look into the stack unwinder (bpf_get_stack()?)
> Does this happen if you unwind user-stack from kprobe event?
> Oleg, would you know anything about this issue?
>
I suspect that this is due to no-frame-pointer libc, so I haven't dug deeper.
> And #2 is the expected behavior.
>
Ok, I see, thanks for confirming!
> Thank you,
>
> >
> > Thanks in advance for taking a look!
> >
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Riham Selim <rihams@fb.com>
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/testing/selftests/bpf/Makefile | 3 ++-
> > .../selftests/bpf/prog_tests/uprobe_autoattach.c | 14 ++++++++++++++
> > .../selftests/bpf/progs/test_uprobe_autoattach.c | 11 +++++++++++
> > 3 files changed, 27 insertions(+), 1 deletion(-)
> >
[...]
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-16 0:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-24 5:57 [PATCH] BUG: demonstration of uprobe/uretprobe corrupted stack traces Andrii Nakryiko
2022-06-09 20:34 ` Andrii Nakryiko
2022-06-12 14:11 ` Masami Hiramatsu
2022-06-16 0:33 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox