[PATCH] ftrace: Fix function name for trampoline

[PATCH] ftrace: Fix function name for trampoline

[Tatsuya S]

The issue that unrelated function name is shown on stack trace like
following even though it should be trampoline code address is caused by
the creation of trampoline code in the area where .init.text section
of module was freed after module is loaded.

bash-1344    [002] .....    43.644608: <stack trace>
  => (MODULE INIT FUNCTION)
  => vfs_write
  => ksys_write
  => do_syscall_64
  => entry_SYSCALL_64_after_hwframe

To resolve this, when function address of stack trace entry is in
trampoline, output without looking up symbol name.

Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
---
 kernel/trace/trace_output.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 868f2f912f28..32a0858373e2 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
 			break;
 
 		trace_seq_puts(s, " => ");
+		if (is_ftrace_trampoline((*p) + delta)) {
+			trace_seq_printf(s, "0x%08lx", (*p) + delta);
+			trace_seq_putc(s, '\n');
+			continue;
+		}
 		seq_print_ip_sym(s, (*p) + delta, flags);
 		trace_seq_putc(s, '\n');
 	}
-- 
2.46.2

Re: [PATCH] ftrace: Fix function name for trampoline

[Steven Rostedt]

On Thu, 10 Oct 2024 22:02:59 +0900
Tatsuya S <tatsuya.s2862@gmail.com> wrote:

> The issue that unrelated function name is shown on stack trace like
> following even though it should be trampoline code address is caused by
> the creation of trampoline code in the area where .init.text section
> of module was freed after module is loaded.
> 
> bash-1344    [002] .....    43.644608: <stack trace>
>   => (MODULE INIT FUNCTION)
>   => vfs_write
>   => ksys_write
>   => do_syscall_64
>   => entry_SYSCALL_64_after_hwframe  
> 
> To resolve this, when function address of stack trace entry is in
> trampoline, output without looking up symbol name.
> 
> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
> ---
>  kernel/trace/trace_output.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 868f2f912f28..32a0858373e2 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>  			break;
>  
>  		trace_seq_puts(s, " => ");
> +		if (is_ftrace_trampoline((*p) + delta)) {

This is not reliable. The output is called when the user reads the trace
file and the ops may no longer exist.

The only way to test this is if you call it during the trace. Yes it may
slow things down a little, but it will be accurate.

-- Steve


> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);
> +			trace_seq_putc(s, '\n');
> +			continue;
> +		}
>  		seq_print_ip_sym(s, (*p) + delta, flags);
>  		trace_seq_putc(s, '\n');
>  	}

Re: [PATCH] ftrace: Fix function name for trampoline

[Masami Hiramatsu]

On Thu, 10 Oct 2024 22:02:59 +0900
Tatsuya S <tatsuya.s2862@gmail.com> wrote:

> The issue that unrelated function name is shown on stack trace like
> following even though it should be trampoline code address is caused by
> the creation of trampoline code in the area where .init.text section
> of module was freed after module is loaded.
> 
> bash-1344    [002] .....    43.644608: <stack trace>
>   => (MODULE INIT FUNCTION)
>   => vfs_write
>   => ksys_write
>   => do_syscall_64
>   => entry_SYSCALL_64_after_hwframe
> 
> To resolve this, when function address of stack trace entry is in
> trampoline, output without looking up symbol name.
> 
> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
> ---
>  kernel/trace/trace_output.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 868f2f912f28..32a0858373e2 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>  			break;
>  
>  		trace_seq_puts(s, " => ");
> +		if (is_ftrace_trampoline((*p) + delta)) {
> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);

If we know that address is the ftrace trampoline, we'd better show something
like "[FTRACE TRAMPOLINE]"

> +			trace_seq_putc(s, '\n');

And this is not needed. So for example,

			trace_seq_puts(s, "[FTRACE TRAMPOLINE]\n");

is enough.

> +			continue;
> +		}
>  		seq_print_ip_sym(s, (*p) + delta, flags);
>  		trace_seq_putc(s, '\n');
>  	}

