[PATCH v4] tracing: Export live module tracepoint strings in printk_formats

["Cao Ruichuang" <create0818@163.com>]

tracepoint_string() mappings for built-in strings are exported through
printk_formats, but the module loader does not collect __tracepoint_str.
Consequently, user space cannot resolve these module string pointers.

Collect the module section and expose its live address-to-text mappings.
Serialize registration, removal and the full printk_formats seq_file
iteration with btrace_mutex. Remove the range before module memory is
freed; do not retain strings after unload.

Keep trace_is_tracepoint_string() and event string validation unchanged.
Exporting decoder metadata does not require accepting module pointers in
core or other modules' events, nor does it extend the strings' lifetime.

Count successfully retained unique module trace_printk formats so the
iterator can locate the following module string ranges. These formats
remain retained after unload, so the count is not decremented.

Add a sample TRACE_EVENT and a dedicated selftest for multiple exported
strings, event output and mapping removal. Make unload and every residual
string/read-error check fail explicitly.

Fixes: 102c9323c35a ("tracing: Add __tracepoint_string() to export string pointers")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217196
Link: https://lore.kernel.org/r/41e81533-0fd6-49f5-b7c1-b4e172affd2a@suse.com
Link: https://lore.kernel.org/r/20261030173049.6656d506@gandalf.local.home
Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Assisted-by: LLM (GPT-6.1-Sol)
Signed-off-by: Cao Ruichuang <create0818@163.com>
---
Hi Steven,

Thanks for the review.

I have kept this revision focused on exporting live module
tracepoint-string mappings through printk_formats.
I rebased it onto your for-next at d7ff48d92a0f and adjusted the sample
header insertion to follow the new foo_timer_fn event. I reran the builds,
ftrace tests and decoder checks on this base.

> Please use list_for_each_entry_safe().

I now use list_for_each_entry_safe() for removal.

> Please add it to samples.

The example is now in samples/trace_events, with a TRACE_EVENT that
records both test strings.
A dedicated ftrace selftest now checks their mappings, event output and
removal on unload. The unload checks explicitly fail for either remaining
string, a read error, or a failed rmmod. I also added a comment explaining
why the retained-format count is not decremented on module unload.

> The order of headers goes by the length of their names.

There is one scope change from v3: I removed the module-string membership
extension to trace_is_tracepoint_string(). The existing module-owner check
already permits the sample event to print its own module's strings.
Userspace decoding needs the exported mappings, without broadening the
kernel's string safety checks. The module ranges are now used only by
printk_formats and protected throughout iteration by btrace_mutex, so the
RCU list operations and added rcupdate.h include are no longer needed.

I also verified the original use case with trace-cmd 3.3.1: record the
sample event into trace.dat, then decode it with trace-cmd report without
plugins. Both module strings resolve correctly, including after module
unload and in a separate offline decoder on the host. With the export
implementation removed, the same workload records the events but report
shows only addresses. The kernel's sample-event text output still works
in that control, confirming that the membership extension is unnecessary
for this case. W=1 builds, CONFIG_MODULES=n, shell regression fixtures and
the preceding QEMU tests with KASAN, lockdep and tp_printk also passed.

Allowing events owned by the core or another module to dereference these
strings needs protection through the final %s use and a defined unload
policy. My separate local changes use buffer cleanup for that, including
offline CPUs, snapshot synchronization and hotplug protection; they remain
outside this revision.

Would you prefer that additional support as a separate series, or
included with this fix?

Thanks,
Cao Ruichuang

Changes since v3:
- Limit the patch to printk_formats export; leave event string validation
  unchanged.
- Use mutex-only range lifetime and list_for_each_entry_safe() on removal.
- Move the sample to trace_events and add a dedicated regression test.
- Fail explicitly on residual strings, read errors and final rmmod failure.
- Explain the permanently retained format count.
- Rebase onto tracing for-next and follow the new foo_timer_fn sample
  event.

AI assistance (including GPT-6.1-Sol) was used for code changes,
selftests, validation scripts and preparation of this message.
The requests were to address the v3 review feedback, keep this patch
limited to live module string export, and investigate lifetime changes
separately. Validation is summarized below.

