From: Ard Biesheuvel <ardb@kernel.org> To: Julien Thierry <jthierry@redhat.com> Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Masahiro Yamada <masahiroy@kernel.org>, Kees Cook <keescook@chromium.org>, Michal Marek <michal.lkml@markovi.net>, Josh Poimboeuf <jpoimboe@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Mark Rutland <mark.rutland@arm.com>, Mark Brown <broonie@kernel.org>, linux-efi <linux-efi@vger.kernel.org>, linux-hardening@vger.kernel.org Subject: Re: [RFC PATCH 00/17] objtool: add base support for arm64 Date: Thu, 21 Jan 2021 10:03:17 +0100 [thread overview] Message-ID: <CAMj1kXHO0wgcZ4ZDxj1vS9s7Szfpz8Nz=SAW_=Dnnjy+S9AtyQ@mail.gmail.com> (raw) In-Reply-To: <20210120173800.1660730-1-jthierry@redhat.com> Hello Julien, On Wed, 20 Jan 2021 at 18:38, Julien Thierry <jthierry@redhat.com> wrote: > > Hi, > > This series enables objtool to start doing stack validation on arm64 > kernel builds. Could we elaborate on this point, please? 'Stack validation' means getting an accurate picture of all kernel code that will be executed at some point in the future, due to the fact that there are stack frames pointing to them. And this ability is essential in order to do live patching safely? If this is the goal, I wonder whether this is the right approach for arm64 (or for any other architecture, for that matter) Parsing/decoding the object code and even worse, relying on GCC plugins to annotate some of the idioms as they are being generated, in order to infer intent on the part of the compiler goes *way* beyond what we should be comfortable with. The whole point of this exercise is to guarantee that there are no false positives when it comes to deciding whether the kernel is in a live patchable state, and I don't see how we can ever provide such a guarantee when it is built on such a fragile foundation. If we want to ensure that the stack contents are always an accurate reflection of the real call stack, we should work with the toolchain folks to identify issues that may interfere with this, and implement controls over these behaviors that we can decide to use in the build. In the past, I have already proposed adding a 'kernel' code model to the AArch64 compiler that guarantees certain things, such as adrp/add for symbol references, and no GOT indirections for position independent code. Inhibiting optimizations that may impact our ability to infer the real call stack from the stack contents is something we might add here as well. Another thing that occurred to me is that inferring which kernel code is actually live in terms of pending function returns could be inferred much more easily from a shadow call stack, which is a thing we already implement for Clang builds. In summary, I would not be in favor of enabling objtool on arm64 at all until we have exhausted other options for providing the functionality that we need it for (given that objtool provides many other things that only x86 cares about, IIUC) -- Ard. > It relies on the previous series I sent, refactoring > the arm64 decoder [1]. > > First, the aarch64 instruction decoder needed to be made available > to code under tools/. This is done in a similar manner to x86 > instruction decoded. One limitation I encountered there is that most > of aarch64 instruction decoder is __kprobe annotated. To bypass that > it remove the kprobe include and had to add an empty __kprobe > definition, but I'd welcome a proper solution to that. > > Then instruction semantics are progressively added so objtool can track > the stack state through the execution flow. > There are a few things that needed consideration: > - Generation of constants within executable sections, these either > caused objtool to fail decoding or to wrongly decode constants > as jumps or other instructions affecting execution flow and > causing confusion. To solve this, tracking locations referenced > by instructions using literals was needed. > - Jump tables from switch statements in aarch64 don't have enough > information to link branches with the branch instruction leading to > them. For this, we use a gcc plugin to add some information to establish > those missing links in a format that is already supported by objtool > > With this, there are still some errors when building with objtool. A > number of cleanups/annotations are needed on the arm64, as well as > handling SYM_DATA objects in objtool. > > Those changes can be found on top of this branch here: > git clone https://github.com/julien-thierry/linux.git -b objtoolxarm64-latest > > But it would be nice to have some feedback on this before I start submitting everyting. > > [1] https://lkml.org/lkml/2021/1/20/791 > > Thanks, > > Julien > > --> > > Julien Thierry (15): > tools: Add some generic functions and headers > tools: arm64: Make aarch64 instruction decoder available to tools > tools: bug: Remove duplicate definition > objtool: arm64: Add base definition for arm64 backend > objtool: arm64: Decode add/sub instructions > objtool: arm64: Decode jump and call related instructions > objtool: arm64: Decode other system instructions > objtool: arm64: Decode load/store instructions > objtool: arm64: Decode LDR instructions > objtool: arm64: Accept padding in code sections > efi: libstub: Ignore relocations for .discard sections > objtool: arm64: Implement functions to add switch tables alternatives > objtool: arm64: Cache section with switch table information > objtool: arm64: Handle supported relocations in alternatives > objtool: arm64: Ignore replacement section for alternative callback > > Raphael Gault (2): > gcc-plugins: objtool: Add plugin to detect switch table on arm64 > objtool: arm64: Enable stack validation for arm64 > > arch/arm64/Kconfig | 2 + > drivers/firmware/efi/libstub/Makefile | 2 +- > scripts/Makefile.gcc-plugins | 2 + > scripts/gcc-plugins/Kconfig | 4 + > .../arm64_switch_table_detection_plugin.c | 85 + > tools/arch/arm64/include/asm/aarch64-insn.h | 551 +++++++ > tools/arch/arm64/lib/aarch64-insn.c | 1425 +++++++++++++++++ > tools/include/asm-generic/bitops/__ffs.h | 11 + > tools/include/linux/bug.h | 6 +- > tools/include/linux/kernel.h | 21 + > tools/include/linux/printk.h | 40 + > tools/objtool/Makefile | 5 + > tools/objtool/arch/arm64/Build | 8 + > tools/objtool/arch/arm64/decode.c | 471 ++++++ > .../arch/arm64/include/arch/cfi_regs.h | 14 + > tools/objtool/arch/arm64/include/arch/elf.h | 6 + > .../arch/arm64/include/arch/endianness.h | 9 + > .../objtool/arch/arm64/include/arch/special.h | 23 + > tools/objtool/arch/arm64/special.c | 134 ++ > tools/objtool/arch/x86/decode.c | 5 + > tools/objtool/check.c | 6 + > tools/objtool/include/objtool/arch.h | 3 + > tools/objtool/sync-check.sh | 5 + > 23 files changed, 2832 insertions(+), 6 deletions(-) > create mode 100644 scripts/gcc-plugins/arm64_switch_table_detection_plugin.c > create mode 100644 tools/arch/arm64/include/asm/aarch64-insn.h > create mode 100644 tools/arch/arm64/lib/aarch64-insn.c > create mode 100644 tools/include/linux/printk.h > create mode 100644 tools/objtool/arch/arm64/Build > create mode 100644 tools/objtool/arch/arm64/decode.c > create mode 100644 tools/objtool/arch/arm64/include/arch/cfi_regs.h > create mode 100644 tools/objtool/arch/arm64/include/arch/elf.h > create mode 100644 tools/objtool/arch/arm64/include/arch/endianness.h > create mode 100644 tools/objtool/arch/arm64/include/arch/special.h > create mode 100644 tools/objtool/arch/arm64/special.c > > -- > 2.25.4 >
WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ardb@kernel.org> To: Julien Thierry <jthierry@redhat.com> Cc: Mark Rutland <mark.rutland@arm.com>, linux-efi <linux-efi@vger.kernel.org>, Michal Marek <michal.lkml@markovi.net>, Kees Cook <keescook@chromium.org>, Peter Zijlstra <peterz@infradead.org>, Catalin Marinas <catalin.marinas@arm.com>, Masahiro Yamada <masahiroy@kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Mark Brown <broonie@kernel.org>, linux-hardening@vger.kernel.org, Josh Poimboeuf <jpoimboe@redhat.com>, Will Deacon <will@kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [RFC PATCH 00/17] objtool: add base support for arm64 Date: Thu, 21 Jan 2021 10:03:17 +0100 [thread overview] Message-ID: <CAMj1kXHO0wgcZ4ZDxj1vS9s7Szfpz8Nz=SAW_=Dnnjy+S9AtyQ@mail.gmail.com> (raw) In-Reply-To: <20210120173800.1660730-1-jthierry@redhat.com> Hello Julien, On Wed, 20 Jan 2021 at 18:38, Julien Thierry <jthierry@redhat.com> wrote: > > Hi, > > This series enables objtool to start doing stack validation on arm64 > kernel builds. Could we elaborate on this point, please? 'Stack validation' means getting an accurate picture of all kernel code that will be executed at some point in the future, due to the fact that there are stack frames pointing to them. And this ability is essential in order to do live patching safely? If this is the goal, I wonder whether this is the right approach for arm64 (or for any other architecture, for that matter) Parsing/decoding the object code and even worse, relying on GCC plugins to annotate some of the idioms as they are being generated, in order to infer intent on the part of the compiler goes *way* beyond what we should be comfortable with. The whole point of this exercise is to guarantee that there are no false positives when it comes to deciding whether the kernel is in a live patchable state, and I don't see how we can ever provide such a guarantee when it is built on such a fragile foundation. If we want to ensure that the stack contents are always an accurate reflection of the real call stack, we should work with the toolchain folks to identify issues that may interfere with this, and implement controls over these behaviors that we can decide to use in the build. In the past, I have already proposed adding a 'kernel' code model to the AArch64 compiler that guarantees certain things, such as adrp/add for symbol references, and no GOT indirections for position independent code. Inhibiting optimizations that may impact our ability to infer the real call stack from the stack contents is something we might add here as well. Another thing that occurred to me is that inferring which kernel code is actually live in terms of pending function returns could be inferred much more easily from a shadow call stack, which is a thing we already implement for Clang builds. In summary, I would not be in favor of enabling objtool on arm64 at all until we have exhausted other options for providing the functionality that we need it for (given that objtool provides many other things that only x86 cares about, IIUC) -- Ard. > It relies on the previous series I sent, refactoring > the arm64 decoder [1]. > > First, the aarch64 instruction decoder needed to be made available > to code under tools/. This is done in a similar manner to x86 > instruction decoded. One limitation I encountered there is that most > of aarch64 instruction decoder is __kprobe annotated. To bypass that > it remove the kprobe include and had to add an empty __kprobe > definition, but I'd welcome a proper solution to that. > > Then instruction semantics are progressively added so objtool can track > the stack state through the execution flow. > There are a few things that needed consideration: > - Generation of constants within executable sections, these either > caused objtool to fail decoding or to wrongly decode constants > as jumps or other instructions affecting execution flow and > causing confusion. To solve this, tracking locations referenced > by instructions using literals was needed. > - Jump tables from switch statements in aarch64 don't have enough > information to link branches with the branch instruction leading to > them. For this, we use a gcc plugin to add some information to establish > those missing links in a format that is already supported by objtool > > With this, there are still some errors when building with objtool. A > number of cleanups/annotations are needed on the arm64, as well as > handling SYM_DATA objects in objtool. > > Those changes can be found on top of this branch here: > git clone https://github.com/julien-thierry/linux.git -b objtoolxarm64-latest > > But it would be nice to have some feedback on this before I start submitting everyting. > > [1] https://lkml.org/lkml/2021/1/20/791 > > Thanks, > > Julien > > --> > > Julien Thierry (15): > tools: Add some generic functions and headers > tools: arm64: Make aarch64 instruction decoder available to tools > tools: bug: Remove duplicate definition > objtool: arm64: Add base definition for arm64 backend > objtool: arm64: Decode add/sub instructions > objtool: arm64: Decode jump and call related instructions > objtool: arm64: Decode other system instructions > objtool: arm64: Decode load/store instructions > objtool: arm64: Decode LDR instructions > objtool: arm64: Accept padding in code sections > efi: libstub: Ignore relocations for .discard sections > objtool: arm64: Implement functions to add switch tables alternatives > objtool: arm64: Cache section with switch table information > objtool: arm64: Handle supported relocations in alternatives > objtool: arm64: Ignore replacement section for alternative callback > > Raphael Gault (2): > gcc-plugins: objtool: Add plugin to detect switch table on arm64 > objtool: arm64: Enable stack validation for arm64 > > arch/arm64/Kconfig | 2 + > drivers/firmware/efi/libstub/Makefile | 2 +- > scripts/Makefile.gcc-plugins | 2 + > scripts/gcc-plugins/Kconfig | 4 + > .../arm64_switch_table_detection_plugin.c | 85 + > tools/arch/arm64/include/asm/aarch64-insn.h | 551 +++++++ > tools/arch/arm64/lib/aarch64-insn.c | 1425 +++++++++++++++++ > tools/include/asm-generic/bitops/__ffs.h | 11 + > tools/include/linux/bug.h | 6 +- > tools/include/linux/kernel.h | 21 + > tools/include/linux/printk.h | 40 + > tools/objtool/Makefile | 5 + > tools/objtool/arch/arm64/Build | 8 + > tools/objtool/arch/arm64/decode.c | 471 ++++++ > .../arch/arm64/include/arch/cfi_regs.h | 14 + > tools/objtool/arch/arm64/include/arch/elf.h | 6 + > .../arch/arm64/include/arch/endianness.h | 9 + > .../objtool/arch/arm64/include/arch/special.h | 23 + > tools/objtool/arch/arm64/special.c | 134 ++ > tools/objtool/arch/x86/decode.c | 5 + > tools/objtool/check.c | 6 + > tools/objtool/include/objtool/arch.h | 3 + > tools/objtool/sync-check.sh | 5 + > 23 files changed, 2832 insertions(+), 6 deletions(-) > create mode 100644 scripts/gcc-plugins/arm64_switch_table_detection_plugin.c > create mode 100644 tools/arch/arm64/include/asm/aarch64-insn.h > create mode 100644 tools/arch/arm64/lib/aarch64-insn.c > create mode 100644 tools/include/linux/printk.h > create mode 100644 tools/objtool/arch/arm64/Build > create mode 100644 tools/objtool/arch/arm64/decode.c > create mode 100644 tools/objtool/arch/arm64/include/arch/cfi_regs.h > create mode 100644 tools/objtool/arch/arm64/include/arch/elf.h > create mode 100644 tools/objtool/arch/arm64/include/arch/endianness.h > create mode 100644 tools/objtool/arch/arm64/include/arch/special.h > create mode 100644 tools/objtool/arch/arm64/special.c > > -- > 2.25.4 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-21 9:11 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 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 ` Ard Biesheuvel [this message] 2021-01-21 9:03 ` [RFC PATCH 00/17] objtool: add base support " 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='CAMj1kXHO0wgcZ4ZDxj1vS9s7Szfpz8Nz=SAW_=Dnnjy+S9AtyQ@mail.gmail.com' \ --to=ardb@kernel.org \ --cc=broonie@kernel.org \ --cc=catalin.marinas@arm.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=peterz@infradead.org \ --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: linkBe 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.