[PATCH] tracing: Fix multiple issues in trace_printk module handling

[PATCH] tracing: Fix multiple issues in trace_printk module handling

[Fidal Palamparambil]

From: Fidal palamparambil <rootuserhere@gmail.com>

This commit addresses several bugs and potential issues in the
trace_printk module format handling code:

1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
   proper cleanup when format string allocation fails by setting
   tb_fmt to NULL after freeing it.

2. NULL pointer dereference prevention: Added NULL checks in
   t_show() function before dereferencing format pointers.

3. Input validation: Added NULL check in trace_is_tracepoint_string()
   to prevent potential NULL pointer dereference.

4. Type safety: Fixed type casting in t_show() to use proper
   unsigned long casting for pointer arithmetic.

5. Error handling: Improved error checking in
   init_trace_printk_function_export() by using IS_ERR() to check
   dentry pointer.

6. Code robustness: Added additional pointer validation throughout
   the code to handle potential edge cases.

7. Memory safety: Ensured consistent handling of format pointers
   when memory allocation failures occur.

These fixes improve the stability and reliability of the trace_printk
infrastructure, particularly when dealing with module loading/unloading
and format string management.

Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
 kernel/trace/trace_printk.c      |  33 +--
 kernel/trace/trace_printk.c.orig | 400 +++++++++++++++++++++++++++++++
 2 files changed, 419 insertions(+), 14 deletions(-)
 create mode 100644 kernel/trace/trace_printk.c.orig

diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
index 29f6e95439b6..cb962c6c02f8 100644
--- a/kernel/trace/trace_printk.c
+++ b/kernel/trace/trace_printk.c
@@ -76,10 +76,12 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
 				list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
 				strcpy(fmt, *iter);
 				tb_fmt->fmt = fmt;
-			} else
+			} else {
 				kfree(tb_fmt);
+				tb_fmt = NULL;
+			}
 		}
-		*iter = fmt;
+		*iter = tb_fmt ? tb_fmt->fmt : NULL;
 
 	}
 	mutex_unlock(&btrace_mutex);
