Re: [RFC PATCH v1 3/5] tracing: Add __print_untrusted_str()
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2025-05-23 18:21:55
Also in:
linux-security-module
On Fri, 23 May 2025 18:57:39 +0200 Mickaël Salaün [off-list ref] wrote:
Add a new __print_untrusted_str() helper to safely print strings after escaping all special characters, including common separators (space, equal sign), quotes, and backslashes. This transforms a string from an untrusted source (e.g. user space) to make it: - safe to parse, - easy to read (for simple strings), - easy to get back the original.
Hmm, so this can be an issue if this is printed out via seq_file()? I'm curious to what exactly can be "unsafe" about a string being printed via "%s"? I'm not against this change, I just want to understand more about what the issue is.
quoted hunk ↗ jump to hunk
Cc: Günther Noack <gnoack@google.com> Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Tingmao Wang <redacted> Signed-off-by: Mickaël Salaün <mic@digikod.net> --- include/linux/trace_events.h | 3 ++ include/trace/stages/stage3_trace_output.h | 4 +++ include/trace/stages/stage7_class_define.h | 1 + kernel/trace/trace_output.c | 40 ++++++++++++++++++++++ 4 files changed, 48 insertions(+)diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index fa9cf4292dff..78f543bb7558 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h@@ -54,6 +54,9 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str, int prefix_type, int rowsize, int groupsize, const void *buf, size_t len, bool ascii); +const char *trace_print_untrusted_str_seq(struct trace_seq *s, const char *str); + + struct trace_iterator; struct trace_event;diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h index 1e7b0bef95f5..36947ca2abcb 100644 --- a/include/trace/stages/stage3_trace_output.h +++ b/include/trace/stages/stage3_trace_output.h@@ -133,6 +133,10 @@ trace_print_hex_dump_seq(p, prefix_str, prefix_type, \ rowsize, groupsize, buf, len, ascii) +#undef __print_untrusted_str +#define __print_untrusted_str(str) \ + trace_print_untrusted_str_seq(p, __get_str(str)) + #undef __print_ns_to_secs #define __print_ns_to_secs(value) \ ({ \diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h index fcd564a590f4..bc10b69b755d 100644 --- a/include/trace/stages/stage7_class_define.h +++ b/include/trace/stages/stage7_class_define.h@@ -24,6 +24,7 @@ #undef __print_array #undef __print_dynamic_array #undef __print_hex_dump +#undef __print_untrusted_string #undef __get_buf /*diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c index b9ab06c99543..17d576941147 100644 --- a/kernel/trace/trace_output.c +++ b/kernel/trace/trace_output.c@@ -16,6 +16,7 @@ #include <linux/btf.h> #include <linux/bpf.h> #include <linux/hashtable.h> +#include <linux/string_helpers.h> #include "trace_output.h" #include "trace_btf.h"@@ -297,6 +298,45 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str, } EXPORT_SYMBOL(trace_print_hex_dump_seq); +/** + * trace_print_untrusted_str_seq - print a string after escaping characters + * @s: trace seq struct to write to + * @src: The string to print + * + * Prints a string to a trace seq after escaping all special characters, + * including common separators (space, equal sign), quotes, and backslashes. + * This transforms a string from an untrusted source (e.g. user space) to make + * it: + * - safe to parse, + * - easy to read (for simple strings), + * - easy to get back the original. + */ +const char *trace_print_untrusted_str_seq(struct trace_seq *s, + const char *src) +{ + int escaped_size; + char *buf; + size_t buf_size = seq_buf_get_buf(&s->seq, &buf); + const char *ret = trace_seq_buffer_ptr(s); + + if (!src || WARN_ON(buf_size == 0))
WARN_ON_ONCE() please. -- Steve
+ return NULL;
+
+ escaped_size = string_escape_mem(src, strlen(src), buf, buf_size,
+ ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_NAP | ESCAPE_APPEND |
+ ESCAPE_OCTAL, " ='\"\\");
+ if (unlikely(escaped_size >= buf_size)) {
+ /* We need some room for the final '\0'. */
+ seq_buf_set_overflow(&s->seq);
+ s->full = 1;
+ return NULL;
+ }
+ seq_buf_commit(&s->seq, escaped_size);
+ trace_seq_putc(s, 0);
+ return ret;
+}
+EXPORT_SYMBOL(trace_print_untrusted_str_seq);
+
int trace_raw_output_prep(struct trace_iterator *iter,
struct trace_event *trace_event)
{