| Message ID | 20190409135243.12424-1-raphael.gault@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | objtool: Add support for Arm64 | expand |
On Tue, Apr 09, 2019 at 02:52:37PM +0100, Raphael Gault wrote: > Hi, > > As of now, objtool only supports the x86_64 architecture but the > groundwork has already been done in order to add support for other > architecture without too much effort. Hi Raphael, This was a pleasant surprise in my inbox. From a quick glance I'm actually surprised at how "easy" it looks, though I can tell it was quite a big effort. Doing the first arch port is always the hardest. Kudos! I will give it a proper review soon.
On Tue, 9 Apr 2019 at 06:53, Raphael Gault <raphael.gault@arm.com> wrote: > > Hi, > > As of now, objtool only supports the x86_64 architecture but the > groundwork has already been done in order to add support for other > architecture without too much effort. > > This series of patches adds support for the arm64 architecture > based on the Armv8.5 Architecture Reference Manual. > I think it makes sense to clarify *why* we want this on arm64. Also, we should identify things that objtool does today that maybe we don't want on arm64, rather than buy into all of it by default. > * Patch 1 adapts the existing code to be able to add support for other > architecture. > * Patch 2 provide implementation of the required function for the arm64 > architecture. > * Patch 3 adapts the checking of the stack state for the arm64 > architecture. > * Patch 4 & 5 fix some warning objtool raised in some particular > functions of ~/arch/arm64/kernel/sleep.S. Patch 4 add a macro to > signal that some function should be ignored by objtool. > * Patch 6 enables stack validation for arm64. > > Theses patches should provide support for the main cases and behaviour. > However a few corner cases are not yet handled by objtool: > > * In the `~/arch/arm64/crypto/` directory, I noticed that some plain > data are sometimes stored in the `.text` section causing objtool to mistake > this for instructions and trying (and failing) to interprete them. If someone > could explain to me why we store data directly in the .text section I would > appreciate it. > Just for convenience, basically. Not specifying a section at all when emitting a literal amounts to putting it into .text, and a a bonus, it is guaranteed to be in range for a ADR instructions (which is not the case when the code is built into the kernel rather than enabled as a module) Due to Spectre I have already updated some of the code so larger tables are moved into .rodata (since literal data might contain binary sequences that are usable as spectre gadgets), but some work remains to be done. > * In the support for arm32 architecture such as in `~/arch/arm64/kernel/kuser32.S` > some A32 instructions are used but such instructions are not understood by > objtool causing a warning. > > I also have a few unclear points I would like to bring to your > attention: > > * For x86_64, when looking for a symbol relocation with explicit > addend, objtool systematically adds a +4 offset to the addend. > I don't understand why even if I have a feeling it is related > to the type of relacation. > > * I currently don't have a clear understanding about how switch-tables > are generated on arm64 and how to retrieve them (based on relocations). > > Please provide me with any feedback and comments as well on the content > than the style of these patches. > > Thanks, > > Raphael > > -> > > Raphael Gault (6): > objtool: Refactor code to make it more suitable for multiple > architecture support > objtool: arm64: Add required implementation for supporting the aarch64 > architecture in objtool. > objtool: arm64: Adapt the stack frame checks and the section analysis > for the arm architecture > arm64: assembler: Add macro to annotate asm function having non > standard stack-frame. > arm64: sleep: Add stack frame setup for __cpu_supsend_enter > objtool: arm64: Enable stack validation for arm64 > > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/assembler.h | 18 + > arch/arm64/kernel/sleep.S | 4 + > tools/objtool/Build | 1 - > tools/objtool/arch.h | 11 + > tools/objtool/arch/arm64/Build | 6 + > tools/objtool/arch/arm64/bit_operations.c | 65 + > tools/objtool/arch/arm64/decode.c | 2870 +++++++++++++++++ > .../objtool/arch/arm64/include/arch_special.h | 44 + > .../arch/arm64/include/asm/orc_types.h | 109 + > .../arch/arm64/include/bit_operations.h | 22 + > tools/objtool/arch/arm64/include/cfi.h | 76 + > .../objtool/arch/arm64/include/insn_decode.h | 219 ++ > tools/objtool/arch/arm64/orc_gen.c | 40 + > tools/objtool/arch/x86/Build | 1 + > tools/objtool/arch/x86/decode.c | 111 + > tools/objtool/arch/x86/include/arch_special.h | 35 + > tools/objtool/{ => arch/x86/include}/cfi.h | 0 > tools/objtool/{ => arch/x86}/orc_gen.c | 10 +- > tools/objtool/check.c | 209 +- > tools/objtool/check.h | 1 + > tools/objtool/elf.c | 3 +- > tools/objtool/orc.h | 4 +- > tools/objtool/special.c | 18 +- > 24 files changed, 3740 insertions(+), 138 deletions(-) > create mode 100644 tools/objtool/arch/arm64/Build > create mode 100644 tools/objtool/arch/arm64/bit_operations.c > create mode 100644 tools/objtool/arch/arm64/decode.c > create mode 100644 tools/objtool/arch/arm64/include/arch_special.h > create mode 100644 tools/objtool/arch/arm64/include/asm/orc_types.h > create mode 100644 tools/objtool/arch/arm64/include/bit_operations.h > create mode 100644 tools/objtool/arch/arm64/include/cfi.h > create mode 100644 tools/objtool/arch/arm64/include/insn_decode.h > create mode 100644 tools/objtool/arch/arm64/orc_gen.c > create mode 100644 tools/objtool/arch/x86/include/arch_special.h > rename tools/objtool/{ => arch/x86/include}/cfi.h (100%) > rename tools/objtool/{ => arch/x86}/orc_gen.c (96%) > > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Apr 09, 2019 at 10:43:18AM -0700, Ard Biesheuvel wrote: > On Tue, 9 Apr 2019 at 06:53, Raphael Gault <raphael.gault@arm.com> wrote: > > > > Hi, > > > > As of now, objtool only supports the x86_64 architecture but the > > groundwork has already been done in order to add support for other > > architecture without too much effort. > > > > This series of patches adds support for the arm64 architecture > > based on the Armv8.5 Architecture Reference Manual. > > > > I think it makes sense to clarify *why* we want this on arm64. Also, > we should identify things that objtool does today that maybe we don't > want on arm64, rather than buy into all of it by default. Agreed, the "why" should at least be in the cover letter. From my perspective, the "why" includes: - Live patching - objtool stack validation is the foundation for a reliable unwinder - ORC unwinder - benefits include presumed improved overall performance from disabling frame pointers, and the ability to unwind across interrupts and exceptions - PeterZ's new uaccess validation?
On 10/04/2019 04:37, Josh Poimboeuf wrote: > On Tue, Apr 09, 2019 at 10:43:18AM -0700, Ard Biesheuvel wrote: >> On Tue, 9 Apr 2019 at 06:53, Raphael Gault <raphael.gault@arm.com> wrote: >>> >>> Hi, >>> >>> As of now, objtool only supports the x86_64 architecture but the >>> groundwork has already been done in order to add support for other >>> architecture without too much effort. >>> >>> This series of patches adds support for the arm64 architecture >>> based on the Armv8.5 Architecture Reference Manual. >>> >> >> I think it makes sense to clarify *why* we want this on arm64. Also, >> we should identify things that objtool does today that maybe we don't >> want on arm64, rather than buy into all of it by default. > > Agreed, the "why" should at least be in the cover letter. From my > perspective, the "why" includes: > > - Live patching - objtool stack validation is the foundation for a > reliable unwinder > Yes, as I understand Live patching is a work in progress for arm64. Having objtool to provide more guarantees would be nice. > - ORC unwinder - benefits include presumed improved overall performance > from disabling frame pointers, and the ability to unwind across > interrupts and exceptions > I'm unsure this will be part of the plan. I believe so far arm64 code heavily relies on the presence of frame pointers. It's also part of the Aarch64 Procedure Call Standard. But who knows. > - PeterZ's new uaccess validation? > Yes, we've reverted twice our implementation of user_access_begin/end() on arm64 because of the issue of potentially calling sleeping functions. Once we have the base of objtool work, this would be one of the next work items. Thanks,
On Tue, Apr 09, 2019 at 02:52:37PM +0100, Raphael Gault wrote: > Hi, > > As of now, objtool only supports the x86_64 architecture but the > groundwork has already been done in order to add support for other > architecture without too much effort. > > This series of patches adds support for the arm64 architecture > based on the Armv8.5 Architecture Reference Manual. > > * Patch 1 adapts the existing code to be able to add support for other > architecture. > * Patch 2 provide implementation of the required function for the arm64 > architecture. > * Patch 3 adapts the checking of the stack state for the arm64 > architecture. > * Patch 4 & 5 fix some warning objtool raised in some particular > functions of ~/arch/arm64/kernel/sleep.S. Patch 4 add a macro to > signal that some function should be ignored by objtool. > * Patch 6 enables stack validation for arm64. > > Theses patches should provide support for the main cases and behaviour. > However a few corner cases are not yet handled by objtool: > > * In the `~/arch/arm64/crypto/` directory, I noticed that some plain > data are sometimes stored in the `.text` section causing objtool to mistake > this for instructions and trying (and failing) to interprete them. If someone > could explain to me why we store data directly in the .text section I would > appreciate it. I haven't looked, but it should probably be moved to .rodata. We had cases like that for x86. > * In the support for arm32 architecture such as in `~/arch/arm64/kernel/kuser32.S` > some A32 instructions are used but such instructions are not understood by > objtool causing a warning. > > I also have a few unclear points I would like to bring to your > attention: > > * For x86_64, when looking for a symbol relocation with explicit > addend, objtool systematically adds a +4 offset to the addend. > I don't understand why even if I have a feeling it is related > to the type of relacation. This is because of how relative call/jump addresses are implemented on x86. It calculates the call/jump destination by adding the encoded offset to the *ending* address of the instruction, rather than to the address of the encoded offset itself. For example: 119ca0: e8 00 00 00 00 callq 119ca5 <__ia32_sys_sched_rr_get_interval+0x5> 119ca1: R_X86_64_PC32 __fentry__-0x4 This instruction is a call to the __fentry__ function. The rela addend is the address of the destination function (__fentry__) minus 4. After applying the relocation, it resolves to: ffffffff81002010: e8 eb f7 9f 00 callq ffffffff81a01800 <__fentry__> The destination address is "0x9ff7eb", which is indeed __fentry__ - 4. x86 expects it to be that way, because the x86 CPU adds the offset to the *end* of the instruction: ffffffff81002015 (last digit is 5, not 0). 0xffffffff81002015 + 0x9ff7eb = 0xfffffff81a01800, which is indeed the address of __fentry__. And there's always a 4-byte gap between the relocation target and the end of the instruction, so the rela addend always has the -4. So when reading the relocation we have to add the 4 bytes back to get the actual destination address. > * I currently don't have a clear understanding about how switch-tables > are generated on arm64 and how to retrieve them (based on relocations). This is indeed a bit tricky on x86. > Please provide me with any feedback and comments as well on the content > than the style of these patches. Overall it looks like a great start. I added some per-patch comments. Has the cross-compile path been tested? Specifically, compiling for a arm64 target on an x86 host? In other words, objtool would be an x86 binary which reads arm64 objects. I imagine that will be a semi-common use case. Objtool already supports cross-compiling for an x86-64 target on an x86-32 host (and also a powerpc host IIRC), so it should be do-able in theory, and it might "just work". For the next version, please base it on the -tip tree, as that's where all the latest objtool changes are. Peter refactored some code which has some minor conflicts with yours.
Hi Josh, On 4/23/19 10:09 PM, Josh Poimboeuf wrote: > On Tue, Apr 09, 2019 at 02:52:37PM +0100, Raphael Gault wrote: >> Hi, >> >> As of now, objtool only supports the x86_64 architecture but the >> groundwork has already been done in order to add support for other >> architecture without too much effort. >> >> This series of patches adds support for the arm64 architecture >> based on the Armv8.5 Architecture Reference Manual. >> >> * Patch 1 adapts the existing code to be able to add support for other >> architecture. >> * Patch 2 provide implementation of the required function for the arm64 >> architecture. >> * Patch 3 adapts the checking of the stack state for the arm64 >> architecture. >> * Patch 4 & 5 fix some warning objtool raised in some particular >> functions of ~/arch/arm64/kernel/sleep.S. Patch 4 add a macro to >> signal that some function should be ignored by objtool. >> * Patch 6 enables stack validation for arm64. >> >> Theses patches should provide support for the main cases and behaviour. >> However a few corner cases are not yet handled by objtool: >> >> * In the `~/arch/arm64/crypto/` directory, I noticed that some plain >> data are sometimes stored in the `.text` section causing objtool to mistake >> this for instructions and trying (and failing) to interprete them. If someone >> could explain to me why we store data directly in the .text section I would >> appreciate it. > > I haven't looked, but it should probably be moved to .rodata. We had > cases like that for x86. > >> * In the support for arm32 architecture such as in `~/arch/arm64/kernel/kuser32.S` >> some A32 instructions are used but such instructions are not understood by >> objtool causing a warning. >> >> I also have a few unclear points I would like to bring to your >> attention: >> >> * For x86_64, when looking for a symbol relocation with explicit >> addend, objtool systematically adds a +4 offset to the addend. >> I don't understand why even if I have a feeling it is related >> to the type of relacation. > > This is because of how relative call/jump addresses are implemented on > x86. It calculates the call/jump destination by adding the encoded offset > to the *ending* address of the instruction, rather than to the address > of the encoded offset itself. > > For example: > > 119ca0: e8 00 00 00 00 callq 119ca5 <__ia32_sys_sched_rr_get_interval+0x5> > 119ca1: R_X86_64_PC32 __fentry__-0x4 > > This instruction is a call to the __fentry__ function. The rela addend > is the address of the destination function (__fentry__) minus 4. After > applying the relocation, it resolves to: > > ffffffff81002010: e8 eb f7 9f 00 callq ffffffff81a01800 <__fentry__> > > The destination address is "0x9ff7eb", which is indeed __fentry__ - 4. > > x86 expects it to be that way, because the x86 CPU adds the offset to > the *end* of the instruction: ffffffff81002015 (last digit is 5, not 0). > > 0xffffffff81002015 + 0x9ff7eb = 0xfffffff81a01800, which is indeed the > address of __fentry__. > > And there's always a 4-byte gap between the relocation target and the > end of the instruction, so the rela addend always has the -4. So when > reading the relocation we have to add the 4 bytes back to get the actual > destination address. > Thank you for the explanation! >> * I currently don't have a clear understanding about how switch-tables >> are generated on arm64 and how to retrieve them (based on relocations). > > This is indeed a bit tricky on x86. > >> Please provide me with any feedback and comments as well on the content >> than the style of these patches. > > Overall it looks like a great start. I added some per-patch comments. > Thank you, I will address them. > Has the cross-compile path been tested? Specifically, compiling for a > arm64 target on an x86 host? In other words, objtool would be an x86 > binary which reads arm64 objects. I imagine that will be a semi-common > use case. Objtool already supports cross-compiling for an x86-64 target > on an x86-32 host (and also a powerpc host IIRC), so it should be > do-able in theory, and it might "just work". > It has indeed been tested to build for arm64 target on a x86 host and it works fine. > For the next version, please base it on the -tip tree, as that's where > all the latest objtool changes are. Peter refactored some code which > has some minor conflicts with yours. > Thanks for letting me know about this. Just a quick precision, when you talk about the -tip tree, are you talking about this tree ? https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/ Thanks, -- Raphael Gault IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Wed, Apr 24, 2019 at 04:08:15PM +0000, Raphael Gault wrote: > > For the next version, please base it on the -tip tree, as that's where > > all the latest objtool changes are. Peter refactored some code which > > has some minor conflicts with yours. > > > > Thanks for letting me know about this. Just a quick precision, when you > talk about the -tip tree, are you talking about this tree ? > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/ Indeed. The git link I use is: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git And generally the master branch of that tree seems to be a good one to use.
Hi, As of now, objtool only supports the x86_64 architecture but the groundwork has already been done in order to add support for other architecture without too much effort. This series of patches adds support for the arm64 architecture based on the Armv8.5 Architecture Reference Manual. * Patch 1 adapts the existing code to be able to add support for other architecture. * Patch 2 provide implementation of the required function for the arm64 architecture. * Patch 3 adapts the checking of the stack state for the arm64 architecture. * Patch 4 & 5 fix some warning objtool raised in some particular functions of ~/arch/arm64/kernel/sleep.S. Patch 4 add a macro to signal that some function should be ignored by objtool. * Patch 6 enables stack validation for arm64. Theses patches should provide support for the main cases and behaviour. However a few corner cases are not yet handled by objtool: * In the `~/arch/arm64/crypto/` directory, I noticed that some plain data are sometimes stored in the `.text` section causing objtool to mistake this for instructions and trying (and failing) to interprete them. If someone could explain to me why we store data directly in the .text section I would appreciate it. * In the support for arm32 architecture such as in `~/arch/arm64/kernel/kuser32.S` some A32 instructions are used but such instructions are not understood by objtool causing a warning. I also have a few unclear points I would like to bring to your attention: * For x86_64, when looking for a symbol relocation with explicit addend, objtool systematically adds a +4 offset to the addend. I don't understand why even if I have a feeling it is related to the type of relacation. * I currently don't have a clear understanding about how switch-tables are generated on arm64 and how to retrieve them (based on relocations). Please provide me with any feedback and comments as well on the content than the style of these patches. Thanks, Raphael -> Raphael Gault (6): objtool: Refactor code to make it more suitable for multiple architecture support objtool: arm64: Add required implementation for supporting the aarch64 architecture in objtool. objtool: arm64: Adapt the stack frame checks and the section analysis for the arm architecture arm64: assembler: Add macro to annotate asm function having non standard stack-frame. arm64: sleep: Add stack frame setup for __cpu_supsend_enter objtool: arm64: Enable stack validation for arm64 arch/arm64/Kconfig | 1 + arch/arm64/include/asm/assembler.h | 18 + arch/arm64/kernel/sleep.S | 4 + tools/objtool/Build | 1 - tools/objtool/arch.h | 11 + tools/objtool/arch/arm64/Build | 6 + tools/objtool/arch/arm64/bit_operations.c | 65 + tools/objtool/arch/arm64/decode.c | 2870 +++++++++++++++++ .../objtool/arch/arm64/include/arch_special.h | 44 + .../arch/arm64/include/asm/orc_types.h | 109 + .../arch/arm64/include/bit_operations.h | 22 + tools/objtool/arch/arm64/include/cfi.h | 76 + .../objtool/arch/arm64/include/insn_decode.h | 219 ++ tools/objtool/arch/arm64/orc_gen.c | 40 + tools/objtool/arch/x86/Build | 1 + tools/objtool/arch/x86/decode.c | 111 + tools/objtool/arch/x86/include/arch_special.h | 35 + tools/objtool/{ => arch/x86/include}/cfi.h | 0 tools/objtool/{ => arch/x86}/orc_gen.c | 10 +- tools/objtool/check.c | 209 +- tools/objtool/check.h | 1 + tools/objtool/elf.c | 3 +- tools/objtool/orc.h | 4 +- tools/objtool/special.c | 18 +- 24 files changed, 3740 insertions(+), 138 deletions(-) create mode 100644 tools/objtool/arch/arm64/Build create mode 100644 tools/objtool/arch/arm64/bit_operations.c create mode 100644 tools/objtool/arch/arm64/decode.c create mode 100644 tools/objtool/arch/arm64/include/arch_special.h create mode 100644 tools/objtool/arch/arm64/include/asm/orc_types.h create mode 100644 tools/objtool/arch/arm64/include/bit_operations.h create mode 100644 tools/objtool/arch/arm64/include/cfi.h create mode 100644 tools/objtool/arch/arm64/include/insn_decode.h create mode 100644 tools/objtool/arch/arm64/orc_gen.c create mode 100644 tools/objtool/arch/x86/include/arch_special.h rename tools/objtool/{ => arch/x86/include}/cfi.h (100%) rename tools/objtool/{ => arch/x86}/orc_gen.c (96%)