linux-kernel - Re: [PATCH v3] ftrace: Hide a extra entry in stack trace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20241004231732.247301975e1e8f1511b50c6a@kernel.org>
Date: Fri, 4 Oct 2024 23:17:32 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Tatsuya S <tatsuya.s2862@...il.com>
Cc: Steven Rostedt <rostedt@...dmis.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH v3] ftrace: Hide a extra entry in stack trace

On Wed,  2 Oct 2024 14:13:48 +0900
Tatsuya S <tatsuya.s2862@...il.com> wrote:

> A extra entry is shown on stack trace(CONFIG_UNWINDER_ORC=y).
> 
> [003] .....   110.171589: vfs_write <-__x64_sys_write
> [003] .....   110.171600: <stack trace>
> => XXXXXXXXX (Wrong function name)
> => vfs_write
> => ksys_write
> => do_syscall_64
> => entry_SYSCALL_64_after_hwframe
> 
> To resolve this, increment skip for __ftrace_trace_stack() in
> function_stack_trace_call().
> The reason why skip is incremented for __ftrace_trace_stack()
> is because __ftrace_trace_stack() in stack trace is the only function
> that wasn't skipped from anywhere.

Hi Tatsuya,

Can you focus on making test cases which checks what combinations caused
this wrong values and what does not? Also it should be checked with
various kconfigs. That is more valuable than making add-hoc fixes.

Thank you,

> 
> Signed-off-by: Tatsuya S <tatsuya.s2862@...il.com>
> ---
> V2 -> V3: Changed the place to increment skip number
> V1 -> V2: Fixed redundant code
> 
>  kernel/trace/trace_functions.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> index 3b0cea37e029..27089d8e65d4 100644
> --- a/kernel/trace/trace_functions.c
> +++ b/kernel/trace/trace_functions.c
> @@ -203,23 +203,21 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
>  	ftrace_test_recursion_unlock(bit);
>  }
>  
> -#ifdef CONFIG_UNWINDER_ORC
> -/*
> - * Skip 2:
> - *
> - *   function_stack_trace_call()
> - *   ftrace_call()
> - */
> -#define STACK_SKIP 2
> -#else
>  /*
>   * Skip 3:
> - *   __trace_stack()
> - *   function_stack_trace_call()
> - *   ftrace_call()
> + *   Skipped functions if CONFIG_UNWINDER_ORC is defined
> + *
> + *     __ftrace_trace_stack()
> + *     function_stack_trace_call()
> + *     ftrace_call()
> + *
> + *   Otherwise
> + *
> + *     __trace_stack()
> + *     function_stack_trace_call()
> + *     ftrace_call()
>   */
>  #define STACK_SKIP 3
> -#endif
>  
>  static void
>  function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
> -- 
> 2.46.2
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>