linux-kernel - Re: [GIT PULL] ftrace: Fixes for v6.13

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]

Message-ID: <CAHk-=wgyWEbWa9k5=z4LxY1hx0Pxqf5TQQC_BKme_+DrzGufKw@mail.gmail.com>
Date: Sat, 14 Dec 2024 16:37:39 -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>, 
	Mark Rutland <mark.rutland@....com>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
	Al Viro <viro@...iv.linux.org.uk>, Michal Simek <monstr@...str.eu>
Subject: Re: [GIT PULL] ftrace: Fixes for v6.13

On Sat, 14 Dec 2024 at 15:21, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> - Fix output of trace when hashing a pointer is disabled

Christ.

I'm looking at this, and my reaction is literally "tracing is doing
crazy things AGAIN".

This stuff is full of crazy special cases for things that should never
be done in the first place.

And - surprise surprise - it was buggy in odd ways as a result.

If I read the code right, this hacky code literally only catches stale
strings, but also has some *insane* code to do "text_delta" to
'%p[sS]' printouts.

And I see how it does that

                text_delta = iter->tr->text_delta;

but I don't actually see where 'text_delta' would ever actually get
set.  Where does tr->text_delta ever get set to anything but zero?

That code should be *removed*. It seems to be there entirely as a
sanity check, and when the sanity check itself is buggy, you don't try
to fix it, you remove the whole crazy garbage.

What makes '%s' so special in trace formats that it merits this
horrible hackery?

What makes 'text_delta' at all sane, never mind where it even gets set?

And why should we maintain that horrible hackery and make it worse?

This code shows a complete lack of taste. This needs extreme
crapectomy, not some alleged "fix".

If "people use stale pointers for '%s' and we actually care" is a real
issue, we could very possibly teach vsnprintf() about that. The code
already has a "check_pointer()" thing for just NULL pointer magic
printouts.

                  Linus