[RFC PATCH 0/3] tracing: Introduce relative stacktrace

[RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Masami Hiramatsu (Google)]

Hi,

This introduces relative stacktrace, which records stacktrace entry as
the offset from _stext instead of raw address. User can enable this
format by setting options/relative-stacktrace.

Basically, this does not change anything for users who are using ftrace
with 'trace' text-formatted interface. This changes how each stacktrace
entry address is stored, so users who is using 'trace_pipe_raw' needs
to change how to decode the stacktrace.

Currently, the stacktrace is stored as raw kernel address. Thus, for
decoding the binary trace data, we need to refer the kallsyms. But this
is not useful on the platform which prohibits to access /proc/kallsyms
for security reason. Since KASLR will change the kernel text address,
we can not decode symbols without kallsyms in userspace.

On the other hand, if we record the stacktrace entries in the offset
from _stext, we can use System.map file to decode it. This is also good
for the stacktrace in the persistent ring buffer, because we don't need
to save the kallsyms before crash anymore.

The problem is to decode the address in the modules because it will be
loaded in the different place. To solve this issue, I also introduced
'module_text_offsets' event, which records module's text and init_text
info as the offset from _stext when loading it. User can store this
event in the (another) persistent ring buffer for decoding.

Thank you,

---

Masami Hiramatsu (Google) (3):
      tracing: Record stacktrace as the offset from _stext
      tracing: Introduce "rel_stack" option
      modules: tracing: Add module_text_offsets event


 include/trace/events/module.h |   40 ++++++++++++++++++++++++++++++++++++++++
 kernel/module/main.c          |    1 +
 kernel/trace/trace.c          |   11 ++++++++++-
 kernel/trace/trace.h          |    2 ++
 kernel/trace/trace_entries.h  |   22 ++++++++++++++++++++++
 kernel/trace/trace_output.c   |   35 +++++++++++++++++++++++++++++++----
 6 files changed, 106 insertions(+), 5 deletions(-)

--
Signature

[RFC PATCH 1/3] tracing: Record stacktrace as the offset from _stext

[Masami Hiramatsu (Google)]

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

Record kernel stacktrace as the offset from _stext so that it does
not affected by KASLR.

For the persistent ring buffer, decoding the stacktrace entries
requires kallsyms in the previous boot because the kernel symbols
will have random offset for each boot by KASLR. But this is not
useful because we always need to save the kallsyms. Alternatively,
we can record the stacktrace entries as the offset value from
_stext. In this case, we can use System.map or nm for the vmlinux
to decode the entries.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace.c        |    6 ++++++
 kernel/trace/trace_output.c |    2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1496a5ac33ae..8e86a43b368c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2973,8 +2973,14 @@ static void __ftrace_trace_stack(struct trace_array *tr,
 		for (int i = 0; i < nr_entries; i++) {
 			if (calls[i] >= tramp_start && calls[i] < tramp_end)
 				calls[i] = FTRACE_TRAMPOLINE_MARKER;
+			else
+				calls[i] -= (unsigned long)_stext;
 		}
 	}
