On Fri, 23 May 2025 18:57:39 +0200 Mickaël Salaün <m...@digikod.net> 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. > > Cc: Günther Noack <gno...@google.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> > Cc: Steven Rostedt <rost...@goodmis.org> > Cc: Tingmao Wang <m...@maowtm.org> > Signed-off-by: Mickaël Salaün <m...@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) > {