• Mark Rutland's avatar
    arm64: stacktrace: avoid tracing arch_stack_walk() · 0c32706d
    Mark Rutland authored
    When the function_graph tracer is in use, arch_stack_walk() may unwind
    the stack incorrectly, erroneously reporting itself, missing the final
    entry which is being traced, and reporting all traced entries between
    these off-by-one from where they should be.
    
    When ftrace hooks a function return, the original return address is
    saved to the fgraph ret_stack, and the return address  in the LR (or the
    function's frame record) is replaced with `return_to_handler`.
    
    When arm64's unwinder encounter frames returning to `return_to_handler`,
    it finds the associated original return address from the fgraph ret
    stack, assuming the most recent `ret_to_hander` entry on the stack
    corresponds to the most recent entry in the fgraph ret stack, and so on.
    
    When arch_stack_walk() is used to dump the current task's stack, it
    starts from the caller of arch_stack_walk(). However, arch_stack_walk()
    can be traced, and so may push an entry on to the fgraph ret stack,
    leaving the fgraph ret stack offset by one from the expected position.
    
    This can be seen when dumping the stack via /proc/self/stack, where
    enabling the graph tracer results in an unexpected
    `stack_trace_save_tsk` entry at the start of the trace, and `el0_svc`
    missing form the end of the trace.
    
    This patch fixes this by marking arch_stack_walk() as notrace, as we do
    for all other functions on the path to ftrace_graph_get_ret_stack().
    While a few helper functions are not marked notrace, their calls/returns
    are balanced, and will have no observable effect when examining the
    fgraph ret stack.
    
    It is possible for an exeption boundary to cause a similar offset if the
    return address of the interrupted context was in the LR. Fixing those
    cases will require some more substantial rework, and is left for
    subsequent patches.
    
    Before:
    
    | # cat /proc/self/stack
    | [<0>] proc_pid_stack+0xc4/0x140
    | [<0>] proc_single_show+0x6c/0x120
    | [<0>] seq_read_iter+0x240/0x4e0
    | [<0>] seq_read+0xe8/0x140
    | [<0>] vfs_read+0xb8/0x1e4
    | [<0>] ksys_read+0x74/0x100
    | [<0>] __arm64_sys_read+0x28/0x3c
    | [<0>] invoke_syscall+0x50/0x120
    | [<0>] el0_svc_common.constprop.0+0xc4/0xd4
    | [<0>] do_el0_svc+0x30/0x9c
    | [<0>] el0_svc+0x2c/0x54
    | [<0>] el0t_64_sync_handler+0x1a8/0x1b0
    | [<0>] el0t_64_sync+0x198/0x19c
    | # echo function_graph > /sys/kernel/tracing/current_tracer
    | # cat /proc/self/stack
    | [<0>] stack_trace_save_tsk+0xa4/0x110
    | [<0>] proc_pid_stack+0xc4/0x140
    | [<0>] proc_single_show+0x6c/0x120
    | [<0>] seq_read_iter+0x240/0x4e0
    | [<0>] seq_read+0xe8/0x140
    | [<0>] vfs_read+0xb8/0x1e4
    | [<0>] ksys_read+0x74/0x100
    | [<0>] __arm64_sys_read+0x28/0x3c
    | [<0>] invoke_syscall+0x50/0x120
    | [<0>] el0_svc_common.constprop.0+0xc4/0xd4
    | [<0>] do_el0_svc+0x30/0x9c
    | [<0>] el0t_64_sync_handler+0x1a8/0x1b0
    | [<0>] el0t_64_sync+0x198/0x19c
    
    After:
    
    | # cat /proc/self/stack
    | [<0>] proc_pid_stack+0xc4/0x140
    | [<0>] proc_single_show+0x6c/0x120
    | [<0>] seq_read_iter+0x240/0x4e0
    | [<0>] seq_read+0xe8/0x140
    | [<0>] vfs_read+0xb8/0x1e4
    | [<0>] ksys_read+0x74/0x100
    | [<0>] __arm64_sys_read+0x28/0x3c
    | [<0>] invoke_syscall+0x50/0x120
    | [<0>] el0_svc_common.constprop.0+0xc4/0xd4
    | [<0>] do_el0_svc+0x30/0x9c
    | [<0>] el0_svc+0x2c/0x54
    | [<0>] el0t_64_sync_handler+0x1a8/0x1b0
    | [<0>] el0t_64_sync+0x198/0x19c
    | # echo function_graph > /sys/kernel/tracing/current_tracer
    | # cat /proc/self/stack
    | [<0>] proc_pid_stack+0xc4/0x140
    | [<0>] proc_single_show+0x6c/0x120
    | [<0>] seq_read_iter+0x240/0x4e0
    | [<0>] seq_read+0xe8/0x140
    | [<0>] vfs_read+0xb8/0x1e4
    | [<0>] ksys_read+0x74/0x100
    | [<0>] __arm64_sys_read+0x28/0x3c
    | [<0>] invoke_syscall+0x50/0x120
    | [<0>] el0_svc_common.constprop.0+0xc4/0xd4
    | [<0>] do_el0_svc+0x30/0x9c
    | [<0>] el0_svc+0x2c/0x54
    | [<0>] el0t_64_sync_handler+0x1a8/0x1b0
    | [<0>] el0t_64_sync+0x198/0x19c
    
    Cc: <stable@vger.kernel.org>
    Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
    Cc: Catalin Marinas <catalin.marinas@arm.com>
    Cc: Madhavan T. Venkataraman <madvenka@linux.microsoft.com>
    Cc: Mark Brown <broonie@kernel.org>
    Cc: Will Deacon <will@kernel.org>
    Reviwed-by: default avatarMark Brown <broonie@kernel.org>
    Link: https://lore.kernel.org/r/20210802164845.45506-3-mark.rutland@arm.comSigned-off-by: default avatarWill Deacon <will@kernel.org>
    0c32706d
stacktrace.c 5.83 KB