+#else
+	/* Adjsut entries as the offset from _stext, instead of raw address. */
+	for (int i = 0; i < nr_entries; i++)
+		fstack->calls[i] -= (unsigned long)_stext;
 #endif
 
 	event = __trace_buffer_lock_reserve(buffer, TRACE_STACK,
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 03d56f711ad1..497872df48f6 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1248,7 +1248,7 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
 	struct trace_seq *s = &iter->seq;
 	unsigned long *p;
 	unsigned long *end;
-	long delta = iter->tr->text_delta;
+	long delta = (unsigned long)_stext + iter->tr->text_delta;
 
 	trace_assign_type(field, iter->ent);
 	end = (unsigned long *)((long)iter->ent + iter->ent_size);

[RFC PATCH 2/3] tracing: Introduce "rel_stack" option

[Masami Hiramatsu (Google)]

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

Since the relative offset stacktrace requires a special module loading
events to decode binary, it should be an optional for normal use case.
User can enable this feature via `options/relative-stacktrace`.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace.c         |   13 ++++++++-----
 kernel/trace/trace.h         |    2 ++
 kernel/trace/trace_entries.h |   22 ++++++++++++++++++++++
 kernel/trace/trace_output.c  |   37 ++++++++++++++++++++++++++++++++-----
 4 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8e86a43b368c..b4da5e29957f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2926,6 +2926,7 @@ static void __ftrace_trace_stack(struct trace_array *tr,
 	struct ftrace_stack *fstack;
 	struct stack_entry *entry;
 	int stackidx;
+	int type;
 
 	/*
 	 * Add one, for this function and the call to save_stack_trace()
@@ -2937,6 +2938,7 @@ static void __ftrace_trace_stack(struct trace_array *tr,
 #endif
 
 	preempt_disable_notrace();
+	type = (tr->trace_flags & TRACE_ITER_REL_STACK_BIT) ? TRACE_REL_STACK : TRACE_STACK;
 
 	stackidx = __this_cpu_inc_return(ftrace_stack_reserve) - 1;
 
@@ -2973,17 +2975,18 @@ static void __ftrace_trace_stack(struct trace_array *tr,
 		for (int i = 0; i < nr_entries; i++) {
 			if (calls[i] >= tramp_start && calls[i] < tramp_end)
 				calls[i] = FTRACE_TRAMPOLINE_MARKER;
-			else
+			else if (type == TRACE_REL_STACK)
 				calls[i] -= (unsigned long)_stext;
 		}
 	}
 #else
-	/* Adjsut entries as the offset from _stext, instead of raw address. */
-	for (int i = 0; i < nr_entries; i++)
-		fstack->calls[i] -= (unsigned long)_stext;
+	if (type == TRACE_REL_STACK)
+		/* Adjsut entries as the offset from _stext, instead of raw address. */
+		for (int i = 0; i < nr_entries; i++)
+			fstack->calls[i] -= (unsigned long)_stext;
 #endif
 
-	event = __trace_buffer_lock_reserve(buffer, TRACE_STACK,
+	event = __trace_buffer_lock_reserve(buffer, type,
 				    struct_size(entry, caller, nr_entries),
 				    trace_ctx);
 	if (!event)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9c21ba45b7af..602aea0ec69a 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -55,6 +55,7 @@ enum trace_type {
 	TRACE_TIMERLAT,
 	TRACE_RAW_DATA,
 	TRACE_FUNC_REPEATS,
+	TRACE_REL_STACK,
 
 	__TRACE_LAST_TYPE,
 };
@@ -1350,6 +1351,7 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
 		C(TRACE_PRINTK,		"trace_printk_dest"),	\
 		C(PAUSE_ON_TRACE,	"pause-on-trace"),	\
 		C(HASH_PTR,		"hash-ptr"),	/* Print hashed pointer */ \
+		C(REL_STACK,	"relative-stacktrace"),	\
 		FUNCTION_FLAGS					\
 		FGRAPH_FLAGS					\
 		STACK_FLAGS					\
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index fbfb396905a6..7769f95b70fe 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -229,6 +229,28 @@ FTRACE_ENTRY(kernel_stack, stack_entry,
 		 (void *)__entry->caller[6], (void *)__entry->caller[7])
 );
 
+FTRACE_ENTRY_DUP(kernel_rel_stack, stack_entry,
+
+	TRACE_REL_STACK,
+
+	F_STRUCT(
+		__field(	int,		size	)
+		__stack_array(	unsigned long,	caller,	FTRACE_STACK_ENTRIES, size)
+	),
+
+	F_printk("\t=> %ps\n\t=> %ps\n\t=> %ps\n"
+		 "\t=> %ps\n\t=> %ps\n\t=> %ps\n"
+		 "\t=> %ps\n\t=> %ps\n",
+		 (void *)__entry->caller[0] + (unsigned long)_stext,
+		 (void *)__entry->caller[1] + (unsigned long)_stext,
+		 (void *)__entry->caller[2] + (unsigned long)_stext,
+		 (void *)__entry->caller[3] + (unsigned long)_stext,
+		 (void *)__entry->caller[4] + (unsigned long)_stext,
+		 (void *)__entry->caller[5] + (unsigned long)_stext,
+		 (void *)__entry->caller[6] + (unsigned long)_stext,
+		 (void *)__entry->caller[7] + (unsigned long)_stext)
+);
+
 FTRACE_ENTRY(user_stack, userstack_entry,
 
 	TRACE_USER_STACK,
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 497872df48f6..47e4ab549e81 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1239,16 +1239,17 @@ static struct trace_event trace_wake_event = {
 	.funcs		= &trace_wake_funcs,
 };
 
-/* TRACE_STACK */
-
-static enum print_line_t trace_stack_print(struct trace_iterator *iter,
-					   int flags, struct trace_event *event)
+static enum print_line_t trace_kernel_stack_print(struct trace_iterator *iter,
+					   int flags, struct trace_event *event, bool relative)
 {
 	struct stack_entry *field;
 	struct trace_seq *s = &iter->seq;
 	unsigned long *p;
 	unsigned long *end;
-	long delta = (unsigned long)_stext + iter->tr->text_delta;
+	long delta = iter->tr->text_delta;
+
+	if (relative)
+		delta += (unsigned long)_stext;
 
 	trace_assign_type(field, iter->ent);
 	end = (unsigned long *)((long)iter->ent + iter->ent_size);
@@ -1272,6 +1273,14 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
 	return trace_handle_return(s);
 }
 
+/* TRACE_STACK */
+
+static enum print_line_t trace_stack_print(struct trace_iterator *iter,
+					   int flags, struct trace_event *event)
+{
+	return trace_kernel_stack_print(iter, flags, event, false);
+}
+
 static struct trace_event_functions trace_stack_funcs = {
 	.trace		= trace_stack_print,
 };
@@ -1281,6 +1290,23 @@ static struct trace_event trace_stack_event = {
 	.funcs		= &trace_stack_funcs,
 };
 
+/* TRACE_REL_STACK */
+
+static enum print_line_t trace_rel_stack_print(struct trace_iterator *iter,
+					   int flags, struct trace_event *event)
+{
+	return trace_kernel_stack_print(iter, flags, event, true);
+}
+
+static struct trace_event_functions trace_rel_stack_funcs = {
+	.trace		= trace_rel_stack_print,
+};
+
+static struct trace_event trace_rel_stack_event = {
+	.type		= TRACE_REL_STACK,
+	.funcs		= &trace_rel_stack_funcs,
+};
+
 /* TRACE_USER_STACK */
 static enum print_line_t trace_user_stack_print(struct trace_iterator *iter,
 						int flags, struct trace_event *event)
@@ -1724,6 +1750,7 @@ static struct trace_event *events[] __initdata = {
 	&trace_ctx_event,
 	&trace_wake_event,
 	&trace_stack_event,
+	&trace_rel_stack_event,
 	&trace_user_stack_event,
 	&trace_bputs_event,
 	&trace_bprint_event,

[RFC PATCH 3/3] modules: tracing: Add module_text_offsets event

[Masami Hiramatsu (Google)]

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

Add new module_text_offsets event which records the offset of module
text and init_text section address and size.

This information is required for decoding the relative stacktrace
entries for modules.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 include/trace/events/module.h |   40 ++++++++++++++++++++++++++++++++++++++++
 kernel/module/main.c          |    1 +
 2 files changed, 41 insertions(+)

diff --git a/include/trace/events/module.h b/include/trace/events/module.h
index e5a006be9dc6..1e5c84cc8d15 100644
--- a/include/trace/events/module.h
+++ b/include/trace/events/module.h
@@ -47,6 +47,46 @@ TRACE_EVENT(module_load,
 	TP_printk("%s %s", __get_str(name), show_module_flags(__entry->taints))
 );
 
+/* Module address offset from _stext */
+TRACE_EVENT(module_text_offsets,
+
+	TP_PROTO(struct module *mod),
+
+	TP_ARGS(mod),
+
+	TP_STRUCT__entry(
+		__string(	name,		mod->name	)
+		__field(	unsigned long,	text_offset	)
+		__field(	unsigned int,	text_size	)
+		__field(	unsigned long,	init_offset	)
+		__field(	unsigned int,	init_size	)
+	),
+
+	TP_fast_assign(
+		if (mod->mem[MOD_TEXT].size) {
+			__entry->text_offset = (unsigned long)mod->mem[MOD_TEXT].base
+					     - (unsigned long)_stext;
+			__entry->text_size = mod->mem[MOD_TEXT].size;
+		} else {
+			__entry->text_offset = 0;
+			__entry->text_size = 0;
+		}
+		if (mod->mem[MOD_INIT_TEXT].size) {
+			__entry->init_offset = (unsigned long)mod->mem[MOD_INIT_TEXT].base
+					     - (unsigned long)_stext;
+			__entry->init_size = mod->mem[MOD_INIT_TEXT].size;
+		} else {
+			__entry->init_offset = 0;
+			__entry->init_size = 0;
+		}
+		__assign_str(name);
+	),
+
+	TP_printk("%s text_offset=0x%lx text_size=%d init_offset=0x%lx init_size=%d",
+		  __get_str(name), __entry->text_offset, __entry->text_size,
+		  __entry->init_offset, __entry->init_size)
+);
+
 TRACE_EVENT(module_free,
 
 	TP_PROTO(struct module *mod),
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5399c182b3cb..9f1ca8730a71 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3372,6 +3372,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
 
 	/* Done! */
 	trace_module_load(mod);
+	trace_module_text_offsets(mod);
 
 	return do_init_module(mod);
 

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Mathieu Desnoyers]

On 2025-01-28 10:36, Masami Hiramatsu (Google) wrote:
> Hi,
> 
> This introduces relative stacktrace, which records stacktrace entry as
> the offset from _stext instead of raw address. User can enable this
> format by setting options/relative-stacktrace.
> 
> Basically, this does not change anything for users who are using ftrace
> with 'trace' text-formatted interface. This changes how each stacktrace
> entry address is stored, so users who is using 'trace_pipe_raw' needs
> to change how to decode the stacktrace.
> 
> Currently, the stacktrace is stored as raw kernel address. Thus, for
> decoding the binary trace data, we need to refer the kallsyms. But this
> is not useful on the platform which prohibits to access /proc/kallsyms
> for security reason. Since KASLR will change the kernel text address,
> we can not decode symbols without kallsyms in userspace.
> 
> On the other hand, if we record the stacktrace entries in the offset
> from _stext, we can use System.map file to decode it. This is also good
> for the stacktrace in the persistent ring buffer, because we don't need
> to save the kallsyms before crash anymore.
> 
> The problem is to decode the address in the modules because it will be
> loaded in the different place. To solve this issue, I also introduced
> 'module_text_offsets' event, which records module's text and init_text
> info as the offset from _stext when loading it. User can store this
> event in the (another) persistent ring buffer for decoding.

This does not handle the situation where a module is already loaded
before tracing starts. In LTTng we have a statedump facility for this,
where we can iterate on all modules at trace start and dump the relevant
information.

You may want to consider a similar approach for other tracers.

Thanks,

Mathieu

> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (Google) (3):
>        tracing: Record stacktrace as the offset from _stext
>        tracing: Introduce "rel_stack" option
>        modules: tracing: Add module_text_offsets event
> 
> 
>   include/trace/events/module.h |   40 ++++++++++++++++++++++++++++++++++++++++
>   kernel/module/main.c          |    1 +
>   kernel/trace/trace.c          |   11 ++++++++++-
>   kernel/trace/trace.h          |    2 ++
>   kernel/trace/trace_entries.h  |   22 ++++++++++++++++++++++
>   kernel/trace/trace_output.c   |   35 +++++++++++++++++++++++++++++++----
>   6 files changed, 106 insertions(+), 5 deletions(-)
> 
> --
> Signature

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: [RFC PATCH 2/3] tracing: Introduce "rel_stack" option

[Steven Rostedt]

On Wed, 29 Jan 2025 00:37:15 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Since the relative offset stacktrace requires a special module loading
> events to decode binary, it should be an optional for normal use case.
> User can enable this feature via `options/relative-stacktrace`.
> 

Let's make this an entirely new event and not based on an option. Reason
being, there's no way for libtraceevent to know which this is. We could
even have a mixture of these in the trace.

Instead of an option, we can add a new trigger: stacktrace_rel that will do
a relative stack trace. And the event can be kernel_stack_rel

Then it can be enabled via:

  echo 'stacktrace_rel if prev_state & 3' > events/sched/sched_switch/trigger

-- Steve

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Steven Rostedt]

On Tue, 28 Jan 2025 10:46:21 -0500
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> This does not handle the situation where a module is already loaded
> before tracing starts. In LTTng we have a statedump facility for this,
> where we can iterate on all modules at trace start and dump the relevant
> information.
> 
> You may want to consider a similar approach for other tracers.

Last night Masami and I were talking about this. The idea I was thinking of
was to simply have a module load notifier that would add modules to an
array. It would only keep track of loaded modules, and when the trace hit,
if the address was outside of core text, it would search the array for the
module, and use that. When a module is removed, it would also be removed
from the array. We currently do not support tracing module removal (if the
module is traced, the buffers are cleared when the module is removed).

If it is a module address, set the MSB, and for 32 bit machines use the
next 7 bits as an index into the module array, and for 64 bit machines, use
the next 10 bits as an index. This would be exposed in the format file for
the kernel_stack_rel event, so if these numbers change, user space can cope
with it. In fact, it would need to use the format file to distinguish the
32 bit and 64 bit values.

That is, a stack trace will contain addresses that are core kernel simply
subtracted from ".text", and the modules address would have the MSB set,
the next bits would be an index into that array that holds the module
information, and the address would be the address minus the module address
where it was loaded.

This way we do not need to save the information from any events. Also, for
the persistent ring buffer, this array could live in that memory, so that
it will be available on the next boot.

-- Steve

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Mathieu Desnoyers]

On 2025-01-28 11:27, Steven Rostedt wrote:
> On Tue, 28 Jan 2025 10:46:21 -0500
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> This does not handle the situation where a module is already loaded
>> before tracing starts. In LTTng we have a statedump facility for this,
>> where we can iterate on all modules at trace start and dump the relevant
>> information.
>>
>> You may want to consider a similar approach for other tracers.
> 
> Last night Masami and I were talking about this. The idea I was thinking of
> was to simply have a module load notifier that would add modules to an
> array. It would only keep track of loaded modules, and when the trace hit,
> if the address was outside of core text, it would search the array for the
> module, and use that. When a module is removed, it would also be removed
> from the array. We currently do not support tracing module removal (if the
> module is traced, the buffers are cleared when the module is removed).

I'm trying to wrap my head around what you are trying to achieve here.

So AFAIU you are aiming to store the relative offset from kernel _text
and module base text address into the traced events rather than the
actual address.

Based on Masami's cover letter, this appears to be  done to make sure
users can get to this base+offset information even if they cannot read
kallsyms.

Why make the tracing fast path more complex for a simple matter of
accessing this base address information ?

All you need to have to convert from kernel address to base + offset is:

- The kernel _text base address,
- Each loaded module text base address,
- Unloaded modules events to prune this information.

What is wrong with simply exporting this base address information in the
trace buffers rather than rely on kallsyms, and deal with the conversion
to module name / base+offset at post-processing ?

Thanks,

Mathieu

> 
> If it is a module address, set the MSB, and for 32 bit machines use the
> next 7 bits as an index into the module array, and for 64 bit machines, use
> the next 10 bits as an index. This would be exposed in the format file for
> the kernel_stack_rel event, so if these numbers change, user space can cope
> with it. In fact, it would need to use the format file to distinguish the
> 32 bit and 64 bit values.
> 
> That is, a stack trace will contain addresses that are core kernel simply
> subtracted from ".text", and the modules address would have the MSB set,
> the next bits would be an index into that array that holds the module
> information, and the address would be the address minus the module address
> where it was loaded.
> 
> This way we do not need to save the information from any events. Also, for
> the persistent ring buffer, this array could live in that memory, so that
> it will be available on the next boot.





-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Steven Rostedt]

On Tue, 28 Jan 2025 11:46:25 -0500
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> I'm trying to wrap my head around what you are trying to achieve here.
> 
> So AFAIU you are aiming to store the relative offset from kernel _text
> and module base text address into the traced events rather than the
> actual address.
> 
> Based on Masami's cover letter, this appears to be  done to make sure
> users can get to this base+offset information even if they cannot read
> kallsyms.
> 
> Why make the tracing fast path more complex for a simple matter of
> accessing this base address information ?
> 
> All you need to have to convert from kernel address to base + offset is:
> 
> - The kernel _text base address,
> - Each loaded module text base address,
> - Unloaded modules events to prune this information.
> 
> What is wrong with simply exporting this base address information in the
> trace buffers rather than rely on kallsyms, and deal with the conversion
> to module name / base+offset at post-processing ?

Hmm, we could probably get away with that too. I think we were focused on
kallsyms, where we wanted a way to not have to distinguish between current
boot info and previous boot info. But when we started pulling in the module
info, it may be possible to do a post processing.

I have said in the past that I wanted module information in the persistent
memory. By doing that this may not be needed. I'll look into it.

-- Steve

Re: [RFC PATCH 2/3] tracing: Introduce "rel_stack" option

[Masami Hiramatsu]

On Tue, 28 Jan 2025 11:07:00 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 29 Jan 2025 00:37:15 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Since the relative offset stacktrace requires a special module loading
> > events to decode binary, it should be an optional for normal use case.
> > User can enable this feature via `options/relative-stacktrace`.
> > 
> 
> Let's make this an entirely new event and not based on an option. Reason
> being, there's no way for libtraceevent to know which this is. We could
> even have a mixture of these in the trace.
> 
> Instead of an option, we can add a new trigger: stacktrace_rel that will do
> a relative stack trace. And the event can be kernel_stack_rel
> 
> Then it can be enabled via:
> 
>   echo 'stacktrace_rel if prev_state & 3' > events/sched/sched_switch/trigger

Oh, I thought this was another feature, adding a new trigger action.
Can't we do this with the event defined by FTRACE_ENTRY_DUP()?

Thank you,

> 
> -- Steve


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

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Masami Hiramatsu]

