2022-08-19 08:55:37

by Chen Zhongjin

[permalink] [raw]
Subject: [PATCH v2] x86/unwind/orc: unwind ftrace trampolines with correct orc

When meeting ftrace trampolines in orc unwinding, unwinder uses address
of ftrace_{regs_}call address to find the orc, which gets next frame at
sp+176.

If there is an irq hitting at sub $0xa8,%rsp, the next frame should be
sp+8 instead of 176. It makes unwinder skip correct frame and throw
warnings such as "wrong direction" or "can't access registers", etc,
depending on the content of the wrong frame address.

By adding the base address ftrace_{regs_}caller with the offset
*ip - ops->trampoline*,
we can get the correct address to find orc.

Also change "caller" to "tramp_addr" to make variable name conform to
its content.

Fixes: 6be7fa3c74d1 ("ftrace, orc, x86: Handle ftrace dynamically allocated trampolines")
Cc: <[email protected]>
Signed-off-by: Chen Zhongjin <[email protected]>
---
v1 -> v2:
Add some comments.
---
arch/x86/kernel/unwind_orc.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 38185aedf7d1..0ea57da92940 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -93,22 +93,27 @@ static struct orc_entry *orc_find(unsigned long ip);
static struct orc_entry *orc_ftrace_find(unsigned long ip)
{
struct ftrace_ops *ops;
- unsigned long caller;
+ unsigned long tramp_addr, offset;

ops = ftrace_ops_trampoline(ip);
if (!ops)
return NULL;

+ /* Set tramp_addr to the start of the code copied by the trampoline */
if (ops->flags & FTRACE_OPS_FL_SAVE_REGS)
- caller = (unsigned long)ftrace_regs_call;
+ tramp_addr = (unsigned long)ftrace_regs_caller;
else
- caller = (unsigned long)ftrace_call;
+ tramp_addr = (unsigned long)ftrace_caller;
+
+ /* Now place tramp_addr to the location within the trampoline ip is at */
+ offset = ip - ops->trampoline;
+ tramp_addr += offset;

/* Prevent unlikely recursion */
- if (ip == caller)
+ if (ip == tramp_addr)
return NULL;

- return orc_find(caller);
+ return orc_find(tramp_addr);
}
#else
static struct orc_entry *orc_ftrace_find(unsigned long ip)
--
2.17.1


2022-08-19 18:05:20

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2] x86/unwind/orc: unwind ftrace trampolines with correct orc

On Fri, 19 Aug 2022 16:43:34 +0800
Chen Zhongjin <[email protected]> wrote:

> When meeting ftrace trampolines in orc unwinding, unwinder uses address
> of ftrace_{regs_}call address to find the orc, which gets next frame at
> sp+176.
>
> If there is an irq hitting at sub $0xa8,%rsp, the next frame should be
> sp+8 instead of 176. It makes unwinder skip correct frame and throw
> warnings such as "wrong direction" or "can't access registers", etc,
> depending on the content of the wrong frame address.
>
> By adding the base address ftrace_{regs_}caller with the offset
> *ip - ops->trampoline*,
> we can get the correct address to find orc.
>
> Also change "caller" to "tramp_addr" to make variable name conform to
> its content.
>
> Fixes: 6be7fa3c74d1 ("ftrace, orc, x86: Handle ftrace dynamically allocated trampolines")
> Cc: <[email protected]>
> Signed-off-by: Chen Zhongjin <[email protected]>

Reviewed-by: Steven Rostedt (Google) <[email protected]>

Would someone from the tip tree care to pull this in?

-- Steve

Subject: [tip: x86/urgent] x86/unwind/orc: Unwind ftrace trampolines with correct ORC entry

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: fc2e426b1161761561624ebd43ce8c8d2fa058da
Gitweb: https://git.kernel.org/tip/fc2e426b1161761561624ebd43ce8c8d2fa058da
Author: Chen Zhongjin <[email protected]>
AuthorDate: Fri, 19 Aug 2022 16:43:34 +08:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 21 Aug 2022 12:19:32 +02:00

x86/unwind/orc: Unwind ftrace trampolines with correct ORC entry

