linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: madvenka@linux.microsoft.com
Cc: mark.rutland@arm.com, broonie@kernel.org, ardb@kernel.org,
	nobuta.keiya@fujitsu.com, sjitindarsingh@gmail.com,
	catalin.marinas@arm.com, will@kernel.org, jmorris@namei.org,
	linux-arm-kernel@lists.infradead.org,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 0/9] arm64: livepatch: Use DWARF Call Frame Information for frame pointer validation
Date: Thu, 7 Apr 2022 17:21:47 -0700	[thread overview]
Message-ID: <20220408002147.pk7clzruj6sawj7z@treble> (raw)
In-Reply-To: <20220407202518.19780-1-madvenka@linux.microsoft.com>

On Thu, Apr 07, 2022 at 03:25:09PM -0500, madvenka@linux.microsoft.com wrote:
> The solution
> ============
> 
> The goal here is to use the absolute minimum CFI needed to compute the FP at
> every instruction address. The unwinder can compute the FP in each frame,
> compare the actual FP with the computed one and validate the actual FP.
> 
> Objtool is enhanced to parse the CFI, extract just the rules required,
> encode them in compact data structures and create special sections for
> the rules. The unwinder uses the special sections to find the rules for
> a given instruction address and compute the FP.
> 
> Objtool can be invoked as follows:
> 
> 	objtool dwarf generate <object-file>

Hi Madhaven,

This is quite interesting.  And it's exactly the kind of crazy idea I
can appreciate ;-)

Some initial thoughts:


1)

I have some concerns about DWARF's reliability, especially considering
a) inline asm, b) regular asm, and c) the kernel's tendency to push
compilers to their limits.

BUT, supplementing the frame pointer unwinding with DWARF, rather than
just relying on DWARF alone, does help a LOT.

I guess the hope is that cross-checking two "mostly reliable" things
against each other (frame pointers and DWARF) will give a reliable
result ;-)

In a general sense, I've never looked at DWARF's reliability, even for
just normal C code.  It would be good to have some way of knowing that
DWARF looks mostly sane for both GCC and Clang.  For example, maybe
somehow cross-checking it with objtool's knowledge.  And then of course
we'd have to hope that it stays bug-free in future compilers.

I'd also be somewhat concerned about assembly.  Since there's nothing
ensuring the unwind hints are valid, and will stay valid over time, I
wonder how likely it would be for that to break, and what the
implications would be.  Most likely I guess it would break silently, but
then get caught by the frame pointer cross-checking.  So a broken hint
might not get noticed for a long time, but at least it (hopefully)
wouldn't break reliable unwinding.

Also, inline asm can sometimes do stack hacks like
"push;do_something;pop" which isn't visible to the toolchain.  But
again, hopefully the frame pointer checking would fail and mark it
unreliable.

So I do have some worries about DWARF, but the fact that it's getting
"fact checked" by frame pointers might be sufficient.


2)

If I understand correctly, objtool is converting parts of DWARF to a new
format which can then be read by the kernel.  In that case, please don't
call it DWARF as that will cause a lot of confusion.

There are actually several similarities between your new format and ORC,
which is also an objtool-created DWARF alternative.  It would be
interesting to see if they could be combined somehow.


3)

Objtool has become an integral part of x86-64, due to security and
performance features and toolchain workarounds.

Not *all* of its features require the full "branch validation" which
follows all code paths -- and was the hardest part to get right -- but
several features *do* need that: stack validation, ORC, uaccess
validation, noinstr validation.

Objtool has been picking up a lot of steam (and features) lately, with
more features currently in active development.  And lately there have
been renewed patches for porting it to powerpc and arm64 (and rumors of
s390).

If arm64 ever wants one of those features -- particularly a "branch
validation" based feature -- I think it would make more sense to just do
the stack validation in objtool, rather than the DWARF supplementation
approach.

Just to give an idea of what objtool already supports and how useful it
has become for x86, here's an excerpt from some documentation I've been
working on, since I'm in the middle of rewriting the interface to make
it more modular.  This is a list of all its current features:


Features
--------

Objtool has the following features:


- Stack unwinding metadata validation -- useful for helping to ensure
  stack traces are reliable for live patching

- ORC unwinder metadata generation -- a faster and more precise
  alternative to frame pointer based unwinding

- Retpoline validation -- ensures that all indirect calls go through
  retpoline thunks, for Spectre v2 mitigations

- Retpoline call site annotation -- annotates all retpoline thunk call
  sites, enabling the kernel to patch them inline, to prevent "thunk
  funneling" for both security and performance reasons

- Non-instrumentation validation -- validates non-instrumentable
  ("noinstr") code rules, preventing unexpected instrumentation in
  low-level C entry code

- Static call annotation -- annotates static call sites, enabling the
  kernel to implement inline static calls, a faster alternative to some
  indirect branches

- Uaccess validation -- validates uaccess rules for a proper safe
  implementation of Supervisor Mode Access Protection (SMAP)

- Straight Line Speculation validation -- validates certain SLS
  mitigations

- Indirect Branch Tracking validation -- validates Intel CET IBT rules
  to ensure that all functions referenced by function pointers have
  corresponding ENDBR instructions

- Indirect Branch Tracking annotation -- annotates unused ENDBR
  instruction sites, enabling the kernel to "seal" them (replace them
  with NOPs) to further harden IBT

- Function entry annotation -- annotates function entries, enabling
  kernel function tracing

- Other toolchain hacks which will go unmentioned at this time...

