[RFC PATCH v1 3/5] tracing: Add __print_untrusted_str()
Mickaël Salaün
mic at digikod.net
Mon May 26 17:46:09 UTC 2025
On Fri, May 23, 2025 at 02:22:42PM -0400, Steven Rostedt wrote:
> On Fri, 23 May 2025 18:57:39 +0200
> Mickaël Salaün <mic at 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"?
There is no issue for the kernel, only for users and user space. :)
>
> I'm not against this change, I just want to understand more about what the
> issue is.
The issue is about a malicious process triggering a trace event with an
arbitrary string. If such string is printed to the root's terminal, it
can print escape sequences and do nasty things. For instance, the
terminal can beep, the window's title can be updated, a path name can be
"hidden" with specific colors, the screen can be completely cleared and
rewritten to trick peoples, and other "original" terminal features can
be triggered by custom escape sequences.
This is definitely not something new but still relevant. There are a
lot of articles about this kind of issues:
https://phrack.org/issues/25/5
https://marc.info/?l=bugtraq&m=104612710031920
https://www.cyberark.com/resources/threat-research-blog/dont-trust-this-title-abusing-terminal-emulators-with-ansi-escape-characters
https://blog.trailofbits.com/2025/04/29/deceiving-users-with-ansi-terminal-codes-in-mcp/
In a less malicious environment, this helper would also be useful to
just sanitize arbitrary text. For instance, because '=' and ' ' are
escaped, it's easy to write a key=value parser in shell (without bug),
or to say it another way, it's more difficult for a parser to fail. ;)
Anyway, this sanitization should not be visible in most cases.
>
> >
> > Cc: Günther Noack <gnoack at google.com>
> > Cc: Masami Hiramatsu <mhiramat at kernel.org>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> > Cc: Steven Rostedt <rostedt at goodmis.org>
> > Cc: Tingmao Wang <m at maowtm.org>
> > Signed-off-by: Mickaël Salaün <mic at 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.
I mimicked nearby code but WARN_ON_ONCE() is indeed better.
Thanks.
>
> -- 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)
> > {
>
>
More information about the Linux-security-module-archive
mailing list