Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220507052451.12890-1-ojeda@kernel.org> <20220507052451.12890-18-ojeda@kernel.org>
 <202205070122.B240F989@keescook>
In-Reply-To: <202205070122.B240F989@keescook>
From:   Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Date:   Sat, 7 May 2022 12:21:14 +0200
Message-ID: <CANiq72m6MaaH-o9McMuDwicosA2xgpU5ZEw0OduR0JT3w7p_fQ@mail.gmail.com>
Subject: Re: [PATCH v6 17/23] scripts: decode_stacktrace: demangle Rust symbols
To:     Kees Cook <keescook@chromium.org>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        rust-for-linux <rust-for-linux@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Jarkko Sakkinen <jarkko@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Kees,

Thanks a lot for taking the time to read all that :)

On Sat, May 7, 2022 at 10:32 AM Kees Cook <keescook@chromium.org> wrote:
>
> I may need some examples here for what you're thinking will cause
> problems. Why a new specifier? Won't demangling just give us text? Is
> the concern about breaking backtrace parsers that only understand C
> symbols?

What I was thinking here is that if we replace how `%pB` works, for
instance, and do demangling unconditionally, then we would break some
of the use cases that are expecting real symbol names, e.g.
`/proc/pid/stack`. So we would need to introduce a way to
differentiate the cases where real symbols should be kept vs.
demangled backtraces for humans, e.g. a new specifier or a modifier
for the existing ones.

Similarly, if we modify the backtrace printed in the kernel log, there
is a high chance we break somebody's userspace backtrace parsers and
other tools connected to those in different ways, e.g.:

  - If we replace the mangled symbol, then some tools may not expect
e.g. whitespace or other characters (which Rust demangled symbols
have); or if they handle them, they may be expecting actual symbols
(like in the case above) because they use them later on to correlate
them to some other data.

  - If we keep the mangled symbols (so that tools still have the real
symbols) and introduce an extra line (or extra length in the same
line) per Rust symbol (where we write the demangled version), that
could still break some parsers just because of the new line (or extra
data).

So my concern is all about how to introduce the new information
without breaking any existing use case.

> It seems all of that would be in the build-time helper, not the kernel
> image, though, so that seems better than run-time demangling.

Hmm... I am not sure what you mean here.

What I meant by this option is that we pre-generate a table (at
compile-time) and put it into `vmlinux` (and similar for each loadable
module) so that we can then just look it up within the kernel instead
of running the demangle algorithm for each symbol (e.g. when printing
a backtrace).

If you mean giving userspace that table (e.g. that distros keep in a
file in `/boot` for tools to use etc.), that could be a good idea to
avoid userspace tools requiring a library for demangling, but it would
be an improvement on top of option 1 ("# Leave demangling to
userspace") rather than an independent option (since we need to choose
what we do for backtraces), no? Or what do you mean?

Cheers,
Miguel