-- 
Josh


  parent reply	other threads:[~2022-04-08  0:21 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <95691cae4f4504f33d0fc9075541b1e7deefe96f>
2022-01-17 14:55 ` [PATCH v13 00/11] arm64: Reorganize the unwinder and implement stack trace reliability checks madvenka
2022-01-17 14:55   ` [PATCH v13 01/11] arm64: Remove NULL task check from unwind_frame() madvenka
2022-01-17 14:55   ` [PATCH v13 02/11] arm64: Rename unwinder functions madvenka
2022-01-17 14:56   ` [PATCH v13 03/11] arm64: Rename stackframe to unwind_state madvenka
2022-01-17 14:56   ` [PATCH v13 04/11] arm64: Split unwind_init() madvenka
2022-02-02 18:44     ` Mark Brown
2022-02-03  0:26       ` Madhavan T. Venkataraman
2022-02-03  0:39         ` Madhavan T. Venkataraman
2022-02-03 11:29           ` Mark Brown
2022-02-15 13:07     ` Mark Rutland
2022-02-15 18:04       ` Madhavan T. Venkataraman
2022-01-17 14:56   ` [PATCH v13 05/11] arm64: Copy the task argument to unwind_state madvenka
2022-02-02 18:45     ` Mark Brown
2022-02-15 13:22     ` Mark Rutland
2022-02-22 16:53       ` Madhavan T. Venkataraman
2022-01-17 14:56   ` [PATCH v13 06/11] arm64: Use stack_trace_consume_fn and rename args to unwind() madvenka
2022-02-02 18:46     ` Mark Brown
2022-02-03  0:34       ` Madhavan T. Venkataraman
2022-02-03 11:30         ` Mark Brown
2022-02-03 14:45           ` Madhavan T. Venkataraman
2022-02-15 13:39     ` Mark Rutland
2022-02-15 18:12       ` Madhavan T. Venkataraman
2022-03-07 16:51       ` Madhavan T. Venkataraman
2022-03-07 17:01         ` Mark Brown
2022-03-08 22:00           ` Madhavan T. Venkataraman
2022-03-09 11:47             ` Mark Brown
2022-03-09 15:34               ` Madhavan T. Venkataraman
2022-03-10  8:33               ` Miroslav Benes
2022-03-10 12:36                 ` Madhavan T. Venkataraman
2022-03-16  3:43               ` Josh Poimboeuf
2022-04-08 14:44         ` Mark Rutland
2022-04-08 17:58           ` Mark Rutland
2022-04-10 17:42             ` Madhavan T. Venkataraman
2022-04-10 17:33           ` Madhavan T. Venkataraman
2022-04-10 17:45           ` Madhavan T. Venkataraman
2022-01-17 14:56   ` [PATCH v13 07/11] arm64: Make the unwind loop in unwind() similar to other architectures madvenka
2022-01-17 14:56   ` [PATCH v13 08/11] arm64: Introduce stack trace reliability checks in the unwinder madvenka
2022-01-17 14:56   ` [PATCH v13 09/11] arm64: Create a list of SYM_CODE functions, check return PC against list madvenka
2022-01-17 14:56   ` [PATCH v13 10/11] arm64: Introduce arch_stack_walk_reliable() madvenka
2022-01-17 14:56   ` [PATCH v13 11/11] arm64: Select HAVE_RELIABLE_STACKTRACE madvenka
2022-01-25  5:21     ` nobuta.keiya
2022-01-25 13:43       ` Madhavan T. Venkataraman
2022-01-26 10:20         ` nobuta.keiya
2022-01-26 17:14           ` Madhavan T. Venkataraman
2022-01-27  1:13             ` nobuta.keiya
2022-01-26 17:16       ` Mark Brown
2022-04-07 20:25 ` [RFC PATCH v1 0/9] arm64: livepatch: Use DWARF Call Frame Information for frame pointer validation madvenka
2022-04-07 20:25   ` [RFC PATCH v1 1/9] objtool: Parse DWARF Call Frame Information in object files madvenka
2022-04-07 20:25   ` [RFC PATCH v1 2/9] objtool: Generate DWARF rules and place them in a special section madvenka
2022-04-07 20:25   ` [RFC PATCH v1 3/9] dwarf: Build the kernel with DWARF information madvenka
2022-04-07 20:25   ` [RFC PATCH v1 4/9] dwarf: Implement DWARF rule processing in the kernel madvenka
2022-04-07 20:25   ` [RFC PATCH v1 5/9] dwarf: Implement DWARF support for modules madvenka
2022-04-07 20:25   ` [RFC PATCH v1 6/9] arm64: unwinder: Add a reliability check in the unwinder based on DWARF CFI madvenka
2022-04-07 20:25   ` [RFC PATCH v1 7/9] arm64: dwarf: Implement unwind hints madvenka
2022-04-07 20:25   ` [RFC PATCH v1 8/9] dwarf: Miscellaneous changes required for enabling livepatch madvenka
2022-04-07 20:25   ` [RFC PATCH v1 9/9] dwarf: Enable livepatch for ARM64 madvenka
2022-04-08  0:21   ` Josh Poimboeuf [this message]
2022-04-08 11:41     ` [RFC PATCH v1 0/9] arm64: livepatch: Use DWARF Call Frame Information for frame pointer validation Peter Zijlstra
2022-04-11 17:26       ` Madhavan T. Venkataraman
2022-04-11 17:18     ` Madhavan T. Venkataraman
2022-04-12  8:32       ` Chen Zhongjin
2022-04-16  0:56         ` Josh Poimboeuf
2022-04-18 12:28           ` Chen Zhongjin
2022-04-18 16:11             ` Josh Poimboeuf
2022-04-18 18:38               ` Madhavan T. Venkataraman
     [not found]       ` <844b3ede-eddb-cbe6-80e0-3529e2da2eb6@huawei.com>
2022-04-12 17:27         ` Madhavan T. Venkataraman
2022-04-16  1:07       ` Josh Poimboeuf
2022-04-14 14:11     ` Madhavan T. Venkataraman
2022-04-08 10:55   ` Peter Zijlstra
2022-04-08 11:54     ` Peter Zijlstra
2022-04-08 14:34       ` Josh Poimboeuf
2022-04-10 17:47     ` Madhavan T. Venkataraman
2022-04-11 16:34       ` Josh Poimboeuf
2022-04-08 12:06   ` Peter Zijlstra
2022-04-11 17:35     ` Madhavan T. Venkataraman

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=20220408002147.pk7clzruj6sawj7z@treble \
    --to=jpoimboe@redhat.com \
    --cc=ardb@kernel.org \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=jmorris@namei.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=madvenka@linux.microsoft.com \
    --cc=mark.rutland@arm.com \
    --cc=nobuta.keiya@fujitsu.com \
    --cc=sjitindarsingh@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).