When meeting ftrace trampolines in ORC unwinding, unwinder uses address
of ftrace_{regs_}call address to find the ORC entry, which gets next frame at
sp+176.

If there is an IRQ hitting at sub $0xa8,%rsp, the next frame should be
sp+8 instead of 176. It makes unwinder skip correct frame and throw
warnings such as "wrong direction" or "can't access registers", etc,
depending on the content of the incorrect frame address.

By adding the base address ftrace_{regs_}caller with the offset
*ip - ops->trampoline*, we can get the correct address to find the ORC entry.

Also change "caller" to "tramp_addr" to make variable name conform to
its content.

[ mingo: Clarified the changelog a bit. ]

Fixes: 6be7fa3c74d1 ("ftrace, orc, x86: Handle ftrace dynamically allocated trampolines")
Signed-off-by: Chen Zhongjin <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Steven Rostedt (Google) <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/unwind_orc.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 38185ae..0ea57da 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -93,22 +93,27 @@ static struct orc_entry *orc_find(unsigned long ip);
static struct orc_entry *orc_ftrace_find(unsigned long ip)
{
struct ftrace_ops *ops;
- unsigned long caller;
+ unsigned long tramp_addr, offset;

ops = ftrace_ops_trampoline(ip);
if (!ops)
return NULL;

+ /* Set tramp_addr to the start of the code copied by the trampoline */
if (ops->flags & FTRACE_OPS_FL_SAVE_REGS)
- caller = (unsigned long)ftrace_regs_call;
+ tramp_addr = (unsigned long)ftrace_regs_caller;
else
- caller = (unsigned long)ftrace_call;
+ tramp_addr = (unsigned long)ftrace_caller;
+
+ /* Now place tramp_addr to the location within the trampoline ip is at */
+ offset = ip - ops->trampoline;
+ tramp_addr += offset;

/* Prevent unlikely recursion */
- if (ip == caller)
+ if (ip == tramp_addr)
return NULL;

- return orc_find(caller);
+ return orc_find(tramp_addr);
}
#else
static struct orc_entry *orc_ftrace_find(unsigned long ip)

2022-08-21 11:15:37

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] x86/unwind/orc: unwind ftrace trampolines with correct orc


* Steven Rostedt <[email protected]> wrote:

> On Fri, 19 Aug 2022 16:43:34 +0800
> Chen Zhongjin <[email protected]> wrote:
>
> > When meeting ftrace trampolines in orc unwinding, unwinder uses address
> > of ftrace_{regs_}call address to find the orc, which gets next frame at
> > sp+176.
> >
> > If there is an irq hitting at sub $0xa8,%rsp, the next frame should be
> > sp+8 instead of 176. It makes unwinder skip correct frame and throw
> > warnings such as "wrong direction" or "can't access registers", etc,
> > depending on the content of the wrong frame address.
> >
> > By adding the base address ftrace_{regs_}caller with the offset
> > *ip - ops->trampoline*,
> > we can get the correct address to find orc.
> >
> > Also change "caller" to "tramp_addr" to make variable name conform to
> > its content.
> >
> > Fixes: 6be7fa3c74d1 ("ftrace, orc, x86: Handle ftrace dynamically allocated trampolines")
> > Cc: <[email protected]>
> > Signed-off-by: Chen Zhongjin <[email protected]>
>
> Reviewed-by: Steven Rostedt (Google) <[email protected]>
>
> Would someone from the tip tree care to pull this in?

Picked it up into tip:x86/urgent with minor edits to the changelog - will
push it out after some testing.

Thanks,

Ingo

2022-08-22 15:07:01

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v2] x86/unwind/orc: unwind ftrace trampolines with correct orc

On Sun, 21 Aug 2022 12:20:50 +0200
Ingo Molnar <[email protected]> wrote:

> > Reviewed-by: Steven Rostedt (Google) <[email protected]>
> >
> > Would someone from the tip tree care to pull this in?
>
> Picked it up into tip:x86/urgent with minor edits to the changelog - will
> push it out after some testing.

Thanks Ingo!

-- Steve