@@ -253,7 +255,10 @@ EXPORT_SYMBOL_GPL(__ftrace_vprintk);
 
 bool trace_is_tracepoint_string(const char *str)
 {
-	const char **ptr = __start___tracepoint_str;
+	const char **ptr;
+
+	if (!str)
+		return false;
 
 	for (ptr = __start___tracepoint_str; ptr < __stop___tracepoint_str; ptr++) {
 		if (str == *ptr)
@@ -311,19 +316,19 @@ static void *t_next(struct seq_file *m, void * v, loff_t *pos)
 static int t_show(struct seq_file *m, void *v)
 {
 	const char **fmt = v;
-	const char *str = *fmt;
-	int i;
+	const char *str;
 
-	if (!*fmt)
+	if (!fmt || !*fmt)
 		return 0;
 
-	seq_printf(m, "0x%lx : \"", *(unsigned long *)fmt);
+	str = *fmt;
+	seq_printf(m, "0x%lx : \"", (unsigned long)fmt);
 
 	/*
 	 * Tabs and new lines need to be converted.
 	 */
-	for (i = 0; str[i]; i++) {
-		switch (str[i]) {
+	for (; *str; str++) {
+		switch (*str) {
 		case '\n':
 			seq_puts(m, "\\n");
 			break;
@@ -337,7 +342,7 @@ static int t_show(struct seq_file *m, void *v)
 			seq_puts(m, "\\\"");
 			break;
 		default:
-			seq_putc(m, str[i]);
+			seq_putc(m, *str);
 		}
 	}
 	seq_puts(m, "\"\n");
@@ -378,10 +383,10 @@ static const struct file_operations ftrace_formats_fops = {
 
 static __init int init_trace_printk_function_export(void)
 {
-	int ret;
+	struct dentry *dentry;
 
-	ret = tracing_init_dentry();
-	if (ret)
+	dentry = tracing_init_dentry();
+	if (IS_ERR(dentry))
 		return 0;
 
 	trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
@@ -397,4 +402,4 @@ static __init int init_trace_printk(void)
 	return register_module_notifier(&module_trace_bprintk_format_nb);
 }
 
-early_initcall(init_trace_printk);
+early_initcall(init_trace_printk);
\ No newline at end of file
diff --git a/kernel/trace/trace_printk.c.orig b/kernel/trace/trace_printk.c.orig
new file mode 100644
index 000000000000..29f6e95439b6
--- /dev/null
+++ b/kernel/trace/trace_printk.c.orig
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * trace binary printk
+ *
+ * Copyright (C) 2008 Lai Jiangshan <laijs@cn.fujitsu.com>
+ *
+ */
+#include <linux/seq_file.h>
+#include <linux/security.h>
+#include <linux/uaccess.h>
+#include <linux/kernel.h>
+#include <linux/ftrace.h>
+#include <linux/string.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/ctype.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include "trace.h"
+
+#ifdef CONFIG_MODULES
+
+/*
+ * modules trace_printk()'s formats are autosaved in struct trace_bprintk_fmt
+ * which are queued on trace_bprintk_fmt_list.
+ */
+static LIST_HEAD(trace_bprintk_fmt_list);
+
+/* serialize accesses to trace_bprintk_fmt_list */
+static DEFINE_MUTEX(btrace_mutex);
+
+struct trace_bprintk_fmt {
+	struct list_head list;
+	const char *fmt;
+};
+
+static inline struct trace_bprintk_fmt *lookup_format(const char *fmt)
+{
+	struct trace_bprintk_fmt *pos;
+
+	if (!fmt)
+		return ERR_PTR(-EINVAL);
+
+	list_for_each_entry(pos, &trace_bprintk_fmt_list, list) {
+		if (!strcmp(pos->fmt, fmt))
+			return pos;
+	}
+	return NULL;
+}
+
+static
+void hold_module_trace_bprintk_format(const char **start, const char **end)
+{
+	const char **iter;
+	char *fmt;
+
+	/* allocate the trace_printk per cpu buffers */
+	if (start != end)
+		trace_printk_init_buffers();
+
+	mutex_lock(&btrace_mutex);
+	for (iter = start; iter < end; iter++) {
+		struct trace_bprintk_fmt *tb_fmt = lookup_format(*iter);
+		if (tb_fmt) {
+			if (!IS_ERR(tb_fmt))
+				*iter = tb_fmt->fmt;
+			continue;
+		}
+
+		fmt = NULL;
+		tb_fmt = kmalloc(sizeof(*tb_fmt), GFP_KERNEL);
+		if (tb_fmt) {
+			fmt = kmalloc(strlen(*iter) + 1, GFP_KERNEL);
+			if (fmt) {
+				list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
+				strcpy(fmt, *iter);
+				tb_fmt->fmt = fmt;
+			} else
+				kfree(tb_fmt);
+		}
+		*iter = fmt;
+
+	}
+	mutex_unlock(&btrace_mutex);
+}
+
+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)
+			hold_module_trace_bprintk_format(start, end);
+	}
+	return NOTIFY_OK;
+}
+
+/*
+ * The debugfs/tracing/printk_formats file maps the addresses with
+ * the ASCII formats that are used in the bprintk events in the
+ * buffer. For userspace tools to be able to decode the events from
+ * the buffer, they need to be able to map the address with the format.
+ *
+ * The addresses of the bprintk formats are in their own section
+ * __trace_printk_fmt. But for modules we copy them into a link list.
+ * The code to print the formats and their addresses passes around the
+ * address of the fmt string. If the fmt address passed into the seq
+ * functions is within the kernel core __trace_printk_fmt section, then
+ * it simply uses the next pointer in the list.
+ *
+ * When the fmt pointer is outside the kernel core __trace_printk_fmt
+ * section, then we need to read the link list pointers. The trick is
+ * we pass the address of the string to the seq function just like
+ * we do for the kernel core formats. To get back the structure that
+ * holds the format, we simply use container_of() and then go to the
+ * next format in the list.
+ */
+static const char **
+find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+{
+	struct trace_bprintk_fmt *mod_fmt;
+
+	if (list_empty(&trace_bprintk_fmt_list))
+		return NULL;
+
+	/*
+	 * v will point to the address of the fmt record from t_next
+	 * v will be NULL from t_start.
+	 * If this is the first pointer or called from start
+	 * then we need to walk the list.
+	 */
+	if (!v || start_index == *pos) {
+		struct trace_bprintk_fmt *p;
+
+		/* search the module list */
+		list_for_each_entry(p, &trace_bprintk_fmt_list, list) {
+			if (start_index == *pos)
+				return &p->fmt;
+			start_index++;
+		}
+		/* pos > index */
+		return NULL;
+	}
+
+	/*
+	 * v points to the address of the fmt field in the mod list
+	 * structure that holds the module print format.
+	 */
+	mod_fmt = container_of(v, typeof(*mod_fmt), fmt);
+	if (mod_fmt->list.next == &trace_bprintk_fmt_list)
+		return NULL;
+
+	mod_fmt = container_of(mod_fmt->list.next, typeof(*mod_fmt), list);
+
+	return &mod_fmt->fmt;
+}
+
+static void format_mod_start(void)
+{
+	mutex_lock(&btrace_mutex);
+}
+
+static void format_mod_stop(void)
+{
+	mutex_unlock(&btrace_mutex);
+}
+
+#else /* !CONFIG_MODULES */
+__init static int
+module_trace_bprintk_format_notify(struct notifier_block *self,
+		unsigned long val, void *data)
+{
+	return NOTIFY_OK;
+}
+static inline const char **
+find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+{
+	return NULL;
+}
+static inline void format_mod_start(void) { }
+static inline void format_mod_stop(void) { }
+#endif /* CONFIG_MODULES */
+
+static bool __read_mostly trace_printk_enabled = true;
+
+void trace_printk_control(bool enabled)
+{
+	trace_printk_enabled = enabled;
+}
+
+__initdata_or_module static
+struct notifier_block module_trace_bprintk_format_nb = {
+	.notifier_call = module_trace_bprintk_format_notify,
+};
+
+int __trace_bprintk(unsigned long ip, const char *fmt, ...)
+{
+	int ret;
+	va_list ap;
+
+	if (unlikely(!fmt))
+		return 0;
+
+	if (!trace_printk_enabled)
+		return 0;
+
+	va_start(ap, fmt);
+	ret = trace_vbprintk(ip, fmt, ap);
+	va_end(ap);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__trace_bprintk);
+
+int __ftrace_vbprintk(unsigned long ip, const char *fmt, va_list ap)
+{
+	if (unlikely(!fmt))
+		return 0;
+
+	if (!trace_printk_enabled)
+		return 0;
+
+	return trace_vbprintk(ip, fmt, ap);
+}
+EXPORT_SYMBOL_GPL(__ftrace_vbprintk);
+
+int __trace_printk(unsigned long ip, const char *fmt, ...)
+{
+	int ret;
+	va_list ap;
+
+	if (!trace_printk_enabled)
+		return 0;
+
+	va_start(ap, fmt);
+	ret = trace_vprintk(ip, fmt, ap);
+	va_end(ap);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__trace_printk);
+
+int __ftrace_vprintk(unsigned long ip, const char *fmt, va_list ap)
+{
+	if (!trace_printk_enabled)
+		return 0;
+
+	return trace_vprintk(ip, fmt, ap);
+}
+EXPORT_SYMBOL_GPL(__ftrace_vprintk);
+
+bool trace_is_tracepoint_string(const char *str)
+{
+	const char **ptr = __start___tracepoint_str;
+
+	for (ptr = __start___tracepoint_str; ptr < __stop___tracepoint_str; ptr++) {
+		if (str == *ptr)
+			return true;
+	}
+	return false;
+}
+
+static const char **find_next(void *v, loff_t *pos)
+{
+	const char **fmt = v;
+	int start_index;
+	int last_index;
+
+	start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt;
+
+	if (*pos < start_index)
+		return __start___trace_bprintk_fmt + *pos;
+
+	/*
+	 * The __tracepoint_str section is treated the same as the
+	 * __trace_printk_fmt section. The difference is that the
+	 * __trace_printk_fmt section should only be used by trace_printk()
+	 * in a debugging environment, as if anything exists in that section
+	 * the trace_prink() helper buffers are allocated, which would just
+	 * waste space in a production environment.
+	 *
+	 * The __tracepoint_str sections on the other hand are used by
+	 * tracepoints which need to map pointers to their strings to
+	 * the ASCII text for userspace.
+	 */
+	last_index = start_index;
+	start_index = __stop___tracepoint_str - __start___tracepoint_str;
+
+	if (*pos < last_index + start_index)
+		return __start___tracepoint_str + (*pos - last_index);
+
+	start_index += last_index;
+	return find_next_mod_format(start_index, v, fmt, pos);
+}
+
+static void *
+t_start(struct seq_file *m, loff_t *pos)
+{
+	format_mod_start();
+	return find_next(NULL, pos);
+}
+
+static void *t_next(struct seq_file *m, void * v, loff_t *pos)
+{
+	(*pos)++;
+	return find_next(v, pos);
+}
+
+static int t_show(struct seq_file *m, void *v)
+{
+	const char **fmt = v;
+	const char *str = *fmt;
+	int i;
+
+	if (!*fmt)
+		return 0;
+
+	seq_printf(m, "0x%lx : \"", *(unsigned long *)fmt);
+
+	/*
+	 * Tabs and new lines need to be converted.
+	 */
+	for (i = 0; str[i]; i++) {
+		switch (str[i]) {
+		case '\n':
+			seq_puts(m, "\\n");
+			break;
+		case '\t':
+			seq_puts(m, "\\t");
+			break;
+		case '\\':
+			seq_putc(m, '\\');
+			break;
+		case '"':
+			seq_puts(m, "\\\"");
+			break;
+		default:
+			seq_putc(m, str[i]);
+		}
+	}
+	seq_puts(m, "\"\n");
+
+	return 0;
+}
+
+static void t_stop(struct seq_file *m, void *p)
+{
+	format_mod_stop();
+}
+
+static const struct seq_operations show_format_seq_ops = {
+	.start = t_start,
+	.next = t_next,
+	.show = t_show,
+	.stop = t_stop,
+};
+
+static int
+ftrace_formats_open(struct inode *inode, struct file *file)
+{
+	int ret;
+
+	ret = security_locked_down(LOCKDOWN_TRACEFS);
+	if (ret)
+		return ret;
+
+	return seq_open(file, &show_format_seq_ops);
+}
+
+static const struct file_operations ftrace_formats_fops = {
+	.open = ftrace_formats_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release,
+};
+
+static __init int init_trace_printk_function_export(void)
+{
+	int ret;
+
+	ret = tracing_init_dentry();
+	if (ret)
+		return 0;
+
+	trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
+				    NULL, &ftrace_formats_fops);
+
+	return 0;
+}
+
+fs_initcall(init_trace_printk_function_export);
+
+static __init int init_trace_printk(void)
+{
+	return register_module_notifier(&module_trace_bprintk_format_nb);
+}
+
+early_initcall(init_trace_printk);
-- 
2.50.1.windows.1

Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling

[Luis Chamberlain]

On Sat, Sep 06, 2025 at 05:41:48PM +0400, Fidal Palamparambil wrote:
> From: Fidal palamparambil <rootuserhere@gmail.com>
> 
> This commit addresses several bugs and potential issues in the
> trace_printk module format handling code:
> 
> 1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
>    proper cleanup when format string allocation fails by setting
>    tb_fmt to NULL after freeing it.
> 
> 2. NULL pointer dereference prevention: Added NULL checks in
>    t_show() function before dereferencing format pointers.
> 
> 3. Input validation: Added NULL check in trace_is_tracepoint_string()
>    to prevent potential NULL pointer dereference.
> 
> 4. Type safety: Fixed type casting in t_show() to use proper
>    unsigned long casting for pointer arithmetic.
> 
> 5. Error handling: Improved error checking in
>    init_trace_printk_function_export() by using IS_ERR() to check
>    dentry pointer.
> 
> 6. Code robustness: Added additional pointer validation throughout
>    the code to handle potential edge cases.
> 
> 7. Memory safety: Ensured consistent handling of format pointers
>    when memory allocation failures occur.
> 
> These fixes improve the stability and reliability of the trace_printk
> infrastructure, particularly when dealing with module loading/unloading
> and format string management.
> 
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>

Anothe waste of time. Stop. I'll suggest everyone ignore your patches.

  Luis

Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling

[kernel test robot]

Hi Fidal,

kernel test robot noticed the following build warnings:

[auto build test WARNING on trace/for-next]
[also build test WARNING on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/tracing-Fix-multiple-issues-in-trace_printk-module-handling/20250906-214415
base:   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link:    https://lore.kernel.org/r/20250906134148.55-1-rootuserhere%40gmail.com
patch subject: [PATCH] tracing: Fix multiple issues in trace_printk module handling
config: x86_64-buildonly-randconfig-001-20250907 (https://download.01.org/0day-ci/archive/20250907/202509070951.9KQcO9l6-lkp@intel.com/config)
compiler: gcc-13 (Debian 13.3.0-16) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250907/202509070951.9KQcO9l6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509070951.9KQcO9l6-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/trace/trace_printk.c: In function 'init_trace_printk_function_export':
>> kernel/trace/trace_printk.c:388:16: warning: assignment to 'struct dentry *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     388 |         dentry = tracing_init_dentry();
         |                ^


vim +388 kernel/trace/trace_printk.c

   383	
   384	static __init int init_trace_printk_function_export(void)
   385	{
   386		struct dentry *dentry;
   387	
 > 388		dentry = tracing_init_dentry();
   389		if (IS_ERR(dentry))
   390			return 0;
   391	
   392		trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
   393					    NULL, &ftrace_formats_fops);
   394	
   395		return 0;
   396	}
   397	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling

[kernel test robot]

Hi Fidal,

kernel test robot noticed the following build errors:

[auto build test ERROR on trace/for-next]
[also build test ERROR on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/tracing-Fix-multiple-issues-in-trace_printk-module-handling/20250906-214415
base:   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link:    https://lore.kernel.org/r/20250906134148.55-1-rootuserhere%40gmail.com
patch subject: [PATCH] tracing: Fix multiple issues in trace_printk module handling
config: s390-randconfig-002-20250907 (https://download.01.org/0day-ci/archive/20250907/202509071126.l4plN0Qk-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250907/202509071126.l4plN0Qk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509071126.l4plN0Qk-lkp@intel.com/

All errors (new ones prefixed by >>):

   kernel/trace/trace_printk.c: In function 'init_trace_printk_function_export':
>> kernel/trace/trace_printk.c:388:16: error: assignment to 'struct dentry *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     388 |         dentry = tracing_init_dentry();
         |                ^


vim +388 kernel/trace/trace_printk.c

   383	
   384	static __init int init_trace_printk_function_export(void)
   385	{
   386		struct dentry *dentry;
   387	
 > 388		dentry = tracing_init_dentry();
   389		if (IS_ERR(dentry))
   390			return 0;
   391	
   392		trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
   393					    NULL, &ftrace_formats_fops);
   394	
   395		return 0;
   396	}
   397	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling

[kernel test robot]

Hi Fidal,

kernel test robot noticed the following build warnings:

[auto build test WARNING on trace/for-next]
[also build test WARNING on linus/master v6.17-rc4 next-20250905]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Fidal-Palamparambil/tracing-Fix-multiple-issues-in-trace_printk-module-handling/20250906-214415
base:   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link:    https://lore.kernel.org/r/20250906134148.55-1-rootuserhere%40gmail.com
patch subject: [PATCH] tracing: Fix multiple issues in trace_printk module handling
config: powerpc-randconfig-r131-20250907 (https://download.01.org/0day-ci/archive/20250907/202509071540.GTxwwstz-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 13.4.0
reproduce: (https://download.01.org/0day-ci/archive/20250907/202509071540.GTxwwstz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509071540.GTxwwstz-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> kernel/trace/trace_printk.c:388:16: sparse: sparse: incorrect type in assignment (different base types) @@     expected struct dentry *dentry @@     got int @@
   kernel/trace/trace_printk.c:388:16: sparse:     expected struct dentry *dentry
   kernel/trace/trace_printk.c:388:16: sparse:     got int

vim +388 kernel/trace/trace_printk.c

   383	
   384	static __init int init_trace_printk_function_export(void)
   385	{
   386		struct dentry *dentry;
   387	
 > 388		dentry = tracing_init_dentry();
   389		if (IS_ERR(dentry))
   390			return 0;
   391	
   392		trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
   393					    NULL, &ftrace_formats_fops);
   394	
   395		return 0;
   396	}
   397	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH] tracing : Fix multiple issues in trace_printk module handling

[Fidal Palamparambil]

From: Fidal palamparambil <rootuserhere@gmail.com>

This commit addresses several bugs and potential issues in the
trace_printk module format handling code:

1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
   proper cleanup when format string allocation fails by setting
   tb_fmt to NULL after freeing it to prevent memory leaks.

2. NULL pointer dereference prevention: Added comprehensive NULL checks
   in t_show() function before dereferencing format pointers to prevent
   kernel crashes.

3. Input validation: Added NULL check in trace_is_tracepoint_string()
   to prevent potential NULL pointer dereference when called with
   invalid input.

4. Type safety: Fixed type casting in t_show() to use proper
   unsigned long casting for pointer arithmetic, ensuring correct
   pointer handling across different architectures.

5. Error handling: Fixed type mismatch in init_trace_printk_function_export()
   by properly handling struct dentry pointer return from tracing_init_dentry()
   and using IS_ERR_OR_NULL() for comprehensive error checking.

6. Code robustness: Added additional pointer validation throughout
   the code to handle potential edge cases and improve overall stability.

7. Memory safety: Ensured consistent handling of format pointers
   when memory allocation failures occur, preventing use-after-free
   and other memory corruption issues.

These fixes improve the stability and reliability of the trace_printk
infrastructure, particularly when dealing with module loading/unloading
and format string management.

Reported-by : kernel test robot <lkp@intel.com>
Closes : https://lore.kernel.org/oe-kbuild-all/202509071540.GTxwwstz-lkp@intel.com/
Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
---
 kernel/trace/trace_printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
index cb962c6c02f8..665effbf50ae 100644
--- a/kernel/trace/trace_printk.c
+++ b/kernel/trace/trace_printk.c
@@ -386,7 +386,7 @@ static __init int init_trace_printk_function_export(void)
 	struct dentry *dentry;
 
 	dentry = tracing_init_dentry();
-	if (IS_ERR(dentry))
+	if (IS_ERR_OR_NULL(dentry))
 		return 0;
 
 	trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
-- 
2.50.1.windows.1

Re: [PATCH] tracing : Fix multiple issues in trace_printk module handling

[Luis Chamberlain]

On Sun, Sep 07, 2025 at 05:52:01PM +0400, Fidal Palamparambil wrote:
> From: Fidal palamparambil <rootuserhere@gmail.com>
> 
> This commit addresses several bugs and potential issues in the
> trace_printk module format handling code:
> 
> 1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
>    proper cleanup when format string allocation fails by setting
>    tb_fmt to NULL after freeing it to prevent memory leaks.
> 
> 2. NULL pointer dereference prevention: Added comprehensive NULL checks
>    in t_show() function before dereferencing format pointers to prevent
>    kernel crashes.
> 
> 3. Input validation: Added NULL check in trace_is_tracepoint_string()
>    to prevent potential NULL pointer dereference when called with
>    invalid input.
> 
> 4. Type safety: Fixed type casting in t_show() to use proper
>    unsigned long casting for pointer arithmetic, ensuring correct
>    pointer handling across different architectures.
> 
> 5. Error handling: Fixed type mismatch in init_trace_printk_function_export()
>    by properly handling struct dentry pointer return from tracing_init_dentry()
>    and using IS_ERR_OR_NULL() for comprehensive error checking.
> 
> 6. Code robustness: Added additional pointer validation throughout
>    the code to handle potential edge cases and improve overall stability.
> 
> 7. Memory safety: Ensured consistent handling of format pointers
>    when memory allocation failures occur, preventing use-after-free
>    and other memory corruption issues.
> 
> These fixes improve the stability and reliability of the trace_printk
> infrastructure, particularly when dealing with module loading/unloading
> and format string management.
> 
> Reported-by : kernel test robot <lkp@intel.com>
> Closes : https://lore.kernel.org/oe-kbuild-all/202509071540.GTxwwstz-lkp@intel.com/
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>

Stop, at this point after being told to stop, your intent is clear: to bug.

Go away. You're on my ignore list now.

  Luis

Re: [PATCH] tracing: Fix multiple issues in trace_printk module handling

[Christophe Leroy]

Le 06/09/2025 à 15:41, Fidal Palamparambil a écrit :
> [Vous ne recevez pas souvent de courriers de rootuserhere@gmail.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> From: Fidal palamparambil <rootuserhere@gmail.com>
> 
> This commit addresses several bugs and potential issues in the
> trace_printk module format handling code:
> 
> 1. Memory leak fix: In hold_module_trace_bprintk_format(), ensure
>     proper cleanup when format string allocation fails by setting
>     tb_fmt to NULL after freeing it.

Why is that needed ?

> 
> 2. NULL pointer dereference prevention: Added NULL checks in
>     t_show() function before dereferencing format pointers.

isn't it already checked by the caller ?

> 
> 3. Input validation: Added NULL check in trace_is_tracepoint_string()
>     to prevent potential NULL pointer dereference.
> 
> 4. Type safety: Fixed type casting in t_show() to use proper
>     unsigned long casting for pointer arithmetic.
> 
> 5. Error handling: Improved error checking in
>     init_trace_printk_function_export() by using IS_ERR() to check
>     dentry pointer.
> 
> 6. Code robustness: Added additional pointer validation throughout
>     the code to handle potential edge cases.
> 
> 7. Memory safety: Ensured consistent handling of format pointers
>     when memory allocation failures occur.

All those points look pointless, please elaborate.


> 
> These fixes improve the stability and reliability of the trace_printk
> infrastructure, particularly when dealing with module loading/unloading
> and format string management.
> 
> Signed-off-by: Fidal palamparambil <rootuserhere@gmail.com>
> ---
>   kernel/trace/trace_printk.c      |  33 +--
>   kernel/trace/trace_printk.c.orig | 400 +++++++++++++++++++++++++++++++

What is the point in adding that .orig file ?

>   2 files changed, 419 insertions(+), 14 deletions(-)
>   create mode 100644 kernel/trace/trace_printk.c.orig
> 
> diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
> index 29f6e95439b6..cb962c6c02f8 100644
> --- a/kernel/trace/trace_printk.c
> +++ b/kernel/trace/trace_printk.c
> @@ -76,10 +76,12 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
>                                  list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
>                                  strcpy(fmt, *iter);
>                                  tb_fmt->fmt = fmt;
> -                       } else
> +                       } else {
>                                  kfree(tb_fmt);
> +                               tb_fmt = NULL;

Why is that needed ? tb_fmt is not reused before being assigned again.

> +                       }
>                  }
> -               *iter = fmt;
> +               *iter = tb_fmt ? tb_fmt->fmt : NULL;

Why complicate it ? fmt is already NULL when needed so what's the point 
here ?

> 
>          }
>          mutex_unlock(&btrace_mutex);
> @@ -253,7 +255,10 @@ EXPORT_SYMBOL_GPL(__ftrace_vprintk);
> 
>   bool trace_is_tracepoint_string(const char *str)
>   {
> -       const char **ptr = __start___tracepoint_str;
> +       const char **ptr;
> +
> +       if (!str)
> +               return false;

What is the problem here ? str is never dereferenced in this function.

> 
>          for (ptr = __start___tracepoint_str; ptr < __stop___tracepoint_str; ptr++) {
>                  if (str == *ptr)
> @@ -311,19 +316,19 @@ static void *t_next(struct seq_file *m, void * v, loff_t *pos)
>   static int t_show(struct seq_file *m, void *v)
>   {
>          const char **fmt = v;
> -       const char *str = *fmt;
> -       int i;
> +       const char *str;
> 
> -       if (!*fmt)
> +       if (!fmt || !*fmt)

How can this happen ?

>                  return 0;
> 
> -       seq_printf(m, "0x%lx : \"", *(unsigned long *)fmt);
> +       str = *fmt;
> +       seq_printf(m, "0x%lx : \"", (unsigned long)fmt);
> 
>          /*
>           * Tabs and new lines need to be converted.
>           */
> -       for (i = 0; str[i]; i++) {
> -               switch (str[i]) {
> +       for (; *str; str++) {
> +               switch (*str) {

Why is this change needed ?

>                  case '\n':
>                          seq_puts(m, "\\n");
>                          break;
> @@ -337,7 +342,7 @@ static int t_show(struct seq_file *m, void *v)
>                          seq_puts(m, "\\\"");
>                          break;
>                  default:
> -                       seq_putc(m, str[i]);
> +                       seq_putc(m, *str);
>                  }
>          }
>          seq_puts(m, "\"\n");
> @@ -378,10 +383,10 @@ static const struct file_operations ftrace_formats_fops = {
> 
>   static __init int init_trace_printk_function_export(void)
>   {
> -       int ret;
> +       struct dentry *dentry;
> 
> -       ret = tracing_init_dentry();
> -       if (ret)
> +       dentry = tracing_init_dentry();

tracing_init_dentry() returns an int, how can this change build ???!!!

> +       if (IS_ERR(dentry))
>                  return 0;
> 
>          trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
> @@ -397,4 +402,4 @@ static __init int init_trace_printk(void)
>          return register_module_notifier(&module_trace_bprintk_format_nb);
>   }
> 
> -early_initcall(init_trace_printk);
> +early_initcall(init_trace_printk);
> \ No newline at end of file
> diff --git a/kernel/trace/trace_printk.c.orig b/kernel/trace/trace_printk.c.orig
> new file mode 100644
> index 000000000000..29f6e95439b6
> --- /dev/null
> +++ b/kernel/trace/trace_printk.c.orig
> @@ -0,0 +1,400 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * trace binary printk
> + *
> + * Copyright (C) 2008 Lai Jiangshan <laijs@cn.fujitsu.com>
> + *
> + */
> +#include <linux/seq_file.h>
> +#include <linux/security.h>
> +#include <linux/uaccess.h>
> +#include <linux/kernel.h>
> +#include <linux/ftrace.h>
> +#include <linux/string.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/ctype.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +
> +#include "trace.h"
> +
> +#ifdef CONFIG_MODULES
> +
> +/*
> + * modules trace_printk()'s formats are autosaved in struct trace_bprintk_fmt
> + * which are queued on trace_bprintk_fmt_list.
> + */
> +static LIST_HEAD(trace_bprintk_fmt_list);
> +
> +/* serialize accesses to trace_bprintk_fmt_list */
> +static DEFINE_MUTEX(btrace_mutex);
> +
> +struct trace_bprintk_fmt {
> +       struct list_head list;
> +       const char *fmt;
> +};
> +
> +static inline struct trace_bprintk_fmt *lookup_format(const char *fmt)
> +{
> +       struct trace_bprintk_fmt *pos;
> +
> +       if (!fmt)
> +               return ERR_PTR(-EINVAL);
> +
> +       list_for_each_entry(pos, &trace_bprintk_fmt_list, list) {
> +               if (!strcmp(pos->fmt, fmt))
> +                       return pos;
> +       }
> +       return NULL;
> +}
> +
> +static
> +void hold_module_trace_bprintk_format(const char **start, const char **end)
> +{
> +       const char **iter;
> +       char *fmt;
> +
> +       /* allocate the trace_printk per cpu buffers */
> +       if (start != end)
> +               trace_printk_init_buffers();
> +
> +       mutex_lock(&btrace_mutex);
> +       for (iter = start; iter < end; iter++) {
> +               struct trace_bprintk_fmt *tb_fmt = lookup_format(*iter);
> +               if (tb_fmt) {
> +                       if (!IS_ERR(tb_fmt))
> +                               *iter = tb_fmt->fmt;
> +                       continue;
> +               }
> +
> +               fmt = NULL;
> +               tb_fmt = kmalloc(sizeof(*tb_fmt), GFP_KERNEL);
> +               if (tb_fmt) {
> +                       fmt = kmalloc(strlen(*iter) + 1, GFP_KERNEL);
> +                       if (fmt) {
> +                               list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
> +                               strcpy(fmt, *iter);
> +                               tb_fmt->fmt = fmt;
> +                       } else
> +                               kfree(tb_fmt);
> +               }
> +               *iter = fmt;
> +
> +       }
> +       mutex_unlock(&btrace_mutex);
> +}
> +
> +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)
> +                       hold_module_trace_bprintk_format(start, end);
> +       }
> +       return NOTIFY_OK;
> +}
> +
> +/*
> + * The debugfs/tracing/printk_formats file maps the addresses with
> + * the ASCII formats that are used in the bprintk events in the
> + * buffer. For userspace tools to be able to decode the events from
> + * the buffer, they need to be able to map the address with the format.
> + *
> + * The addresses of the bprintk formats are in their own section
> + * __trace_printk_fmt. But for modules we copy them into a link list.
> + * The code to print the formats and their addresses passes around the
> + * address of the fmt string. If the fmt address passed into the seq
> + * functions is within the kernel core __trace_printk_fmt section, then
> + * it simply uses the next pointer in the list.
> + *
> + * When the fmt pointer is outside the kernel core __trace_printk_fmt
> + * section, then we need to read the link list pointers. The trick is
> + * we pass the address of the string to the seq function just like
> + * we do for the kernel core formats. To get back the structure that
> + * holds the format, we simply use container_of() and then go to the
> + * next format in the list.
> + */
> +static const char **
> +find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
> +{
> +       struct trace_bprintk_fmt *mod_fmt;
> +
> +       if (list_empty(&trace_bprintk_fmt_list))
> +               return NULL;
> +
> +       /*
> +        * v will point to the address of the fmt record from t_next
> +        * v will be NULL from t_start.
> +        * If this is the first pointer or called from start
> +        * then we need to walk the list.
> +        */
> +       if (!v || start_index == *pos) {
> +               struct trace_bprintk_fmt *p;
> +
> +               /* search the module list */
> +               list_for_each_entry(p, &trace_bprintk_fmt_list, list) {
> +                       if (start_index == *pos)
> +                               return &p->fmt;
> +                       start_index++;
> +               }
> +               /* pos > index */
> +               return NULL;
> +       }
> +
> +       /*
> +        * v points to the address of the fmt field in the mod list
> +        * structure that holds the module print format.
> +        */
> +       mod_fmt = container_of(v, typeof(*mod_fmt), fmt);
> +       if (mod_fmt->list.next == &trace_bprintk_fmt_list)
> +               return NULL;
> +
> +       mod_fmt = container_of(mod_fmt->list.next, typeof(*mod_fmt), list);
> +
> +       return &mod_fmt->fmt;
> +}
> +
> +static void format_mod_start(void)
> +{
> +       mutex_lock(&btrace_mutex);
> +}
> +
> +static void format_mod_stop(void)
> +{
> +       mutex_unlock(&btrace_mutex);
> +}
> +
> +#else /* !CONFIG_MODULES */
> +__init static int
> +module_trace_bprintk_format_notify(struct notifier_block *self,
> +               unsigned long val, void *data)
> +{
> +       return NOTIFY_OK;
> +}
> +static inline const char **
> +find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
> +{
> +       return NULL;
> +}
> +static inline void format_mod_start(void) { }
> +static inline void format_mod_stop(void) { }
> +#endif /* CONFIG_MODULES */
> +
> +static bool __read_mostly trace_printk_enabled = true;
> +
> +void trace_printk_control(bool enabled)
> +{
> +       trace_printk_enabled = enabled;
> +}
> +
> +__initdata_or_module static
> +struct notifier_block module_trace_bprintk_format_nb = {
> +       .notifier_call = module_trace_bprintk_format_notify,
> +};
> +
> +int __trace_bprintk(unsigned long ip, const char *fmt, ...)
> +{
> +       int ret;
> +       va_list ap;
> +
> +       if (unlikely(!fmt))
> +               return 0;
> +
> +       if (!trace_printk_enabled)
> +               return 0;
> +
> +       va_start(ap, fmt);
> +       ret = trace_vbprintk(ip, fmt, ap);
> +       va_end(ap);
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(__trace_bprintk);
> +
> +int __ftrace_vbprintk(unsigned long ip, const char *fmt, va_list ap)
> +{
> +       if (unlikely(!fmt))
> +               return 0;
> +
> +       if (!trace_printk_enabled)
> +               return 0;
> +
> +       return trace_vbprintk(ip, fmt, ap);
> +}
> +EXPORT_SYMBOL_GPL(__ftrace_vbprintk);
> +
> +int __trace_printk(unsigned long ip, const char *fmt, ...)
> +{
> +       int ret;
> +       va_list ap;
> +
> +       if (!trace_printk_enabled)
> +               return 0;
> +
> +       va_start(ap, fmt);
> +       ret = trace_vprintk(ip, fmt, ap);
> +       va_end(ap);
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(__trace_printk);
> +
> +int __ftrace_vprintk(unsigned long ip, const char *fmt, va_list ap)
> +{
> +       if (!trace_printk_enabled)
> +               return 0;
> +
> +       return trace_vprintk(ip, fmt, ap);
> +}
> +EXPORT_SYMBOL_GPL(__ftrace_vprintk);
> +
> +bool trace_is_tracepoint_string(const char *str)
> +{
> +       const char **ptr = __start___tracepoint_str;
> +
> +       for (ptr = __start___tracepoint_str; ptr < __stop___tracepoint_str; ptr++) {
> +               if (str == *ptr)
> +                       return true;
> +       }
> +       return false;
> +}
> +
> +static const char **find_next(void *v, loff_t *pos)
> +{
> +       const char **fmt = v;
> +       int start_index;
> +       int last_index;
> +
> +       start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt;
> +
> +       if (*pos < start_index)
> +               return __start___trace_bprintk_fmt + *pos;
> +
> +       /*
> +        * The __tracepoint_str section is treated the same as the
> +        * __trace_printk_fmt section. The difference is that the
> +        * __trace_printk_fmt section should only be used by trace_printk()
> +        * in a debugging environment, as if anything exists in that section
> +        * the trace_prink() helper buffers are allocated, which would just
> +        * waste space in a production environment.
> +        *
> +        * The __tracepoint_str sections on the other hand are used by
> +        * tracepoints which need to map pointers to their strings to
> +        * the ASCII text for userspace.
> +        */
> +       last_index = start_index;
> +       start_index = __stop___tracepoint_str - __start___tracepoint_str;
> +
> +       if (*pos < last_index + start_index)
> +               return __start___tracepoint_str + (*pos - last_index);
> +
> +       start_index += last_index;
> +       return find_next_mod_format(start_index, v, fmt, pos);
> +}
> +
> +static void *
> +t_start(struct seq_file *m, loff_t *pos)
> +{
> +       format_mod_start();
> +       return find_next(NULL, pos);
> +}
> +
> +static void *t_next(struct seq_file *m, void * v, loff_t *pos)
> +{
> +       (*pos)++;
> +       return find_next(v, pos);
> +}
> +
> +static int t_show(struct seq_file *m, void *v)
> +{
> +       const char **fmt = v;
> +       const char *str = *fmt;
> +       int i;
> +
> +       if (!*fmt)
> +               return 0;
> +
> +       seq_printf(m, "0x%lx : \"", *(unsigned long *)fmt);
> +
> +       /*
> +        * Tabs and new lines need to be converted.
> +        */
> +       for (i = 0; str[i]; i++) {
> +               switch (str[i]) {
> +               case '\n':
> +                       seq_puts(m, "\\n");
> +                       break;
> +               case '\t':
> +                       seq_puts(m, "\\t");
> +                       break;
> +               case '\\':
> +                       seq_putc(m, '\\');
> +                       break;
> +               case '"':
> +                       seq_puts(m, "\\\"");
> +                       break;
> +               default:
> +                       seq_putc(m, str[i]);
> +               }
> +       }
> +       seq_puts(m, "\"\n");
> +
> +       return 0;
> +}
> +
> +static void t_stop(struct seq_file *m, void *p)
> +{
> +       format_mod_stop();
> +}
> +
> +static const struct seq_operations show_format_seq_ops = {
> +       .start = t_start,
> +       .next = t_next,
> +       .show = t_show,
> +       .stop = t_stop,
> +};
> +
> +static int
> +ftrace_formats_open(struct inode *inode, struct file *file)
> +{
> +       int ret;
> +
> +       ret = security_locked_down(LOCKDOWN_TRACEFS);
> +       if (ret)
> +               return ret;
> +
> +       return seq_open(file, &show_format_seq_ops);
> +}
> +
> +static const struct file_operations ftrace_formats_fops = {
> +       .open = ftrace_formats_open,
> +       .read = seq_read,
> +       .llseek = seq_lseek,
> +       .release = seq_release,
> +};
> +
> +static __init int init_trace_printk_function_export(void)
> +{
> +       int ret;
> +
> +       ret = tracing_init_dentry();
> +       if (ret)
> +               return 0;
> +
> +       trace_create_file("printk_formats", TRACE_MODE_READ, NULL,
> +                                   NULL, &ftrace_formats_fops);
> +
> +       return 0;
> +}
> +
> +fs_initcall(init_trace_printk_function_export);
> +
> +static __init int init_trace_printk(void)
> +{
> +       return register_module_notifier(&module_trace_bprintk_format_nb);
> +}
> +
> +early_initcall(init_trace_printk);
> --
> 2.50.1.windows.1
> 
>