All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Julien Thierry <jthierry@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>
Subject: Re: [RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64
Date: Mon, 1 Feb 2021 15:17:40 -0800	[thread overview]
Message-ID: <CAKwvOdmgNPSpY2oPHFr8EKGXYJbm7K9gySKFgyn4FERa9nTXmw@mail.gmail.com> (raw)
In-Reply-To: <20210201214423.dhsma73k7ccscovm@treble>

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.)

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.

> > > > Distributions (like Android, CrOS) wont be able to use such a feature as
> > > > is.
> > >
> > > Would a Clang plugin be out of the question?
> >
> > Generally, we frown on out of tree kernel modules for a couple reasons.
> >
> > Maintaining ABI compatibility when the core kernel changes is
> > generally not instantaneous; someone has to notice the ABI has changed
> > which will be more delayed than if the module was in tree.  Worse is
> > when semantics subtly change.  While we must not break userspace, we
> > provide no such guarantees within the kernel proper.
> >
> > Also, it's less likely that out of tree kernel modules have been
> > reviewed by kernel developers.  They may not have the same quality,
> > use the recommended interfaces, follow coding conventions, etc..
> >
> > Oh, did I say "out of tree kernel modules?"  I meant "compiler
> > plugins."  But it's two different sides of the same coin to me.
>
> I thought Android already relied on OOT modules.

Android Common Kernel does not *rely* on OOT modules or compiler
plugins.  Android doesn't forbid vendors from using OOT modules,
though, just as the mainline kernel does not prevent modules from
being built out of tree, or users from insmod'ing them.  It's
certainly a pragmatic approach; idealism doesn't work for all OEMs.

Personally, I lean more towards idealistic; I prefer in-tree modules,
dislike compiler plugins (for much the same reasons), and idealize
loose coupling of software components. This series looks to me like it
tightly couples arm64 live patching to objtool and GCC.

On the earlier thread, Julien writes:

>> I think most people interested in livepatching are using GCC built
>> kernels, but I could be mistaken (althought in the long run, both
>> compilers should be supported, and yes, I realize the objtool solution
>> currently only would support GCC).

Google's production kernels are using livepatching and are built with
Clang.  Getting similar functionality working for arm64 would be of
interest.

> GCC plugins generally enforce the exact same GCC version for OOT
> modules.  So there's no ABI to worry about.  I assume Clang does the
> same?
>
> Or did I miss your point?

I think so.  It would seem that the current solution for stack
validation would require multiple different compiler plugins to work
reliably.  If jump tables are an issue, I don't see why you wouldn't
pursue a more portable option like -fno-jump-tables first, then work
to claw back any performance loss from that, if relevant?  Then
there's no complaint about toolchain specific implementations or tight
coupling.

Objtool support on arm64 is interesting to me though, because it has
found bugs in LLVM codegen. That alone is extremely valuable.  But not
it's not helpful if it's predicated or tightly coupled to GCC, as this
series appears to do.  The idea of rebuilding control flow from binary
analysis and using that to find codegen bugs is a really cool idea
(novel, even? idk), and I wish we had some analog for userspace
binaries that could perform similar checks.

>
> > FWIW, I think the approach taken by -mstack-protector-guard-reg= is a
> > useful case study.  It was prototyped as a GCC extension, then added
> > to GCC proper, then added to LLVM (currently only x86, but most of the
> > machinery is in place in the compiler to get it running on arm64).  My
> > recommendation is to skip the plugin part and work on a standard
> > interface for compilers to implement, with input from compiler
> > developers.
>
> I like the idea.  Is there a recommended forum for such discussions?
> Just an email to GCC/Clang development lists?

linux-toolchains@vger.kernel.org is probably a good start.

--
Thanks,
~Nick Desaulniers

WARNING: multiple messages have this Message-ID (diff)
From: Nick Desaulniers <ndesaulniers@google.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Bill Wendling <morbo@google.com>,
	linux-efi <linux-efi@vger.kernel.org>,
	Julien Thierry <jthierry@redhat.com>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	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, Will Deacon <will@kernel.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64
