linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
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>
Subject: Re: [PATCH v6 17/23] scripts: decode_stacktrace: demangle Rust symbols
Date: Sat, 7 May 2022 12:21:14 +0200	[thread overview]
Message-ID: <CANiq72m6MaaH-o9McMuDwicosA2xgpU5ZEw0OduR0JT3w7p_fQ@mail.gmail.com> (raw)
In-Reply-To: <202205070122.B240F989@keescook>

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

  reply	other threads:[~2022-05-07 10:21 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-07  5:23 [PATCH v6 00/23] Rust support Miguel Ojeda
2022-05-07  5:23 ` [PATCH v6 01/23] kallsyms: avoid hardcoding the buffer size Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 02/23] kallsyms: support "big" kernel symbols Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 03/23] kallsyms: increase maximum kernel symbol length to 512 Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 04/23] kunit: take `kunit_assert` as `const` Miguel Ojeda
2022-05-12 19:01   ` Brendan Higgins
2022-05-07  5:24 ` [PATCH v6 05/23] rust: add C helpers Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 06/23] rust: add `compiler_builtins` crate Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 07/23] rust: import upstream `alloc` crate Miguel Ojeda
2022-05-07  9:23   ` Kees Cook
2022-05-07  9:33     ` Miguel Ojeda
2022-05-07 17:06       ` Kees Cook
2022-05-07 17:30   ` Linus Torvalds
2022-05-07 19:34     ` Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 08/23] rust: adapt `alloc` crate to the kernel Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 09/23] rust: add `build_error` crate Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 10/23] rust: add `macros` crate Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 11/23] rust: add `kernel` crate's `sync` module Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 12/23] rust: add `kernel` crate Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 13/23] rust: export generated symbols Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 14/23] vsprintf: add new `%pA` format specifier Miguel Ojeda
2022-05-07  8:19   ` Kees Cook
2022-05-07  9:35     ` Miguel Ojeda
2022-05-10  8:38   ` Petr Mladek
2022-05-10 10:45     ` Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 15/23] scripts: add `rustdoc_test_{builder,gen}.py` scripts Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 16/23] scripts: add `generate_rust_analyzer.py` scripts Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 17/23] scripts: decode_stacktrace: demangle Rust symbols Miguel Ojeda
2022-05-07  8:32   ` Kees Cook
2022-05-07 10:21     ` Miguel Ojeda [this message]
2022-05-07 17:09       ` Kees Cook
2022-05-07  5:24 ` [PATCH v6 18/23] docs: add Rust documentation Miguel Ojeda
2022-05-07  8:15   ` Kees Cook
2022-05-07  8:45     ` Miguel Ojeda
2022-05-09  4:02   ` Akira Yokosawa
2022-05-09 10:41     ` Miguel Ojeda
2022-05-09 14:56       ` Akira Yokosawa
2022-05-09 22:37         ` Jonathan Corbet
2022-05-10 11:57           ` Miguel Ojeda
2022-05-09 22:32   ` Jonathan Corbet
2022-05-10  3:14     ` Gaelan Steele
2022-05-10  5:53       ` Josh Triplett
2022-05-11 13:49     ` Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 19/23] Kbuild: add Rust support Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 20/23] samples: add Rust examples Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 21/23] MAINTAINERS: Rust Miguel Ojeda
2022-05-07  8:06   ` Kees Cook
2022-05-07  5:24 ` [PATCH v6 22/23] [RFC] drivers: gpio: PrimeCell PL061 in Rust Miguel Ojeda
2022-05-07  5:24 ` [PATCH v6 23/23] [RFC] drivers: android: Binder IPC " Miguel Ojeda
2022-05-07  7:55   ` Kees Cook
2022-05-07  8:13     ` Greg Kroah-Hartman
2022-05-09 17:52       ` Todd Kjos
2022-05-07  8:06 ` [PATCH v6 00/23] Rust support Kees Cook
2022-05-08 18:06   ` Matthew Wilcox
2022-05-09  9:39   ` Wei Liu
2022-05-07  9:29 ` David Gow
2022-05-07 15:03   ` Miguel Ojeda
2022-05-10  4:44     ` David Gow
2022-05-10 11:36       ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CANiq72m6MaaH-o9McMuDwicosA2xgpU5ZEw0OduR0JT3w7p_fQ@mail.gmail.com \
    --to=miguel.ojeda.sandonis@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jarkko@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=wedsonaf@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).