On Tue, 28 Jan 2025 11:27:33 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 28 Jan 2025 10:46:21 -0500
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
> > This does not handle the situation where a module is already loaded
> > before tracing starts. In LTTng we have a statedump facility for this,
> > where we can iterate on all modules at trace start and dump the relevant
> > information.
> > 
> > You may want to consider a similar approach for other tracers.
> 
> Last night Masami and I were talking about this. The idea I was thinking of
> was to simply have a module load notifier that would add modules to an
> array. It would only keep track of loaded modules, and when the trace hit,
> if the address was outside of core text, it would search the array for the
> module, and use that. When a module is removed, it would also be removed
> from the array. We currently do not support tracing module removal (if the
> module is traced, the buffers are cleared when the module is removed).

Actually, we already have similar info in /proc/modules. Of course it is
not persistent.

> If it is a module address, set the MSB, and for 32 bit machines use the
> next 7 bits as an index into the module array, and for 64 bit machines, use
> the next 10 bits as an index.

I thought 7 bits were not enough because some stacktrace were kept after
the module was unloaded. Of course we can ignore such case (anyway current
"live" stacktrace does not care such case too).


> This would be exposed in the format file for
> the kernel_stack_rel event, so if these numbers change, user space can cope
> with it. In fact, it would need to use the format file to distinguish the
> 32 bit and 64 bit values.

