linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Valentin Obst <kernel@valentinobst.de>
To: samitolvanen@google.com, aliceryhl@google.com
Cc: Jamie.Cunliffe@arm.com, a.hindborg@samsung.com,
	alex.gaynor@gmail.com, ardb@kernel.org, benno.lossin@proton.me,
	bjorn3_gh@protonmail.com, boqun.feng@gmail.com,
	broonie@kernel.org, catalin.marinas@arm.com, gary@garyguo.net,
	keescook@chromium.org, linux-arm-kernel@lists.infradead.org,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	mark.rutland@arm.com, masahiroy@kernel.org, maz@kernel.org,
	nathan@kernel.org, ndesaulniers@google.com, nicolas@fjasle.eu,
	ojeda@kernel.org, rust-for-linux@vger.kernel.org,
	wedsonaf@gmail.com, will@kernel.org,
	Valentin Obst <kernel@valentinobst.de>
Subject: Re: [PATCH] rust: add flags for shadow call stack sanitizer
Date: Tue,  5 Mar 2024 00:31:51 +0100	[thread overview]
Message-ID: <20240304233151.248925-1-kernel@valentinobst.de> (raw)
In-Reply-To: <CABCJKuem3GbLO-G7+wi8LPA8rFgNzFVjNof7zcAO1UGJR4u44Q@mail.gmail.com>

> >
> > Add flags to support the shadow call stack sanitizer, both in the
> > dynamic and non-dynamic modes.
> >
> > Right now, the compiler will emit the warning "unknown feature specified
> > for `-Ctarget-feature`: `reserve-x18`". However, the compiler still
> > passes it to the codegen backend, so the flag will work just fine. Once
> > rustc starts recognizing the flag (or provides another way to enable the
> > feature), it will stop emitting this warning. See [1] for the relevant
> > issue.
> >
> > Currently, the compiler thinks that the aarch64-unknown-none target
> > doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if
> > you enable shadow call stack in non-dynamic mode. However, I still think
> > it is reasonable to add the flag now, as it will at least fail the build
> > when using an invalid configuration, until the Rust compiler is fixed to
> > list -Zsanitizer=shadow-call-stack as supported for the target. See [2]
> > for the feature request to add this.
> >
> > I have tested this change with Rust Binder on an Android device using
> > CONFIG_DYNAMIC_SCS. Without the -Ctarget-feature=+reserve-x18 flag, the
> > phone crashes immediately on boot, and with the flag, the phone appears
> > to work normally.
> >
> > Link: https://github.com/rust-lang/rust/issues/121970 [1]
> > Link: https://github.com/rust-lang/rust/issues/121972 [2]
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > It's not 100% clear to me whether this patch is enough for full SCS
> > support in Rust. If there is some issue where this makes things compile
> > and work without actually applying SCS to the Rust code, please let me
> > know. Is there some way to verify that it is actually working?
>
> Perhaps you could write a Rust version of the CFI_BACKWARD test in LKDTM?
>
> Alternatively, the simplest way to verify this is to look at the
> disassembly and verify that shadow stack instructions are emitted to
> Rust functions too. In case of dynamic SCS, you might need to dump
> function memory in a debugger to verify that PAC instructions were
> patched correctly. If they're not, the code will just quietly continue
> working without using shadow stacks.

Was just in the process of doing that:

- `paciasp`/`autiasp` pairs are emitted for functions in Rust modules.
- Rust modules have no `.init.eh_frame` section, which implies that
  `module_finalize` is _not_ rewriting the pac insns when SCS is dynamic.
  - Confirmed that behavior in the debugger (C modules and the C part of the
    kernel are correctly rewritten, Rust modules execute with
    `paciasp`/`autiasp` still in place).
- Kernel boots just fine with Rust kunit tests, tested with and without dynamic
  SCS, i.e., on a CPU that supports PAC/BTI and one that does not.
- Rust sample modules load and unload without problems as well.
- `x18` is indeed not used in the codegen.

I guess we might be able to get this working when we tweak the build system
to emit the missing section for Rust modules.

    - Best Valentin

>
> > This patch raises the question of whether we should change the Rust
> > aarch64 support to use a custom target.json specification. If we do
> > that, then we can fix both the warning for dynamic SCS and the
> > build-failure for non-dynamic SCS without waiting for a new version of
> > rustc with the mentioned issues fixed.
>
> Sure, having a custom target description for the kernel might be
> useful for other purposes too. In the meantime:
>
> Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
>
> Sami
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-03-04 23:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-04 13:17 [PATCH] rust: add flags for shadow call stack sanitizer Alice Ryhl
2024-03-04 20:09 ` Sami Tolvanen
2024-03-04 23:31   ` Valentin Obst [this message]
2024-03-05  7:15     ` Alice Ryhl
2024-03-05 11:20       ` Valentin Obst
2024-03-05 11:28         ` Alice Ryhl
2024-03-05 11:31           ` Alice Ryhl
2024-03-05 12:08             ` Alice Ryhl
2024-03-05 12:09   ` 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=20240304233151.248925-1-kernel@valentinobst.de \
    --to=kernel@valentinobst.de \
    --cc=Jamie.Cunliffe@arm.com \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=ardb@kernel.org \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=gary@garyguo.net \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=maz@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=samitolvanen@google.com \
    --cc=wedsonaf@gmail.com \
    --cc=will@kernel.org \
    /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).