Re: [PATCH] x86: kprobes: orc: Fix ORC walks in kretprobes

[Masami Hiramatsu]

On Fri, 5 Mar 2021 18:28:06 +0900
Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:

> Hi Daniel,
> 
> On Thu,  4 Mar 2021 16:07:52 -0800
> Daniel Xu <dxu@xxxxxxxxx> wrote:
> 
> > Getting a stack trace from inside a kretprobe used to work with frame
> > pointer stack walks. After the default unwinder was switched to ORC,
> > stack traces broke because ORC did not know how to skip the
> > `kretprobe_trampoline` "frame".
> > 
> > Frame based stack walks used to work with kretprobes because
> > `kretprobe_trampoline` does not set up a new call frame. Thus, the frame
> > pointer based unwinder could walk directly to the kretprobe'd caller.
> > 
> > For example, this stack is walked incorrectly with ORC + kretprobe:
> > 
> >     # bpftrace -e 'kretprobe:do_nanosleep { @[kstack] = count() }'
> >     Attaching 1 probe...
> >     ^C
> > 
> >     @[
> >         kretprobe_trampoline+0
> >     ]: 1
> > 
> > After this patch, the stack is walked correctly:
> > 
> >     # bpftrace -e 'kretprobe:do_nanosleep { @[kstack] = count() }'
> >     Attaching 1 probe...
> >     ^C
> > 
> >     @[
> >         kretprobe_trampoline+0
> >         __x64_sys_nanosleep+150
> >         do_syscall_64+51
> >         entry_SYSCALL_64_after_hwframe+68
> >     ]: 12
> > 
> > Fixes: fc72ae40e303 ("x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit")
> > Signed-off-by: Daniel Xu <dxu@xxxxxxxxx>
> 
> OK, basically good, but this is messy, and doing much more than fixing issue.

BTW, is this a regression? or CONFIG_UNWINDER_ORC has this issue before?
It seems that the above commit just changed the default unwinder. This means
OCR stack unwinder has this bug before that commit. If you choose the 
CONFIG_UNWINDER_FRAME_POINTER, it might work again.

If so, it is not a regression. this need to be fixed, but ORC has this issue
from the original code.

Thank you,

-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>