Yeah, that can simplify the userspace. But the problem of using relative
address from the module .text is that it has bigger overhead to find the
module for each stacktrace entry.

Thank you,

> 
> That is, a stack trace will contain addresses that are core kernel simply
> subtracted from ".text", and the modules address would have the MSB set,
> the next bits would be an index into that array that holds the module
> information, and the address would be the address minus the module address
> where it was loaded.
> 
> This way we do not need to save the information from any events. Also, for
> the persistent ring buffer, this array could live in that memory, so that
> it will be available on the next boot.
> 
> -- Steve
> 


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

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Masami Hiramatsu]

On Tue, 28 Jan 2025 10:46:21 -0500
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> On 2025-01-28 10:36, Masami Hiramatsu (Google) wrote:
> > Hi,
> > 
> > This introduces relative stacktrace, which records stacktrace entry as
> > the offset from _stext instead of raw address. User can enable this
> > format by setting options/relative-stacktrace.
> > 
> > Basically, this does not change anything for users who are using ftrace
> > with 'trace' text-formatted interface. This changes how each stacktrace
> > entry address is stored, so users who is using 'trace_pipe_raw' needs
> > to change how to decode the stacktrace.
> > 
> > Currently, the stacktrace is stored as raw kernel address. Thus, for
> > decoding the binary trace data, we need to refer the kallsyms. But this
> > is not useful on the platform which prohibits to access /proc/kallsyms
> > for security reason. Since KASLR will change the kernel text address,
> > we can not decode symbols without kallsyms in userspace.
> > 
> > On the other hand, if we record the stacktrace entries in the offset
> > from _stext, we can use System.map file to decode it. This is also good
> > for the stacktrace in the persistent ring buffer, because we don't need
> > to save the kallsyms before crash anymore.
> > 
> > The problem is to decode the address in the modules because it will be
> > loaded in the different place. To solve this issue, I also introduced
> > 'module_text_offsets' event, which records module's text and init_text
> > info as the offset from _stext when loading it. User can store this
> > event in the (another) persistent ring buffer for decoding.
> 
> This does not handle the situation where a module is already loaded
> before tracing starts. In LTTng we have a statedump facility for this,
> where we can iterate on all modules at trace start and dump the relevant
> information.

