linux-kernel - Re: [GIT PULL] tracing: Prevent trace_marker being bigger than unsigned short

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 2 Mar 2024 09:24:37 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [GIT PULL] tracing: Prevent trace_marker being bigger than
 unsigned short

On Sat, 2 Mar 2024 at 08:10, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> - The change to allow trace_marker writes to be as big as the trace_seq can
>   hold, and also the change that increases the size of the trace_seq to two
>   pages, caused PowerPC kselftest trace_marker test to fail. The trace_marker
>   kselftest writes up to subbuffer size which is determined by PAGE_SIZE.
>   On PowerPC, the PAGE_SIZE can be 64K, which means the selftest will write
>   a string that is around 64K in size. The output of the trace_marker is
>   performed with a vsnprintf("%.*s", size, string), but this write would make
>   the size greater than 32K, which is the max precision of "%.*s", and that
>   causes a kernel warning. The fix is simply to keep the write of trace_marker
>   less than or equal to max signed short.

Please don't just add random limits that are based on other random limits.

That printk warning is for "you did something obviously crazy".

That does NOT MEAN that you now should limit your strings to something
JUST BORDERLINE CRAZY.

See?

There is not a way in hell that printing a 32kB string in tracing is
valid. EVER.

So stop it. Stop making limits be some random implementation detail.
Make limits *sane*.

Make a *sane* limit for tracing. Not a "avoid being called crazy" limit.

Honestly, I suspect that a sane limit for tracing strings is likely on
the order of tens or maybe hundreds of bytes. Not some kind of "fits
in a short" that is just printk saying "I refuse to waste memory on
the stack".

Side note: for similar reasons the field-width is a 24-bit integer.
And no, if you think that passing a 16MB field width is sane, you need
to rethink your life. Again, that's a small implementation detail, not
a "let's explore how stupid we can be".

          Linus