From: Josh Poimboeuf <jpoimboe@redhat.com> To: Ard Biesheuvel <ardb@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org>, Julien Thierry <jthierry@redhat.com>, 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>, Mark Rutland <mark.rutland@arm.com>, Mark Brown <broonie@kernel.org>, linux-efi <linux-efi@vger.kernel.org>, linux-hardening@vger.kernel.org, live-patching@vger.kernel.org Subject: Re: [RFC PATCH 00/17] objtool: add base support for arm64 Date: Thu, 21 Jan 2021 12:54:52 -0600 [thread overview] Message-ID: <20210121185452.fxoz4ehqfv75bdzq@treble> (raw) In-Reply-To: <CAMj1kXE+675mbS66kteKHNfcrco84WTaEL6ncVkkV7tQgbMpFw@mail.gmail.com> Ard, Sorry, I was late to the party, attempting to reply to the entire thread at once. Also, adding the live-patching ML. I agree with a lot of your concerns. Reverse engineering the control flow of the compiled binary is kind of ridiculous. I was always surprised that it works. I still am! But I think it's more robust than you give it credit for. Most of the existing code just works, with (annual) tweaks for new compiler versions. In fact now it works well with both GCC and Clang, across several versions. Soon it will work with LTO. It has grown many uses beyond stack validation: ORC, static calls, retpolines validation, noinstr validation, SMAP validation. It has found a *lot* of compiler bugs. And there will probably be more use cases when we get vmlinux validation running fast enough. But there is indeed a maintenance burden. I often ask myself if it's worth it. So far the answer has been yes :-) Particularly because it has influenced many positive changes to the kernel. And it helps now that even more people are contributing and adding useful features. But you should definitely think twice before letting it loose on your arch, especially if you have some other way to ensure reliable stack metadata, and if you don't have a need for the other objtool features. Regarding your other proposals: 1) I'm doubtful we can ever rely on the toolchain to ensure reliable unwind metadata, because: a) most of the problems are in asm and inline-asm; good luck getting the toolchain to care about those. b) the toolchain is fragile; do we want to entrust the integrity of live patching to the compiler's frame pointer generation (or other unwind metadata) without having some sort of checking mechanism? 2) The shadow stack idea sounds promising -- how hard would it be to make a prototype reliable unwinder? More comments below: On Thu, Jan 21, 2021 at 12:48:43PM +0100, Ard Biesheuvel wrote: > On Thu, 21 Jan 2021 at 12:23, Peter Zijlstra <peterz@infradead.org> wrote: > > > > On Thu, Jan 21, 2021 at 12:08:23PM +0100, Ard Biesheuvel wrote: > > > On Thu, 21 Jan 2021 at 11:26, Julien Thierry <jthierry@redhat.com> wrote: > > > > > > I'm not familiar with toolcahin code models, but would this approach be > > > > able to validate assembly code (either inline or in assembly files?) > > > > > > > > > > No, it would not. But those files are part of the code base, and can > > > be reviewed and audited. > > > > x86 has a long history if failing at exactly that. > > That's a fair point. But on the flip side, maintaining objtool does > not look like it has been a walk in the park either. I think you missed Peter's point: it's not that it's *hard* for humans to continuously review/audit all asm and inline-asm; it's just not feasible to do it 100% correctly, 100% of the time. Like any other code, objtool requires maintenance, but its analysis is orders of magnitude more robust than any human. > What i am especially concerned about is things like 3193c0836f20, > where we actually have to disable certain compiler optimizations > because they interfere with objtool's ability to understand the > resulting object code. Correctness and performance are challenging > enough as requirements for generated code. Well, you managed to find the worst case scenario. I think that's the only time we ever had to do that. Please don't think that's normal, or even a generally realistic concern. Objtool tries really hard to stay out of the way. Long term we really want to prevent that type of thing with the help of annotations from compiler plugins, similar to what Julien did here. Yes, it would mean two objtool compiler plugins (GCC and Clang), but it would ease the objtool maintenance burden and risk in many ways. And prevent situations like that commit you found. It may sound fragile, but it will actually make things simpler overall: less reverse engineering of GCC switch jump tables and __noreturn functions is a good thing. > Mind you, I am not saying it is not worth it *for x86*, where there is > a lot of other stuff going on. But on arm64, we don't care about ORC, > about -fomit-frame-pointer, about retpolines or about any of the other > things objtool enables. > > On arm64, all it currently seems to provide is a way to capture the > call stack accurately, and given that it needs a GCC plugin for this > (which needs to be maintained as well, which is non-trivial, and also > bars us from using objtool with Clang builds), my current position is > simply that opening this can of worms at this point is just not worth > it. As far as GCC plugins go, it looks pretty basic to me. Also this doesn't *at all* prevent Clang from being used for live patching. If anybody actually tries running Julien's patches on a Clang-built kernel, it might just work. But if not, and the switch tables turn out to be unparseable like on GCC, we could have a Clang plugin. As I mentioned, we'll probably end up having one anyway for x86. -- Josh
WARNING: multiple messages have this Message-ID (diff)
From: Josh Poimboeuf <jpoimboe@redhat.com> To: Ard Biesheuvel <ardb@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com>, linux-efi <linux-efi@vger.kernel.org>, Michal Marek <michal.lkml@markovi.net>, Julien Thierry <jthierry@redhat.com>, 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, live-patching@vger.kernel.org, Will Deacon <will@kernel.org>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Kees Cook <keescook@chromium.org> Subject: Re: [RFC PATCH 00/17] objtool: add base support for arm64 Date: Thu, 21 Jan 2021 12:54:52 -0600 [thread overview] Message-ID: <20210121185452.fxoz4ehqfv75bdzq@treble> (raw) In-Reply-To: <CAMj1kXE+675mbS66kteKHNfcrco84WTaEL6ncVkkV7tQgbMpFw@mail.gmail.com> Ard, Sorry, I was late to the party, attempting to reply to the entire thread at once. Also, adding the live-patching ML. I agree with a lot of your concerns. Reverse engineering the control flow of the compiled binary is kind of ridiculous. I was always surprised that it works. I still am! But I think it's more robust than you give it credit for. Most of the existing code just works, with (annual) tweaks for new compiler versions. In fact now it works well with both GCC and Clang, across several versions. Soon it will work with LTO. It has grown many uses beyond stack validation: ORC, static calls, retpolines validation, noinstr validation, SMAP validation. It has found a *lot* of compiler bugs. And there will probably be more use cases when we get vmlinux validation running fast enough. But there is indeed a maintenance burden. I often ask myself if it's worth it. So far the answer has been yes :-) Particularly because it has influenced many positive changes to the kernel. And it helps now that even more people are contributing and adding useful features. But you should definitely think twice before letting it loose on your arch, especially if you have some other way to ensure reliable stack metadata, and if you don't have a need for the other objtool features. Regarding your other proposals: 1) I'm doubtful we can ever rely on the toolchain to ensure reliable unwind metadata, because: a) most of the problems are in asm and inline-asm; good luck getting the toolchain to care about those. b) the toolchain is fragile; do we want to entrust the integrity of live patching to the compiler's frame pointer generation (or other unwind metadata) without having some sort of checking mechanism? 2) The shadow stack idea sounds promising -- how hard would it be to make a prototype reliable unwinder? More comments below: On Thu, Jan 21, 2021 at 12:48:43PM +0100, Ard Biesheuvel wrote: > On Thu, 21 Jan 2021 at 12:23, Peter Zijlstra <peterz@infradead.org> wrote: > > > > On Thu, Jan 21, 2021 at 12:08:23PM +0100, Ard Biesheuvel wrote: > > > On Thu, 21 Jan 2021 at 11:26, Julien Thierry <jthierry@redhat.com> wrote: > > > > > > I'm not familiar with toolcahin code models, but would this approach be > > > > able to validate assembly code (either inline or in assembly files?) > > > > > > > > > > No, it would not. But those files are part of the code base, and can > > > be reviewed and audited. > > > > x86 has a long history if failing at exactly that. > > That's a fair point. But on the flip side, maintaining objtool does > not look like it has been a walk in the park either. I think you missed Peter's point: it's not that it's *hard* for humans to continuously review/audit all asm and inline-asm; it's just not feasible to do it 100% correctly, 100% of the time. Like any other code, objtool requires maintenance, but its analysis is orders of magnitude more robust than any human. > What i am especially concerned about is things like 3193c0836f20, > where we actually have to disable certain compiler optimizations > because they interfere with objtool's ability to understand the > resulting object code. Correctness and performance are challenging > enough as requirements for generated code. Well, you managed to find the worst case scenario. I think that's the only time we ever had to do that. Please don't think that's normal, or even a generally realistic concern. Objtool tries really hard to stay out of the way. Long term we really want to prevent that type of thing with the help of annotations from compiler plugins, similar to what Julien did here. Yes, it would mean two objtool compiler plugins (GCC and Clang), but it would ease the objtool maintenance burden and risk in many ways. And prevent situations like that commit you found. It may sound fragile, but it will actually make things simpler overall: less reverse engineering of GCC switch jump tables and __noreturn functions is a good thing. > Mind you, I am not saying it is not worth it *for x86*, where there is > a lot of other stuff going on. But on arm64, we don't care about ORC, > about -fomit-frame-pointer, about retpolines or about any of the other > things objtool enables. > > On arm64, all it currently seems to provide is a way to capture the > call stack accurately, and given that it needs a GCC plugin for this > (which needs to be maintained as well, which is non-trivial, and also > bars us from using objtool with Clang builds), my current position is > simply that opening this can of worms at this point is just not worth > it. As far as GCC plugins go, it looks pretty basic to me. Also this doesn't *at all* prevent Clang from being used for live patching. If anybody actually tries running Julien's patches on a Clang-built kernel, it might just work. But if not, and the switch tables turn out to be unparseable like on GCC, we could have a Clang plugin. As I mentioned, we'll probably end up having one anyway for x86. -- Josh _______________________________________________ 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 19:31 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 ` [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 [this message] 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=20210121185452.fxoz4ehqfv75bdzq@treble \ --to=jpoimboe@redhat.com \ --cc=ardb@kernel.org \ --cc=broonie@kernel.org \ --cc=catalin.marinas@arm.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=live-patching@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.