Thanks for the comment!
For the persistent ring buffer, I think we can enable this event in early
boot stage which allows us to store it. (But this overwrites the previous
data, hmm, we need A-B buffer...)

Thank you,

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

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Masami Hiramatsu]

On Tue, 28 Jan 2025 10:46:21 -0500
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> On 2025-01-28 10:36, Masami Hiramatsu (Google) wrote:
> > Hi,
> > 
> > This introduces relative stacktrace, which records stacktrace entry as
> > the offset from _stext instead of raw address. User can enable this
> > format by setting options/relative-stacktrace.
> > 
> > Basically, this does not change anything for users who are using ftrace
> > with 'trace' text-formatted interface. This changes how each stacktrace
> > entry address is stored, so users who is using 'trace_pipe_raw' needs
> > to change how to decode the stacktrace.
> > 
> > Currently, the stacktrace is stored as raw kernel address. Thus, for
> > decoding the binary trace data, we need to refer the kallsyms. But this
> > is not useful on the platform which prohibits to access /proc/kallsyms
> > for security reason. Since KASLR will change the kernel text address,
> > we can not decode symbols without kallsyms in userspace.
> > 
> > On the other hand, if we record the stacktrace entries in the offset
> > from _stext, we can use System.map file to decode it. This is also good
> > for the stacktrace in the persistent ring buffer, because we don't need
> > to save the kallsyms before crash anymore.
> > 
> > The problem is to decode the address in the modules because it will be
> > loaded in the different place. To solve this issue, I also introduced
> > 'module_text_offsets' event, which records module's text and init_text
> > info as the offset from _stext when loading it. User can store this
> > event in the (another) persistent ring buffer for decoding.
> 
> This does not handle the situation where a module is already loaded
> before tracing starts. In LTTng we have a statedump facility for this,
> where we can iterate on all modules at trace start and dump the relevant
> information.

