All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Julien Thierry <jthierry@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	Ard Biesheuvel <ardb@kernel.org>, Mark Brown <broonie@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Kees Cook <keescook@chromium.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	linux-efi <linux-efi@vger.kernel.org>,
	linux-hardening@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Peter Zijlstra <peterz@infradead.org>,
	raphael.gault@arm.com, Will Deacon <will@kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Bill Wendling <morbo@google.com>,
	swine@google.com, yonghyun@google.com
Subject: Re: [RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64
Date: Tue, 2 Feb 2021 15:01:22 -0800	[thread overview]
Message-ID: <CAKwvOdkqWyDbAvMJAd6gkc2QAEL7DiZg6_uRJ6NUE4tCip4Jvw@mail.gmail.com> (raw)
In-Reply-To: <671f1aa9-975e-1bda-6768-259adbdc24c8@redhat.com>

On Tue, Feb 2, 2021 at 12:57 AM Julien Thierry <jthierry@redhat.com> wrote:
>
>
>
> On 2/2/21 12:17 AM, Nick Desaulniers wrote:
> > On Mon, Feb 1, 2021 at 1:44 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >>
> >> On Fri, Jan 29, 2021 at 10:10:01AM -0800, Nick Desaulniers wrote:
> >>> On Wed, Jan 27, 2021 at 3:27 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >>>>
> >>>> On Wed, Jan 27, 2021 at 02:15:57PM -0800, Nick Desaulniers wrote:
> >>>>>> From: Raphael Gault <raphael.gault@arm.com>
> >>>>>>
> >>>>>> This plugins comes into play before the final 2 RTL passes of GCC and
> >>>>>> detects switch-tables that are to be outputed in the ELF and writes
> >>>>>> information in an ".discard.switch_table_info" section which will be
> >>>>>> used by objtool.
> >>>>>>
> >>>>>> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> >>>>>> [J.T.: Change section name to store switch table information,
> >>>>>>         Make plugin Kconfig be selected rather than opt-in by user,
> >>>>>>         Add a relocation in the switch_table_info that points to
> >>>>>>         the jump operation itself]
> >>>>>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> >>>>>
> >>>>> Rather than tightly couple this feature to a particular toolchain via
> >>>>> plugin, it might be nice to consider what features could be spec'ed out
> >>>>> for toolchains to implement (perhaps via a -f flag).
> >>>>
> >>>> The problem is being able to detect switch statement jump table vectors.
> >>>>
> >>>> For a given indirect branch (due to a switch statement), what are all
> >>>> the corresponding jump targets?
> >>>>
> >>>> We would need the compiler to annotate that information somehow.
> >>>
> >>> Makes sense, the compiler should have this information.  How is this
> >>> problem solved on x86?
> >>
> >> Thus far we've been able to successfully reverse engineer it on x86,
> >> though it hasn't been easy.
> >>
> >> There were some particulars for arm64 which made doing so impossible.
> >> (I don't remember the details.)
>
> The main issue is that the tables for arm64 have more indirection than x86.

I wonder if PAC or BTI also make this slightly more complex?  PAC at
least has implications for unwinders, IIUC.

>
> On x86, the dispatching jump instruction fetches the target address from
> a contiguous array of addresses based on a given offset. So the list of
> potential targets of the jump is neatly organized in a table (and sure,
> before link time these are just relocation, but still processable).
>
> On arm64 (with GCC at least), what is stored in a table is an array of
> candidate offsets from the jump instruction. And because arm64 is
> limited to 32bit instructions, the encoding often requires multiple
> instructions to compute the target address:
>
> ldr<*>  x_offset, [x_offsets_table, x_index, ...]  // load offset
> adr     x_dest_base, <addr>          // load target branch for offset 0
> add     x_dest, x_target_base, x_offset, ...  // compute final address
> br      x_dest        // jump
>
> Where this gets trickier is that (with GCC) the offsets stored in the
> table might or might not be signed constants (and this can be seen in
> GCC intermediate representations, but I do not believe this information
> is output in the final object file). And on top of that, GCC might
> decide to use offsets that are seen as unsigned during intermediate
> representation as signed offset by sign extending them in the add
> instruction.
>
> So, to handle this we'd have to track the different operation done with
> the offset, from the load to the final jump, decoding the instructions
> and deducing the potential target instructions from the table of offsets.
>
> But that is error prone as we don't really know how many instructions
> can be between the ones doing the address computation, and I remember
> some messy case of a jump table inside a jump table where tracking the
> instruction touching one or the other offset would need a lot of corner
> case handling.
>
> And this of course is just for GCC, I haven't looked at what it all
> looks like on Clang's end.

Sure, but this is what production unwinders do, and they don't require
compiler plugins, right?  I don't doubt unwinders can be made simpler
with changes to toolchain output; please work with your compiler
vendor on making such changes rather than relying on compiler plugins
to do so.

> > I think the details are pertinent to finding a portable solution.  The
> > commit message of this commit in particular doesn't document such
> > details, such as why such an approach is necessary or how the data is
> > laid out for objtool to consume it.
> >
>
> Sorry, I will need to make that clearer. The next patch explains it a
> bit [1]
>
> Basically, for simplicity, the plugin creates a new section containing

Right, this takes a focus on simplicity, at the cost of alienating a toolchain.

Ard's point about 3193c0836f20 relating to -fgcse is that when
presented with tricky cases to unwind, the simplest approach is taken.
There it was disabling a compiler specific compiler optimization, here
it's either a compiler specific compiler plugin (or disabling another
compiler optimization).  The pattern seems to be "Objtool isn't smart
enough" ... "compiler optimization disabled" or "compiler plugin
dependency."

> tables (one per jump table) of references to the jump targets, similar
> to what x86 has, except that in this case this table isn't actually used
> by runtime code and is discarded at link time. I only chose this to
> minimize what needed to be changed in objtool and because the format
> seemed simple enough.
>
> But I'm open on some alternative, whether it's a -fjump-table-info

Yes, I think we could spec out something like that.  But I would
appreciate revisiting open questions around stack validation (frame
pointers), preventing the generation of jump tables to begin with
(-fno-jump-tables) in place of making objtool more robust, or
generally the need to depend on compiler plugins.

> option added to compilers with a different format to do the links. The
> important requirement is to be able to know all the candidate targets
> for a "br <reg>" instruction.
>
> [1] https://lkml.org/lkml/2021/1/20/910
>
> Thanks,
>
> --
> Julien Thierry
>


-- 
Thanks,
~Nick Desaulniers

WARNING: multiple messages have this Message-ID (diff)
From: Nick Desaulniers <ndesaulniers@google.com>
To: Julien Thierry <jthierry@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	swine@google.com, linux-efi <linux-efi@vger.kernel.org>,
	Kees Cook <keescook@chromium.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	yonghyun@google.com, LKML <linux-kernel@vger.kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	raphael.gault@arm.com, Mark Brown <broonie@kernel.org>,
	linux-hardening@vger.kernel.org,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Bill Wendling <morbo@google.com>
Subject: Re: [RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64
Date: Tue, 2 Feb 2021 15:01:22 -0800	[thread overview]
Message-ID: <CAKwvOdkqWyDbAvMJAd6gkc2QAEL7DiZg6_uRJ6NUE4tCip4Jvw@mail.gmail.com> (raw)
In-Reply-To: <671f1aa9-975e-1bda-6768-259adbdc24c8@redhat.com>

On Tue, Feb 2, 2021 at 12:57 AM Julien Thierry <jthierry@redhat.com> wrote:
>
>
>
> On 2/2/21 12:17 AM, Nick Desaulniers wrote:
> > On Mon, Feb 1, 2021 at 1:44 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >>
> >> On Fri, Jan 29, 2021 at 10:10:01AM -0800, Nick Desaulniers wrote:
> >>> On Wed, Jan 27, 2021 at 3:27 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >>>>
> >>>> On Wed, Jan 27, 2021 at 02:15:57PM -0800, Nick Desaulniers wrote:
> >>>>>> From: Raphael Gault <raphael.gault@arm.com>
> >>>>>>
> >>>>>> This plugins comes into play before the final 2 RTL passes of GCC and
> >>>>>> detects switch-tables that are to be outputed in the ELF and writes
> >>>>>> information in an ".discard.switch_table_info" section which will be
> >>>>>> used by objtool.
> >>>>>>
> >>>>>> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> >>>>>> [J.T.: Change section name to store switch table information,
> >>>>>>         Make plugin Kconfig be selected rather than opt-in by user,
> >>>>>>         Add a relocation in the switch_table_info that points to
> >>>>>>         the jump operation itself]
> >>>>>> Signed-off-by: Julien Thierry <jthierry@redhat.com>
> >>>>>
> >>>>> Rather than tightly couple this feature to a particular toolchain via
> >>>>> plugin, it might be nice to consider what features could be spec'ed out
> >>>>> for toolchains to implement (perhaps via a -f flag).
> >>>>
> >>>> The problem is being able to detect switch statement jump table vectors.
> >>>>
> >>>> For a given indirect branch (due to a switch statement), what are all
> >>>> the corresponding jump targets?
> >>>>
> >>>> We would need the compiler to annotate that information somehow.
> >>>
> >>> Makes sense, the compiler should have this information.  How is this
> >>> problem solved on x86?
> >>
> >> Thus far we've been able to successfully reverse engineer it on x86,
> >> though it hasn't been easy.
> >>
> >> There were some particulars for arm64 which made doing so impossible.
> >> (I don't remember the details.)
>
> The main issue is that the tables for arm64 have more indirection than x86.

I wonder if PAC or BTI also make this slightly more complex?  PAC at
least has implications for unwinders, IIUC.

>
> On x86, the dispatching jump instruction fetches the target address from
> a contiguous array of addresses based on a given offset. So the list of
> potential targets of the jump is neatly organized in a table (and sure,
> before link time these are just relocation, but still processable).
>
> On arm64 (with GCC at least), what is stored in a table is an array of
> candidate offsets from the jump instruction. And because arm64 is
> limited to 32bit instructions, the encoding often requires multiple
> instructions to compute the target address:
>
> ldr<*>  x_offset, [x_offsets_table, x_index, ...]  // load offset
> adr     x_dest_base, <addr>          // load target branch for offset 0
> add     x_dest, x_target_base, x_offset, ...  // compute final address
> br      x_dest        // jump
>
> Where this gets trickier is that (with GCC) the offsets stored in the
> table might or might not be signed constants (and this can be seen in
> GCC intermediate representations, but I do not believe this information
> is output in the final object file). And on top of that, GCC might
> decide to use offsets that are seen as unsigned during intermediate
> representation as signed offset by sign extending them in the add
> instruction.
>
> So, to handle this we'd have to track the different operation done with
> the offset, from the load to the final jump, decoding the instructions
> and deducing the potential target instructions from the table of offsets.
>
> But that is error prone as we don't really know how many instructions
> can be between the ones doing the address computation, and I remember
> some messy case of a jump table inside a jump table where tracking the
> instruction touching one or the other offset would need a lot of corner
> case handling.
>
> And this of course is just for GCC, I haven't looked at what it all
> looks like on Clang's end.

Sure, but this is what production unwinders do, and they don't require
compiler plugins, right?  I don't doubt unwinders can be made simpler
with changes to toolchain output; please work with your compiler
vendor on making such changes rather than relying on compiler plugins
to do so.

> > I think the details are pertinent to finding a portable solution.  The
> > commit message of this commit in particular doesn't document such
> > details, such as why such an approach is necessary or how the data is
> > laid out for objtool to consume it.
> >
>
> Sorry, I will need to make that clearer. The next patch explains it a
> bit [1]
>
> Basically, for simplicity, the plugin creates a new section containing

Right, this takes a focus on simplicity, at the cost of alienating a toolchain.

Ard's point about 3193c0836f20 relating to -fgcse is that when
presented with tricky cases to unwind, the simplest approach is taken.
There it was disabling a compiler specific compiler optimization, here
it's either a compiler specific compiler plugin (or disabling another
compiler optimization).  The pattern seems to be "Objtool isn't smart
enough" ... "compiler optimization disabled" or "compiler plugin
dependency."

> tables (one per jump table) of references to the jump targets, similar
> to what x86 has, except that in this case this table isn't actually used
> by runtime code and is discarded at link time. I only chose this to
> minimize what needed to be changed in objtool and because the format
> seemed simple enough.
>
> But I'm open on some alternative, whether it's a -fjump-table-info

Yes, I think we could spec out something like that.  But I would
appreciate revisiting open questions around stack validation (frame
pointers), preventing the generation of jump tables to begin with
(-fno-jump-tables) in place of making objtool more robust, or
generally the need to depend on compiler plugins.

> option added to compilers with a different format to do the links. The
> important requirement is to be able to know all the candidate targets
> for a "br <reg>" instruction.
>
> [1] https://lkml.org/lkml/2021/1/20/910
>
> Thanks,
>
> --
> Julien Thierry
>


-- 
Thanks,
~Nick Desaulniers

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

  reply	other threads:[~2021-02-02 23:02 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-20 17:37 [RFC PATCH 00/17] objtool: add base support for arm64 Julien Thierry
2021-01-20 17:37 ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 01/17] tools: Add some generic functions and headers Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 02/17] tools: arm64: Make aarch64 instruction decoder available to tools Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 03/17] tools: bug: Remove duplicate definition Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 04/17] objtool: arm64: Add base definition for arm64 backend Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 05/17] objtool: arm64: Decode add/sub instructions Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 06/17] objtool: arm64: Decode jump and call related instructions Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 07/17] objtool: arm64: Decode other system instructions Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 08/17] objtool: arm64: Decode load/store instructions Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 09/17] objtool: arm64: Decode LDR instructions Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 10/17] objtool: arm64: Accept padding in code sections Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 11/17] efi: libstub: Ignore relocations for .discard sections Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64 Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-27 22:15   ` Nick Desaulniers
2021-01-27 22:15     ` Nick Desaulniers
2021-01-27 23:26     ` Josh Poimboeuf
2021-01-27 23:26       ` Josh Poimboeuf
2021-01-29 18:10       ` Nick Desaulniers
2021-01-29 18:10         ` Nick Desaulniers
2021-02-01 21:44         ` Josh Poimboeuf
2021-02-01 21:44           ` Josh Poimboeuf
2021-02-01 23:17           ` Nick Desaulniers
2021-02-01 23:17             ` Nick Desaulniers
2021-02-02  0:02             ` Josh Poimboeuf
2021-02-02  0:02               ` Josh Poimboeuf
2021-02-02 14:24               ` David Laight
2021-02-02 14:24                 ` David Laight
2021-02-02 22:33               ` Nick Desaulniers
2021-02-02 22:33                 ` Nick Desaulniers
2021-02-02 23:36                 ` Josh Poimboeuf
2021-02-02 23:36                   ` Josh Poimboeuf
2021-02-02 23:52                   ` Nick Desaulniers
2021-02-02 23:52                     ` Nick Desaulniers
2021-02-02  8:57             ` Julien Thierry
2021-02-02  8:57               ` Julien Thierry
2021-02-02 23:01               ` Nick Desaulniers [this message]
2021-02-02 23:01                 ` Nick Desaulniers
2021-02-03  0:14                 ` Josh Poimboeuf
2021-02-03  0:14                   ` Josh Poimboeuf
2021-02-03 11:57                   ` Peter Zijlstra
2021-02-03 11:57                     ` Peter Zijlstra
2021-02-03 13:04                   ` Mark Brown
2021-02-03 13:04                     ` Mark Brown
2021-02-03 13:58                   ` Mark Rutland
2021-02-03 13:58                     ` Mark Rutland
2021-02-03  8:11                 ` Julien Thierry
2021-02-03  8:11                   ` Julien Thierry
2021-02-09 16:30                 ` Daniel Kiss
2021-02-09 16:30                   ` Daniel Kiss
2021-01-20 17:37 ` [RFC PATCH 13/17] objtool: arm64: Implement functions to add switch tables alternatives Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 14/17] objtool: arm64: Cache section with switch table information Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 15/17] objtool: arm64: Handle supported relocations in alternatives Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:37 ` [RFC PATCH 16/17] objtool: arm64: Ignore replacement section for alternative callback Julien Thierry
2021-01-20 17:37   ` Julien Thierry
2021-01-20 17:38 ` [RFC PATCH 17/17] objtool: arm64: Enable stack validation for arm64 Julien Thierry
2021-01-20 17:38   ` Julien Thierry
2021-01-21  5:39   ` kernel test robot
2021-01-21  9:03 ` [RFC PATCH 00/17] objtool: add base support " Ard Biesheuvel
2021-01-21  9:03   ` Ard Biesheuvel
2021-01-21 10:26   ` Julien Thierry
2021-01-21 10:26     ` Julien Thierry
2021-01-21 11:08     ` Ard Biesheuvel
2021-01-21 11:08       ` Ard Biesheuvel
2021-01-21 11:23       ` Peter Zijlstra
2021-01-21 11:23         ` Peter Zijlstra
2021-01-21 11:48         ` Ard Biesheuvel
2021-01-21 11:48           ` Ard Biesheuvel
2021-01-21 18:54           ` Josh Poimboeuf
2021-01-21 18:54             ` Josh Poimboeuf
2021-01-22 17:43             ` Mark Brown
2021-01-22 17:43               ` Mark Brown
2021-01-22 17:54               ` Ard Biesheuvel
2021-01-22 17:54                 ` Ard Biesheuvel
2021-01-28 22:10                 ` Madhavan T. Venkataraman
2021-01-28 22:10                   ` Madhavan T. Venkataraman
2021-01-29 15:47                   ` Mark Brown
2021-01-22 21:15               ` Madhavan T. Venkataraman
2021-01-22 21:15                 ` Madhavan T. Venkataraman
2021-01-22 21:43                 ` Ard Biesheuvel
2021-01-22 21:43                   ` Ard Biesheuvel
2021-01-22 21:44                   ` Madhavan T. Venkataraman
2021-01-22 21:44                     ` Madhavan T. Venkataraman
2021-01-25 21:19                   ` Josh Poimboeuf
2021-01-25 21:19                     ` Josh Poimboeuf
2021-01-22 21:16               ` Madhavan T. Venkataraman
2021-01-22 21:16                 ` Madhavan T. Venkataraman
2021-01-21 13:23       ` Julien Thierry
2021-01-21 13:23         ` Julien Thierry
2021-01-21 14:23         ` Mark Brown
2021-01-21 14:23           ` Mark Brown

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=CAKwvOdkqWyDbAvMJAd6gkc2QAEL7DiZg6_uRJ6NUE4tCip4Jvw@mail.gmail.com \
    --to=ndesaulniers@google.com \
    --cc=ardb@kernel.org \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=jpoimboe@redhat.com \
    --cc=jthierry@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=morbo@google.com \
    --cc=peterz@infradead.org \
    --cc=raphael.gault@arm.com \
    --cc=swine@google.com \
    --cc=will@kernel.org \
    --cc=yonghyun@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.