Thank you,

> -- 
> 2.46.2
> 


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

Re: [PATCH] ftrace: Fix function name for trampoline

[Tatsuya S]

On 10/11/24 12:03 AM, Masami Hiramatsu (Google) wrote:
> On Thu, 10 Oct 2024 22:02:59 +0900
> Tatsuya S <tatsuya.s2862@gmail.com> wrote:
> 
>> The issue that unrelated function name is shown on stack trace like
>> following even though it should be trampoline code address is caused by
>> the creation of trampoline code in the area where .init.text section
>> of module was freed after module is loaded.
>>
>> bash-1344    [002] .....    43.644608: <stack trace>
>>    => (MODULE INIT FUNCTION)
>>    => vfs_write
>>    => ksys_write
>>    => do_syscall_64
>>    => entry_SYSCALL_64_after_hwframe
>>
>> To resolve this, when function address of stack trace entry is in
>> trampoline, output without looking up symbol name.
>>
>> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
>> ---
>>   kernel/trace/trace_output.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
>> index 868f2f912f28..32a0858373e2 100644
>> --- a/kernel/trace/trace_output.c
>> +++ b/kernel/trace/trace_output.c
>> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>>   			break;
>>   
>>   		trace_seq_puts(s, " => ");
>> +		if (is_ftrace_trampoline((*p) + delta)) {
>> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);
> 
> If we know that address is the ftrace trampoline, we'd better show something
> like "[FTRACE TRAMPOLINE]"
> 
>> +			trace_seq_putc(s, '\n');
> 
> And this is not needed. So for example,
> 
> 			trace_seq_puts(s, "[FTRACE TRAMPOLINE]\n");
> 
> is enough.

Thanks for review. OK.

> 
>> +			continue;
>> +		}
>>   		seq_print_ip_sym(s, (*p) + delta, flags);
>>   		trace_seq_putc(s, '\n');
>>   	}
> 
> Thank you,
> 
>> -- 
>> 2.46.2
>>
> 
> 

Re: [PATCH] ftrace: Fix function name for trampoline

[Tatsuya S]

On 10/11/24 12:02 AM, Steven Rostedt wrote:
> On Thu, 10 Oct 2024 22:02:59 +0900
> Tatsuya S <tatsuya.s2862@gmail.com> wrote:
> 
>> The issue that unrelated function name is shown on stack trace like
>> following even though it should be trampoline code address is caused by
>> the creation of trampoline code in the area where .init.text section
>> of module was freed after module is loaded.
>>
>> bash-1344    [002] .....    43.644608: <stack trace>
>>    => (MODULE INIT FUNCTION)
>>    => vfs_write
>>    => ksys_write
>>    => do_syscall_64
>>    => entry_SYSCALL_64_after_hwframe
>>
>> To resolve this, when function address of stack trace entry is in
>> trampoline, output without looking up symbol name.
>>
>> Signed-off-by: Tatsuya S <tatsuya.s2862@gmail.com>
>> ---
>>   kernel/trace/trace_output.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
>> index 868f2f912f28..32a0858373e2 100644
>> --- a/kernel/trace/trace_output.c
>> +++ b/kernel/trace/trace_output.c
>> @@ -1246,6 +1246,11 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
>>   			break;
>>   
>>   		trace_seq_puts(s, " => ");
>> +		if (is_ftrace_trampoline((*p) + delta)) {

Thank you for review.

> 
> This is not reliable. The output is called when the user reads the trace
> file and the ops may no longer exist.
> 
> The only way to test this is if you call it during the trace. Yes it may
> slow things down a little, but it will be accurate.

OK, I will do it.

> 
> -- Steve
> 
> 
>> +			trace_seq_printf(s, "0x%08lx", (*p) + delta);
>> +			trace_seq_putc(s, '\n');
>> +			continue;
>> +		}
>>   		seq_print_ip_sym(s, (*p) + delta, flags);
>>   		trace_seq_putc(s, '\n');
>>   	}
> 

Thanks