BTW, if we only covers the crash by watchdog or oops, we can dump the
all loaded module info at the panic code.

Thank you,



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

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Masami Hiramatsu]

On Tue, 28 Jan 2025 11:46:25 -0500
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> On 2025-01-28 11:27, Steven Rostedt wrote:
> > On Tue, 28 Jan 2025 10:46:21 -0500
> > Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> > 
> >> This does not handle the situation where a module is already loaded
> >> before tracing starts. In LTTng we have a statedump facility for this,
> >> where we can iterate on all modules at trace start and dump the relevant
> >> information.
> >>
> >> You may want to consider a similar approach for other tracers.
> > 
> > Last night Masami and I were talking about this. The idea I was thinking of
> > was to simply have a module load notifier that would add modules to an
> > array. It would only keep track of loaded modules, and when the trace hit,
> > if the address was outside of core text, it would search the array for the
> > module, and use that. When a module is removed, it would also be removed
> > from the array. We currently do not support tracing module removal (if the
> > module is traced, the buffers are cleared when the module is removed).
> 
> I'm trying to wrap my head around what you are trying to achieve here.
> 
> So AFAIU you are aiming to store the relative offset from kernel _text
> and module base text address into the traced events rather than the
> actual address.
> 
> Based on Masami's cover letter, this appears to be  done to make sure
> users can get to this base+offset information even if they cannot read
> kallsyms.
> 
> Why make the tracing fast path more complex for a simple matter of
> accessing this base address information ?
> 
> All you need to have to convert from kernel address to base + offset is:
> 
> - The kernel _text base address,
> - Each loaded module text base address,
> - Unloaded modules events to prune this information.
> 
> What is wrong with simply exporting this base address information in the
> trace buffers rather than rely on kallsyms, and deal with the conversion
> to module name / base+offset at post-processing ?

