mbox series

[RFC,v2,00/13] objtool: add base support for arm64

Message ID 20210303170932.1838634-1-jthierry@redhat.com (mailing list archive)
Headers show
Series objtool: add base support for arm64 | expand

Message

Julien Thierry March 3, 2021, 5:09 p.m. UTC
Hi,

This series enables objtool to start doing stack validation on arm64
kernel builds. 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. Following suggestions, I've dropped the previously used GCC
  plugin and instead disabled the generation of jump tables by the
  compiler. I've not noticed performance deterioration nor concerning
  Image size increase after doing so. This approach has the benefit of
  working for both GCC and clang.

With those changes, 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

Changes since v1[2]:
- Drop gcc plugin in favor of -fno-jump-tables
- miscelaneous fixes and cleanups

[1] https://lkml.org/lkml/2021/1/20/791
[2] https://lkml.org/lkml/2021/1/20/923

Thanks,

Julien

-->

Julien Thierry (12):
  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
  objtool: arm64: Handle supported relocations in alternatives
  objtool: arm64: Ignore replacement section for alternative callback

Raphael Gault (1):
  objtool: arm64: Enable stack validation for arm64

 arch/arm64/Kconfig                            |    1 +
 arch/arm64/Makefile                           |    4 +
 tools/arch/arm64/include/asm/insn.h           |  565 +++++++
 tools/arch/arm64/lib/insn.c                   | 1456 +++++++++++++++++
 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             |  502 ++++++
 .../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 |   21 +
 tools/objtool/arch/arm64/special.c            |   37 +
 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 +
 20 files changed, 2720 insertions(+), 5 deletions(-)
 create mode 100644 tools/arch/arm64/include/asm/insn.h
 create mode 100644 tools/arch/arm64/lib/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

Comments

Peter Zijlstra March 3, 2021, 7:17 p.m. UTC | #1
On Wed, Mar 03, 2021 at 06:09:19PM +0100, Julien Thierry wrote:
> Hi,
> 
> This series enables objtool to start doing stack validation on arm64
> kernel builds. 

> -->
> 
> Julien Thierry (12):
>   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
>   objtool: arm64: Handle supported relocations in alternatives
>   objtool: arm64: Ignore replacement section for alternative callback
> 
> Raphael Gault (1):
>   objtool: arm64: Enable stack validation for arm64


These patches look very reasonable to me,

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>


One selfish thing, would it make sense to have a make target that builds
all supported srcarch targets?

This might be useful when hacking on objtool to make sure everything
builds.
Julien Thierry March 4, 2021, 2:03 p.m. UTC | #2
On 3/3/21 8:17 PM, Peter Zijlstra wrote:
> One selfish thing, would it make sense to have a make target that builds
> all supported srcarch targets?
>
> This might be useful when hacking on objtool to make sure everything
> builds.
>

That makes sense. I can offer something like bellow which is simple
enough to use and update.

Otherwise you could have per arch targets, but since the generic part of
objtool uses arch specific headers, you'll have to rebuild object files
between two arch builds anyway.

Julien

-->

From 36cf9e05f2ee40bd5239c3b78cd1c5260941cb94 Mon Sep 17 00:00:00 2001
Date: Thu, 4 Mar 2021 14:46:39 +0100
Subject: [PATCH] objtool: Add target to test build of different architectures

To make sure support for other architecture doesn't break when updating
objtool, it's useful to have a shorthand to build the different objtool
configuration.

Add a target that can be called from the top level as such:

    $ make tools/objtool/build-test

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 tools/Makefile         | 3 +++
 tools/objtool/Makefile | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/tools/Makefile b/tools/Makefile
index 7e9d34ddd74c..79e4a5ff0576 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -68,6 +68,9 @@ cpupower: FORCE
 cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging tracing: FORCE
 	$(call descend,$@)

+objtool/%: FORCE
+	$(call descend,objtool,$@)
+
 bpf/%: FORCE
 	$(call descend,$@)

diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index d5cfbec87c02..4c57f8cdaeb6 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -56,6 +56,12 @@ export SUBCMD_CHECK SUBCMD_ORC
 export srctree OUTPUT CFLAGS SRCARCH AWK
 include $(srctree)/tools/build/Makefile.include

+objtool/build-test: FORCE
+	@SRCARCH=x86 $(MAKE)
+	@SRCARCH=x86 $(MAKE) clean
+	@SRCARCH=arm64 $(MAKE)
+	@SRCARCH=arm64 $(MAKE) clean
+
 $(OBJTOOL_IN): fixdep FORCE
 	@$(CONFIG_SHELL) ./sync-check.sh
 	@$(MAKE) $(build)=objtool
--
2.25.4
Nick Desaulniers March 5, 2021, 11:51 p.m. UTC | #3
(in response to
https://lore.kernel.org/linux-arm-kernel/20210303170932.1838634-1-jthierry@redhat.com/
from the command line)