Date: Mon, 1 Feb 2021 15:17:40 -0800	[thread overview]
Message-ID: <CAKwvOdmgNPSpY2oPHFr8EKGXYJbm7K9gySKFgyn4FERa9nTXmw@mail.gmail.com> (raw)
In-Reply-To: <20210201214423.dhsma73k7ccscovm@treble>

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.)

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.

> > > > Distributions (like Android, CrOS) wont be able to use such a feature as
> > > > is.
> > >
> > > Would a Clang plugin be out of the question?
> >
> > Generally, we frown on out of tree kernel modules for a couple reasons.
> >
> > Maintaining ABI compatibility when the core kernel changes is
> > generally not instantaneous; someone has to notice the ABI has changed
> > which will be more delayed than if the module was in tree.  Worse is
> > when semantics subtly change.  While we must not break userspace, we
> > provide no such guarantees within the kernel proper.
> >
> > Also, it's less likely that out of tree kernel modules have been
> > reviewed by kernel developers.  They may not have the same quality,
> > use the recommended interfaces, follow coding conventions, etc..
> >
> > Oh, did I say "out of tree kernel modules?"  I meant "compiler
> > plugins."  But it's two different sides of the same coin to me.
>
> I thought Android already relied on OOT modules.

Android Common Kernel does not *rely* on OOT modules or compiler
plugins.  Android doesn't forbid vendors from using OOT modules,
though, just as the mainline kernel does not prevent modules from
being built out of tree, or users from insmod'ing them.  It's
certainly a pragmatic approach; idealism doesn't work for all OEMs.

Personally, I lean more towards idealistic; I prefer in-tree modules,
dislike compiler plugins (for much the same reasons), and idealize
loose coupling of software components. This series looks to me like it
tightly couples arm64 live patching to objtool and GCC.

On the earlier thread, Julien writes:

>> I think most people interested in livepatching are using GCC built
>> kernels, but I could be mistaken (althought in the long run, both
>> compilers should be supported, and yes, I realize the objtool solution
>> currently only would support GCC).

Google's production kernels are using livepatching and are built with
Clang.  Getting similar functionality working for arm64 would be of
interest.

> GCC plugins generally enforce the exact same GCC version for OOT
> modules.  So there's no ABI to worry about.  I assume Clang does the
> same?
>
> Or did I miss your point?

I think so.  It would seem that the current solution for stack
validation would require multiple different compiler plugins to work
reliably.  If jump tables are an issue, I don't see why you wouldn't
pursue a more portable option like -fno-jump-tables first, then work
to claw back any performance loss from that, if relevant?  Then
there's no complaint about toolchain specific implementations or tight
coupling.

Objtool support on arm64 is interesting to me though, because it has
found bugs in LLVM codegen. That alone is extremely valuable.  But not
it's not helpful if it's predicated or tightly coupled to GCC, as this
series appears to do.  The idea of rebuilding control flow from binary
analysis and using that to find codegen bugs is a really cool idea
(novel, even? idk), and I wish we had some analog for userspace
binaries that could perform similar checks.

>
> > FWIW, I think the approach taken by -mstack-protector-guard-reg= is a
> > useful case study.  It was prototyped as a GCC extension, then added
> > to GCC proper, then added to LLVM (currently only x86, but most of the
> > machinery is in place in the compiler to get it running on arm64).  My
> > recommendation is to skip the plugin part and work on a standard
> > interface for compilers to implement, with input from compiler
> > developers.
>
> I like the idea.  Is there a recommended forum for such discussions?
> Just an email to GCC/Clang development lists?

linux-toolchains@vger.kernel.org is probably a good start.

--
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-01 23:19 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 [this message]
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
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=CAKwvOdmgNPSpY2oPHFr8EKGXYJbm7K9gySKFgyn4FERa9nTXmw@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=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 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.