Hmm, that also works if we only consider the kallsyms access. But that
means to export KASLR information in the trace buffer. We need to check
it is OK.

My another concern is how to handle this stacktrace on live system. The
stacktrace has to be handled in both crash and live trace, but in both case
we need to consider not leaking KASLR offset.

Hmm, for avoiding the security concern, as Steve said, we may need to save
the module relative address, which may introduce a bit more overhead, but
it should be safer.

Anyway, this v1 may be able to leak the KASLR offset (or estimate it easier).
I think we have 2 options; (A) as Mathieu pointed, expose the offset
information via trace buffer. (B) as Steve pointed, fully relative offset
in stacktrace.

For the crash analysis, if we expose the offset information only when the
machine get a panic, (A) is safe because no one will continue to work. But
this may not work with live system (if we can not access to kallsyms).

(B) is always OK, but it takes more overhead to save the stacktrace.
(but how much it increase, we need to measure that)

Thank you,

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

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Steven Rostedt]

On Wed, 29 Jan 2025 09:58:19 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> Hmm, that also works if we only consider the kallsyms access. But that
> means to export KASLR information in the trace buffer. We need to check
> it is OK.

If they say we can't have KASLR information in the ring buffer then
that is pretty much a brick wall, and we are done with this. The best
we can do is to prevent reading the current trace buffer. But honestly,
we want that too. Heck, already get kernel stack traces from perfetto
right? That has KASLR information doesn't it?