Validation on this base:
- W=1 kernel/modules and CONFIG_MODULES=n object builds.
- Shell regression fixtures and QEMU ftrace/load-unload tests with
  KASAN/lockdep.
- Real trace-cmd record/report, module unload and host offline decoding.
- Export-removed negative controls reproduce missing mappings and raw
  addresses.
 include/linux/module.h                        |   2 +
 include/linux/tracepoint.h                    |   9 +-
 kernel/module/main.c                          |   4 +
 kernel/trace/trace_printk.c                   | 111 ++++++++++++++++--
 samples/trace_events/trace-events-sample.c    |   3 +
 samples/trace_events/trace-events-sample.h    |  17 +++
 .../ftrace/test.d/event/tracepoint_string.tc  |  47 ++++++++
 7 files changed, 178 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/event/tracepoint_string.tc

diff --git a/include/linux/module.h b/include/linux/module.h
index 96cc98568..7f7282798 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -509,7 +509,9 @@ struct module {
 #endif
 #ifdef CONFIG_TRACING
 	unsigned int num_trace_bprintk_fmt;
+	unsigned int num_tracepoint_strings;
 	const char **trace_bprintk_fmt_start;
+	const char **tracepoint_strings_start;
 #endif
 #ifdef CONFIG_EVENT_TRACING
 	struct trace_event_call **trace_events;
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index e0d838c9c..7b3ee936a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -508,11 +508,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
  * the ASCII strings they represent.
  *
  * The @str used must be a constant string and persistent as it would not
- * make sense to show a string that no longer exists. But it is still fine
- * to be used with modules, because when modules are unloaded, if they
- * had tracepoints, the ring buffers are cleared too. As long as the string
- * does not change during the life of the module, it is fine to use
- * tracepoint_string() within a module.
+ * make sense to show a string that no longer exists. For modules, the string
+ * mapping is exported while the module is loaded and removed on unload.
+ * Exporting the mapping does not extend the lifetime of the string. The
+ * existing lifetime requirements for strings referenced by events still apply.
  */
 #define tracepoint_string(str)						\
 	({								\
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c1b34dc1e..c9f67bfd3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2740,6 +2740,10 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 	mod->trace_bprintk_fmt_start = section_objs(info, "__trace_printk_fmt",
 					 sizeof(*mod->trace_bprintk_fmt_start),
 					 &mod->num_trace_bprintk_fmt);
+	mod->tracepoint_strings_start =
+		section_objs(info, "__tracepoint_str",
+			     sizeof(*mod->tracepoint_strings_start),
+			     &mod->num_tracepoint_strings);
 #endif
 #ifdef CONFIG_DYNAMIC_FTRACE
 	/* sechdrs[0].sh_size is always zero */
diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
index 98171a239..d82a62df3 100644
--- a/kernel/trace/trace_printk.c
+++ b/kernel/trace/trace_printk.c
@@ -19,15 +19,23 @@
 
 #include "trace.h"
 
+/* Counts unique retained formats; module unload does not decrement this. */
+static int trace_bprintk_fmt_cnt;
+
 #ifdef CONFIG_MODULES
 
 /*
  * modules trace_printk()'s formats are autosaved in struct trace_bprintk_fmt
  * which are queued on trace_bprintk_fmt_list.
+ *
+ * modules tracepoint_string() entries are kept as ranges into the owning
+ * module's __tracepoint_str section and are removed again when the module
+ * goes away.
  */
 static LIST_HEAD(trace_bprintk_fmt_list);
+static LIST_HEAD(tracepoint_str_list);
 
-/* serialize accesses to trace_bprintk_fmt_list */
+/* Serialize module list updates and the full printk_formats iteration. */
 static DEFINE_MUTEX(btrace_mutex);
 
 struct trace_bprintk_fmt {
@@ -35,6 +43,13 @@ struct trace_bprintk_fmt {
 	const char *fmt;
 };
 
+struct tracepoint_mod_str {
+	struct list_head list;
+	struct module *mod;
+	const char **start;
+	unsigned int num;
+};
+
 static inline struct trace_bprintk_fmt *lookup_format(const char *fmt)
 {
 	struct trace_bprintk_fmt *pos;
@@ -75,6 +90,7 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
 			if (fmt) {
 				list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
 				tb_fmt->fmt = fmt;
+				trace_bprintk_fmt_cnt++;
 			} else
 				kfree(tb_fmt);
 		}
@@ -84,16 +100,61 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
 	mutex_unlock(&btrace_mutex);
 }
 
+static void hold_module_tracepoint_strings(struct module *mod)
+{
+	struct tracepoint_mod_str *tp_str;
+
+	if (!mod->num_tracepoint_strings)
+		return;
+
+	tp_str = kmalloc_obj(*tp_str);
+	if (!tp_str)
+		return;
+
+	tp_str->mod = mod;
+	tp_str->start = mod->tracepoint_strings_start;
+	tp_str->num = mod->num_tracepoint_strings;
+
+	guard(mutex)(&btrace_mutex);
+	list_add_tail(&tp_str->list, &tracepoint_str_list);
+}
+
+static void release_module_tracepoint_strings(struct module *mod)
+{
+	struct tracepoint_mod_str *tp_str, *next;
+
+	if (!mod->num_tracepoint_strings)
+		return;
+
+	guard(mutex)(&btrace_mutex);
+	list_for_each_entry_safe(tp_str, next, &tracepoint_str_list, list) {
+		if (tp_str->mod != mod)
+			continue;
+
+		list_del(&tp_str->list);
+		kfree(tp_str);
+		return;
+	}
+}
+
 static int module_trace_bprintk_format_notify(struct notifier_block *self,
 		unsigned long val, void *data)
 {
 	struct module *mod = data;
-	if (mod->num_trace_bprintk_fmt) {
-		const char **start = mod->trace_bprintk_fmt_start;
-		const char **end = start + mod->num_trace_bprintk_fmt;
 
-		if (val == MODULE_STATE_COMING)
+	switch (val) {
+	case MODULE_STATE_COMING:
+		if (mod->num_trace_bprintk_fmt) {
+			const char **start = mod->trace_bprintk_fmt_start;
+			const char **end = start + mod->num_trace_bprintk_fmt;
+
 			hold_module_trace_bprintk_format(start, end);
+		}
+		hold_module_tracepoint_strings(mod);
+		break;
+	case MODULE_STATE_GOING:
+		release_module_tracepoint_strings(mod);
+		break;
 	}
 	return NOTIFY_OK;
 }
@@ -119,7 +180,7 @@ static int module_trace_bprintk_format_notify(struct notifier_block *self,
  * next format in the list.
  */
 static const char **
-find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+find_next_mod_format(loff_t start_index, void *v, const char **fmt, loff_t *pos)
 {
 	struct trace_bprintk_fmt *mod_fmt;
 
@@ -158,6 +219,25 @@ find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
 	return &mod_fmt->fmt;
 }
 
+static const char **
+find_next_mod_tracepoint_str(loff_t start_index, loff_t *pos)
+{
+	struct tracepoint_mod_str *tp_str;
+	loff_t offset = *pos - start_index;
+
+	if (offset < 0)
+		return NULL;
+
+	list_for_each_entry(tp_str, &tracepoint_str_list, list) {
+		if (offset < (loff_t)tp_str->num)
+			return tp_str->start + offset;
+
+		offset -= tp_str->num;
+	}
+
+	return NULL;
+}
+
 static void format_mod_start(void)
 {
 	mutex_lock(&btrace_mutex);
@@ -176,10 +256,17 @@ module_trace_bprintk_format_notify(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 static inline const char **
-find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+find_next_mod_format(loff_t start_index, void *v, const char **fmt, loff_t *pos)
 {
 	return NULL;
 }
+
+static inline const char **
+find_next_mod_tracepoint_str(loff_t start_index, loff_t *pos)
+{
+	return NULL;
+}
+
 static inline void format_mod_start(void) { }
 static inline void format_mod_stop(void) { }
 #endif /* CONFIG_MODULES */
@@ -265,8 +352,8 @@ bool trace_is_tracepoint_string(const char *str)
 static const char **find_next(void *v, loff_t *pos)
 {
 	const char **fmt = v;
-	int start_index;
-	int last_index;
+	loff_t start_index;
+	loff_t last_index;
 
 	start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt;
 
@@ -292,7 +379,11 @@ static const char **find_next(void *v, loff_t *pos)
 		return __start___tracepoint_str + (*pos - last_index);
 
 	start_index += last_index;
-	return find_next_mod_format(start_index, v, fmt, pos);
+	if (*pos - start_index < trace_bprintk_fmt_cnt)
+		return find_next_mod_format(start_index, v, fmt, pos);
+
+	start_index += trace_bprintk_fmt_cnt;
+	return find_next_mod_tracepoint_str(start_index, pos);
 }
 
 static void *
diff --git a/samples/trace_events/trace-events-sample.c b/samples/trace_events/trace-events-sample.c
index ca5d98c36..0164cbdc4 100644
--- a/samples/trace_events/trace-events-sample.c
+++ b/samples/trace_events/trace-events-sample.c
@@ -51,6 +51,9 @@ static void do_simple_thread_func(int cnt, const char *fmt, ...)
 	trace_foo_with_template_print("I have to be different", cnt);
 
 	trace_foo_rel_loc("Hello __rel_loc", cnt, bitmask, current->cpus_ptr);
+
+	trace_foo_tracepoint_str(tracepoint_string("tracepoint_string_test_module_string_1"));
+	trace_foo_tracepoint_str(tracepoint_string("tracepoint_string_test_module_string_2"));
 }
 
 static void simple_thread_func(int cnt)
diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
index 816848a45..021fd0e9d 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -623,6 +623,23 @@ TRACE_EVENT(foo_timer_fn,
 
 	TP_printk("name=%s count=%d", __get_str(name), __entry->count)
 );
+
+TRACE_EVENT(foo_tracepoint_str,
+
+	TP_PROTO(const char *str),
+
+	TP_ARGS(str),
+
+	TP_STRUCT__entry(
+		__field(const char *, str)
+	),
+
+	TP_fast_assign(
+		__entry->str = str;
+	),
+
+	TP_printk("str=%s", __entry->str)
+);
 #endif
 
 /***** NOTICE! The #if protection ends here. *****/
diff --git a/tools/testing/selftests/ftrace/test.d/event/tracepoint_string.tc b/tools/testing/selftests/ftrace/test.d/event/tracepoint_string.tc
new file mode 100644
index 000000000..f0916f34f
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/event/tracepoint_string.tc
@@ -0,0 +1,47 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Test tracepoint strings from a module
+# requires: printk_formats
+
+string1=tracepoint_string_test_module_string_1
+string2=tracepoint_string_test_module_string_2
+event=events/sample-trace/foo_tracepoint_str
+
+rmmod trace-events-sample 2>/dev/null ||:
+if ! modprobe trace-events-sample; then
+    echo "No trace-events-sample module - please make CONFIG_SAMPLE_TRACE_EVENTS=m"
+    exit_unresolved
+fi
+trap 'rmmod trace-events-sample >/dev/null 2>&1 ||:' EXIT
+
+grep -Fq "$string1" printk_formats || exit_fail
+grep -Fq "$string2" printk_formats || exit_fail
+
+echo 1 > "$event/enable"
+echo 1 > tracing_on
+
+# The sample thread emits both strings once per second.
+attempts=100
+while [ "$attempts" -gt 0 ]; do
+    if grep -Fq "foo_tracepoint_str: str=$string1" trace &&
+       grep -Fq "foo_tracepoint_str: str=$string2" trace; then
+        break
+    fi
+    sleep 0.1
+    attempts=$((attempts - 1))
+done
+[ "$attempts" -gt 0 ] || exit_fail
+
+echo 0 > "$event/enable"
+rmmod trace-events-sample || exit_fail
+trap - EXIT
+
+for string in "$string1" "$string2"; do
+    if grep -Fq "$string" printk_formats; then
+        echo "Module string remains after unload: $string"
+        exit_fail
+    else
+        # grep returns 1 for no match; a read error must also fail the test.
+        [ "$?" -eq 1 ] || exit_fail
+    fi
+done

base-commit: d7ff48d92a0f28bc0612da47c054efbc28c19562
-- 
2.47.3