> Changes since v1[2]:
> - Drop gcc plugin in favor of -fno-jump-tables

Thank you for this!  I built+booted(under emulation) arm64 defconfig and built
arm64 allmodconfig with LLVM=1 with this series applied.

Tested-by: Nick Desaulniers <ndesaulniers@google.com>

One thing I noticed was a spew of warnings for allmodconfig, like:
init/main.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
init/main.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup

I assume those are from the KASAN constructors. See also:
https://github.com/ClangBuiltLinux/linux/issues/1238

Can we disable HAVE_STACK_PROTECTOR if CC_IS_CLANG and CONFIG_KASAN is set,
until we can resolve the above issue?
Nick Desaulniers March 6, 2021, 12:04 a.m. UTC | #4
On Fri, Mar 5, 2021 at 3:51 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> (in response to
> https://lore.kernel.org/linux-arm-kernel/20210303170932.1838634-1-jthierry@redhat.com/
> from the command line)
>
> > Changes since v1[2]:
> > - Drop gcc plugin in favor of -fno-jump-tables
>
> Thank you for this!  I built+booted(under emulation) arm64 defconfig and built
> arm64 allmodconfig with LLVM=1 with this series applied.
>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
>
> One thing I noticed was a spew of warnings for allmodconfig, like:
> init/main.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> init/main.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
>
> I assume those are from the KASAN constructors. See also:
> https://github.com/ClangBuiltLinux/linux/issues/1238
>
> Can we disable HAVE_STACK_PROTECTOR if CC_IS_CLANG and CONFIG_KASAN is set,
> until we can resolve the above issue?

Ah, filtering the logs more, it looks like GCOV is has the same issue
KASAN does (known issue).  Here's a filtered log:
https://gist.github.com/nickdesaulniers/01358015b33bd16ccd7d951c4a8c44e7

I'm curious about the failure to decode certain instructions?

The stack state mismatches are what are valuable to me; we'll need
some help digging into those at some point.  The logs from defconfig
are clean.
Julien Thierry March 9, 2021, 2:27 p.m. UTC | #5
On 3/6/21 1:04 AM, Nick Desaulniers wrote:
> On Fri, Mar 5, 2021 at 3:51 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>>
>> (in response to
>> https://lore.kernel.org/linux-arm-kernel/20210303170932.1838634-1-jthierry@redhat.com/
>> from the command line)
>>
>>> Changes since v1[2]:
>>> - Drop gcc plugin in favor of -fno-jump-tables
>>
>> Thank you for this!  I built+booted(under emulation) arm64 defconfig and built
>> arm64 allmodconfig with LLVM=1 with this series applied.
>>
>> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
>>
>> One thing I noticed was a spew of warnings for allmodconfig, like:
>> init/main.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
>> init/main.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
>>
>> I assume those are from the KASAN constructors. See also:
>> https://github.com/ClangBuiltLinux/linux/issues/1238
>>
>> Can we disable HAVE_STACK_PROTECTOR if CC_IS_CLANG and CONFIG_KASAN is set,
>> until we can resolve the above issue?

So that concerns objtool for all arches, right?

> 
> Ah, filtering the logs more, it looks like GCOV is has the same issue
> KASAN does (known issue).  Here's a filtered log:
> https://gist.github.com/nickdesaulniers/01358015b33bd16ccd7d951c4a8c44e7
> 
> I'm curious about the failure to decode certain instructions?
> 

This is probably related to data constants present in code sections. To 
fix this, objtool needs to handle SYM_DATA_* annotations [1] and then 
the relevant bytes need to be annotated in the kernel sources (e.g. [2], 
but there are multiple parts in arm64 assembler needing this). I have 
not submitted those yet because I didn't want the amount of patches to 
become overwhelming and mixing objtool + kernel sources.

[1] 
https://github.com/julien-thierry/linux/commit/9005e9f3bb10aac663c42bb87d337b7a1aae5a67

[2] 
https://github.com/julien-thierry/linux/commit/ad132b032b4141c7ffce95d784b5254120e5bf65

> The stack state mismatches are what are valuable to me; we'll need
> some help digging into those at some point.  The logs from defconfig
> are clean.
> 

I think at the moment this is mostly a limitation of objtool to track 
code flow. aarch64 code tends to do a lot more register load/store 
inside a function than x86, and objtool doesn't track multiple register 
states (e.g. a registered stored at some offset on the stack at the 
beginning of the function, and later at some other stack offset). 
Although, when looking at the disassembled code, I'm not 100% I 
understand why there are so many intermediary store/load for these 
registers since it doesn't look like those values are actually used (I 
don't know whether this is caused by kasan/probes instrumentation).

But I'd need to investigate a bit more.