> 
> My another concern is how to handle this stacktrace on live system. The
> stacktrace has to be handled in both crash and live trace, but in both case
> we need to consider not leaking KASLR offset.

I don't think we do.

> 
> Hmm, for avoiding the security concern, as Steve said, we may need to save
> the module relative address, which may introduce a bit more overhead, but
> it should be safer.

Actually, if we save the addresses of where the modules are in the
persistent ring buffer, and expose the addresses only if they are from
the previous boot (if it's the current boot, it just says "current"),
then we can decipher the modules from the previous boot.

> 
> Anyway, this v1 may be able to leak the KASLR offset (or estimate it easier).
> I think we have 2 options; (A) as Mathieu pointed, expose the offset
> information via trace buffer. (B) as Steve pointed, fully relative offset
> in stacktrace.

It should be fine to read the full offsets. Again, perf already does this.

-- Steve

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Masami Hiramatsu]

On Tue, 28 Jan 2025 20:09:38 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 29 Jan 2025 09:58:19 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > Hmm, that also works if we only consider the kallsyms access. But that
> > means to export KASLR information in the trace buffer. We need to check
> > it is OK.
> 
> If they say we can't have KASLR information in the ring buffer then
> that is pretty much a brick wall, and we are done with this. The best
> we can do is to prevent reading the current trace buffer. But honestly,
> we want that too. Heck, already get kernel stack traces from perfetto
> right? That has KASLR information doesn't it?

I read the perfetto callstack feature, but it seems to support user
space callstack.

https://perfetto.dev/docs/quickstart/callstack-sampling

> 
> > 
> > My another concern is how to handle this stacktrace on live system. The
> > stacktrace has to be handled in both crash and live trace, but in both case
> > we need to consider not leaking KASLR offset.
> 
> I don't think we do.

I meant that my [PATCH 3/3] can do it intermediately (not directly).
So I think your idea (storing relative offset from module) is better.

> 
> > 
> > Hmm, for avoiding the security concern, as Steve said, we may need to save
> > the module relative address, which may introduce a bit more overhead, but
> > it should be safer.
> 
> Actually, if we save the addresses of where the modules are in the
> persistent ring buffer, and expose the addresses only if they are from
> the previous boot (if it's the current boot, it just says "current"),
> then we can decipher the modules from the previous boot.

OK, but when would we save it? it is OK to do it in panic()?

> 
> > 
> > Anyway, this v1 may be able to leak the KASLR offset (or estimate it easier).
> > I think we have 2 options; (A) as Mathieu pointed, expose the offset
> > information via trace buffer. (B) as Steve pointed, fully relative offset
> > in stacktrace.
> 
> It should be fine to read the full offsets. Again, perf already does this.

Indeed. Hmm, I need to know how perf solve this limitation.

Thank you,

> 
> -- Steve


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

Re: [RFC PATCH 0/3] tracing: Introduce relative stacktrace

[Steven Rostedt]

On Wed, 29 Jan 2025 16:25:38 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:


> > Actually, if we save the addresses of where the modules are in the
> > persistent ring buffer, and expose the addresses only if they are from
> > the previous boot (if it's the current boot, it just says "current"),
> > then we can decipher the modules from the previous boot.  
> 
> OK, but when would we save it? it is OK to do it in panic()?

It would be saved in the persistent memory region, and added when a module
is loaded. That is, it will already be recorded when a panic() occurs.

-- Steve