Re: [RFC PATCH v2 13/13] objtool: arm64: Enable stack validation for arm64
On 3/7/21 11:25 AM, Ard Biesheuvel wrote: On Wed, 3 Mar 2021 at 18:10, Julien Thierry wrote: From: Raphael Gault Add build option to run stack validation at compile time. When requiring stack validation, jump tables are disabled as it simplifies objtool analysis (without having to introduce unreliable artifacs). In local testing, this does not appear to significaly affect final binary size nor system performance. Signed-off-by: Raphael Gault Signed-off-by: Julien Thierry --- arch/arm64/Kconfig | 1 + arch/arm64/Makefile | 4 2 files changed, 5 insertions(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1f212b47a48a..928323c03318 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -187,6 +187,7 @@ config ARM64 select MMU_GATHER_RCU_TABLE_FREE select HAVE_RSEQ select HAVE_STACKPROTECTOR + select HAVE_STACK_VALIDATION select HAVE_SYSCALL_TRACEPOINTS select HAVE_KPROBES select HAVE_KRETPROBES diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 5b84aec31ed3..b819fb2e8eda 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -136,6 +136,10 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_REGS),y) CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif +ifeq ($(CONFIG_STACK_VALIDATION),y) +KBUILD_CFLAGS += -fno-jump-tables +endif + This is a bit misleading: the Kconfig option in question is selected automatically in all cases, so jump tables are always disabled. So, at the moment, the arch Kconfig only advertises that arm64 has stack validation with objtool, but currently stack validation itself is not enabled by default. However, I think disabling jump tables make sense anyway, at least when building the relocatable kernel for KASLR: we currently don't use -fpic/fpie in that case when building the vmlinux objects (because we don't want/need GOT tables), and so jump tables are emitted using absolute addresses, which induce some space overhead in the image. (24 bytes of RELA data per absolute address) ... unless I am missing something, and jump tables can/will be emitted as relative, even when not compiling in PIC mode? Personally I don't have enough context to assess whether it's the way to go. But if nobody opposes I'm fine having -fno-jump-tables in the default arm64 CFLAGS. Thanks, -- Julien Thierry
Re: [RFC PATCH v2 00/13] objtool: add base support for arm64
On 3/6/21 1:04 AM, Nick Desaulniers wrote: On Fri, Mar 5, 2021 at 3:51 PM Nick Desaulniers wrote: (in response to https://lore.kernel.org/linux-arm-kernel/20210303170932.1838634-1-jthie...@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 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. -- Julien Thierry
Re: [RFC PATCH v2 00/13] objtool: add base support for arm64
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 --- 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
[RFC PATCH v2 12/13] objtool: arm64: Ignore replacement section for alternative callback
ARM64_CB_PATCH doesn't have static replacement instructions. Skip trying to validate the alternative section. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/special.c | 12 tools/objtool/check.c | 3 +++ 2 files changed, 15 insertions(+) diff --git a/tools/objtool/arch/arm64/special.c b/tools/objtool/arch/arm64/special.c index a70b91e8bd7d..ed642bd6f886 100644 --- a/tools/objtool/arch/arm64/special.c +++ b/tools/objtool/arch/arm64/special.c @@ -4,6 +4,18 @@ void arch_handle_alternative(unsigned short feature, struct special_alt *alt) { + if (alt->orig_len && !alt->new_len) { + /* +* ARM64_CB_PATCH has no alternative instruction. +* a callback is called at alternative replacement time +* to dynamically change the original instructions. +* +* ARM64_CB_PATCH is the last ARM64 feature, it's value changes +* every time a new feature is added. So the orig/alt region +* length are used to detect those alternatives +*/ + alt->skip_alt = true; + } } bool arch_support_alt_relocation(struct special_alt *special_alt, diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 7750f6342855..1999d1f1967a 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1293,6 +1293,9 @@ static int add_special_section_alts(struct objtool_file *file) continue; } + if (special_alt->skip_alt && !special_alt->new_len) + continue; + ret = handle_group_alt(file, special_alt, orig_insn, &new_insn); if (ret) -- 2.25.4
[RFC PATCH v2 03/13] tools: bug: Remove duplicate definition
Under tools, bug.h only defines BUILD_BUG_ON_ZERO() which is already defined in build_bug.h. This prevents a file to include both headers at the same time. Have bug.h include build_bug.h instead. Signed-off-by: Julien Thierry --- tools/include/linux/bug.h | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/include/linux/bug.h b/tools/include/linux/bug.h index 85f80258a15f..548be7cffa8e 100644 --- a/tools/include/linux/bug.h +++ b/tools/include/linux/bug.h @@ -2,10 +2,6 @@ #ifndef _TOOLS_PERF_LINUX_BUG_H #define _TOOLS_PERF_LINUX_BUG_H -/* Force a compilation error if condition is true, but also produce a - result (of value 0 and type size_t), so the expression can be used - e.g. in a structure initializer (or where-ever else comma expressions - aren't permitted). */ -#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); })) +#include #endif /* _TOOLS_PERF_LINUX_BUG_H */ -- 2.25.4
[RFC PATCH v2 07/13] objtool: arm64: Decode other system instructions
Decode ERET, BRK and NOPs Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 983f16b8b2af..3008dcbb5e64 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -233,6 +233,13 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* Remaining branch opcodes are conditional */ *type = INSN_JUMP_CONDITIONAL; *immediate = aarch64_get_branch_offset(insn); + } else if (aarch64_insn_is_eret(insn)) { + *type = INSN_CONTEXT_SWITCH; + } else if (aarch64_insn_is_steppable_hint(insn)) { + *type = INSN_NOP; + } else if (aarch64_insn_is_brk(insn)) { + *immediate = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_16, insn); + *type = INSN_BUG; } else { *type = INSN_OTHER; } -- 2.25.4
[RFC PATCH v2 09/13] objtool: arm64: Decode LDR instructions
Load literal instructions can generate constants inside code sections. Record the locations of the constants in order to be able to remove their corresponding "struct instruction". Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c| 86 tools/objtool/arch/x86/decode.c | 5 ++ tools/objtool/check.c| 3 + tools/objtool/include/objtool/arch.h | 3 + 4 files changed, 97 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 4e086d2251f5..b4631d79f13f 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -30,6 +30,73 @@ static unsigned long sign_extend(unsigned long x, int nbits) return ((~0UL + (sign_bit ^ 1)) << nbits) | x; } +struct insn_loc { + const struct section *sec; + unsigned long offset; + struct hlist_node hnode; + bool ignorable; +}; + +DEFINE_HASHTABLE(invalid_insns, 16); + +static int record_invalid_insn(const struct section *sec, + unsigned long offset, + bool ignore) +{ + struct insn_loc *loc; + struct hlist_head *l; + + l = &invalid_insns[hash_min(offset, HASH_BITS(invalid_insns))]; + if (!hlist_empty(l)) { + loc = hlist_entry(l->first, struct insn_loc, hnode); + loc->ignorable |= ignore; + return 0; + } + + loc = malloc(sizeof(*loc)); + if (!loc) { + WARN("malloc failed"); + return -1; + } + + loc->sec = sec; + loc->offset = offset; + loc->ignorable = ignore; + + hash_add(invalid_insns, &loc->hnode, loc->offset); + + return 0; +} + +int arch_post_process_instructions(struct objtool_file *file) +{ + struct hlist_node *tmp; + struct insn_loc *loc; + unsigned int bkt; + int res = 0; + + hash_for_each_safe(invalid_insns, bkt, tmp, loc, hnode) { + struct instruction *insn; + + insn = find_insn(file, (struct section *) loc->sec, loc->offset); + if (insn) { + if (loc->ignorable) { + list_del(&insn->list); + hash_del(&insn->hash); + free(insn); + } else { + WARN_FUNC("can't decode instruction", insn->sec, insn->offset); + return -1; + } + } + + hash_del(&loc->hnode); + free(loc); + } + + return res; +} + bool arch_callee_saved_reg(unsigned char reg) { switch (reg) { @@ -389,6 +456,25 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, if (ret <= 0) return ret; + if (aarch64_insn_is_ldr_lit(insn)) { + long pc_offset; + + pc_offset = insn & GENMASK(23, 5); + /* Sign extend and multiply by 4 */ + pc_offset = (pc_offset << (64 - 23)); + pc_offset = ((pc_offset >> (64 - 23)) >> 5) << 2; + + if (record_invalid_insn(sec, offset + pc_offset, true)) + return -1; + + /* 64-bit literal */ + if (insn & BIT(30)) { + if (record_invalid_insn(sec, + offset + pc_offset + 4, + true)) + return -1; + } + } *type = INSN_OTHER; break; } diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 431bafb881d4..54f57bfd6f8f 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -619,6 +619,11 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, return 0; } +int arch_post_process_instructions(struct objtool_file *file) +{ + return 0; +} + void arch_initial_func_cfi_state(struct cfi_init_state *state) { int i; diff --git a/tools/objtool/check.c b/tools/objtool/check.c index a0f762a15ad5..7750f6342855 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -325,6 +325,9 @@ static int decode_instructions(struct objtool_file *file) if (stats) printf("nr_insns: %lu\n", nr_insns); + if (arch_post_process_instructions(file)) + return -1; + return 0; err: diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/includ
[RFC PATCH v2 06/13] objtool: arm64: Decode jump and call related instructions
Decode branch, branch and link (aarch64's call) and return instructions. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 54eeb8704a42..983f16b8b2af 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -215,6 +215,28 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, } *type = INSN_OTHER; break; + case AARCH64_INSN_CLS_BR_SYS: + if (aarch64_insn_is_ret(insn) && + aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, insn) == AARCH64_INSN_REG_LR) { + *type = INSN_RETURN; + } else if (aarch64_insn_is_bl(insn)) { + *type = INSN_CALL; + *immediate = aarch64_get_branch_offset(insn); + } else if (aarch64_insn_is_blr(insn)) { + *type = INSN_CALL_DYNAMIC; + } else if (aarch64_insn_is_b(insn)) { + *type = INSN_JUMP_UNCONDITIONAL; + *immediate = aarch64_get_branch_offset(insn); + } else if (aarch64_insn_is_br(insn)) { + *type = INSN_JUMP_DYNAMIC; + } else if (aarch64_insn_is_branch_imm(insn)) { + /* Remaining branch opcodes are conditional */ + *type = INSN_JUMP_CONDITIONAL; + *immediate = aarch64_get_branch_offset(insn); + } else { + *type = INSN_OTHER; + } + break; default: *type = INSN_OTHER; break; -- 2.25.4
[RFC PATCH v2 02/13] tools: arm64: Make aarch64 instruction decoder available to tools
Add aarch64 encoder/decoder implementation under tools/ as well as the necessary arm64 headers. Signed-off-by: Julien Thierry --- tools/arch/arm64/include/asm/insn.h | 565 +++ tools/arch/arm64/lib/insn.c | 1456 +++ 2 files changed, 2021 insertions(+) create mode 100644 tools/arch/arm64/include/asm/insn.h create mode 100644 tools/arch/arm64/lib/insn.c diff --git a/tools/arch/arm64/include/asm/insn.h b/tools/arch/arm64/include/asm/insn.h new file mode 100644 index ..71de52d1532f --- /dev/null +++ b/tools/arch/arm64/include/asm/insn.h @@ -0,0 +1,565 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu + * + * Copyright (C) 2014 Zi Shen Lim + */ +#ifndef__ASM_INSN_H +#define__ASM_INSN_H +#include +#include + +/* A64 instructions are always 32 bits. */ +#define AARCH64_INSN_SIZE 4 + +#ifndef __ASSEMBLY__ +/* + * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a + * Section C3.1 "A64 instruction index by encoding": + * AArch64 main encoding table + * Bit position + * 28 27 26 25 Encoding Group + * 0 0 - -Unallocated + * 1 0 0 -Data processing, immediate + * 1 0 1 -Branch, exception generation and system instructions + * - 1 - 0Loads and stores + * - 1 0 1Data processing - register + * 0 1 1 1Data processing - SIMD and floating point + * 1 1 1 1Data processing - SIMD and floating point + * "-" means "don't care" + */ +enum aarch64_insn_encoding_class { + AARCH64_INSN_CLS_UNKNOWN, /* UNALLOCATED */ + AARCH64_INSN_CLS_SVE, /* SVE instructions */ + AARCH64_INSN_CLS_DP_IMM,/* Data processing - immediate */ + AARCH64_INSN_CLS_DP_REG,/* Data processing - register */ + AARCH64_INSN_CLS_DP_FPSIMD, /* Data processing - SIMD and FP */ + AARCH64_INSN_CLS_LDST, /* Loads and stores */ + AARCH64_INSN_CLS_BR_SYS,/* Branch, exception generation and +* system instructions */ +}; + +enum aarch64_insn_hint_cr_op { + AARCH64_INSN_HINT_NOP = 0x0 << 5, + AARCH64_INSN_HINT_YIELD = 0x1 << 5, + AARCH64_INSN_HINT_WFE = 0x2 << 5, + AARCH64_INSN_HINT_WFI = 0x3 << 5, + AARCH64_INSN_HINT_SEV = 0x4 << 5, + AARCH64_INSN_HINT_SEVL = 0x5 << 5, + + AARCH64_INSN_HINT_XPACLRI= 0x07 << 5, + AARCH64_INSN_HINT_PACIA_1716 = 0x08 << 5, + AARCH64_INSN_HINT_PACIB_1716 = 0x0A << 5, + AARCH64_INSN_HINT_AUTIA_1716 = 0x0C << 5, + AARCH64_INSN_HINT_AUTIB_1716 = 0x0E << 5, + AARCH64_INSN_HINT_PACIAZ = 0x18 << 5, + AARCH64_INSN_HINT_PACIASP= 0x19 << 5, + AARCH64_INSN_HINT_PACIBZ = 0x1A << 5, + AARCH64_INSN_HINT_PACIBSP= 0x1B << 5, + AARCH64_INSN_HINT_AUTIAZ = 0x1C << 5, + AARCH64_INSN_HINT_AUTIASP= 0x1D << 5, + AARCH64_INSN_HINT_AUTIBZ = 0x1E << 5, + AARCH64_INSN_HINT_AUTIBSP= 0x1F << 5, + + AARCH64_INSN_HINT_ESB = 0x10 << 5, + AARCH64_INSN_HINT_PSB = 0x11 << 5, + AARCH64_INSN_HINT_TSB = 0x12 << 5, + AARCH64_INSN_HINT_CSDB = 0x14 << 5, + + AARCH64_INSN_HINT_BTI = 0x20 << 5, + AARCH64_INSN_HINT_BTIC = 0x22 << 5, + AARCH64_INSN_HINT_BTIJ = 0x24 << 5, + AARCH64_INSN_HINT_BTIJC = 0x26 << 5, +}; + +enum aarch64_insn_imm_type { + AARCH64_INSN_IMM_ADR, + AARCH64_INSN_IMM_26, + AARCH64_INSN_IMM_19, + AARCH64_INSN_IMM_16, + AARCH64_INSN_IMM_14, + AARCH64_INSN_IMM_12, + AARCH64_INSN_IMM_9, + AARCH64_INSN_IMM_7, + AARCH64_INSN_IMM_6, + AARCH64_INSN_IMM_S, + AARCH64_INSN_IMM_R, + AARCH64_INSN_IMM_N, + AARCH64_INSN_IMM_MAX +}; + +enum aarch64_insn_register_type { + AARCH64_INSN_REGTYPE_RT, + AARCH64_INSN_REGTYPE_RN, + AARCH64_INSN_REGTYPE_RT2, + AARCH64_INSN_REGTYPE_RM, + AARCH64_INSN_REGTYPE_RD, + AARCH64_INSN_REGTYPE_RA, + AARCH64_INSN_REGTYPE_RS, +}; + +enum aarch64_insn_register { + AARCH64_INSN_REG_0 = 0, + AARCH64_INSN_REG_1 = 1, + AARCH64_INSN_REG_2 = 2, + AARCH64_INSN_REG_3 = 3, + AARCH64_INSN_REG_4 = 4, + AARCH64_INSN_REG_5 = 5, + AARCH64_INSN_REG_6 = 6, + AARCH64_INSN_REG_7 = 7, + AARCH64_INSN_REG_8 = 8, + AARCH64_INSN_REG_9 = 9, + AARCH64_INSN_REG_10 = 10, + AARCH64_INSN_REG_11 = 11, + AARCH64_INSN_REG_12 = 12, + AARCH64_INSN_REG_13 = 13, + AARCH64_INSN_REG_14 = 14, + AARCH64_INSN_REG_15 = 15,
[RFC PATCH v2 01/13] tools: Add some generic functions and headers
These will be needed to be able to use arm64 instruction decoder in userland tools. Signed-off-by: Julien Thierry --- tools/include/asm-generic/bitops/__ffs.h | 11 +++ tools/include/linux/kernel.h | 21 + tools/include/linux/printk.h | 40 3 files changed, 72 insertions(+) create mode 100644 tools/include/linux/printk.h diff --git a/tools/include/asm-generic/bitops/__ffs.h b/tools/include/asm-generic/bitops/__ffs.h index 9d1310519497..963f8a22212f 100644 --- a/tools/include/asm-generic/bitops/__ffs.h +++ b/tools/include/asm-generic/bitops/__ffs.h @@ -42,4 +42,15 @@ static __always_inline unsigned long __ffs(unsigned long word) return num; } +static inline unsigned long __ffs64(u64 word) +{ +#if BITS_PER_LONG == 32 + if (((u32)word) == 0UL) + return __ffs((u32)(word >> 32)) + 32; +#elif BITS_PER_LONG != 64 +#error BITS_PER_LONG not 32 or 64 +#endif + return __ffs((unsigned long)word); +} + #endif /* _TOOLS_LINUX_ASM_GENERIC_BITOPS___FFS_H_ */ diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h index a7e54a08fb54..e748982ed5c1 100644 --- a/tools/include/linux/kernel.h +++ b/tools/include/linux/kernel.h @@ -114,6 +114,27 @@ int scnprintf_pad(char * buf, size_t size, const char * fmt, ...); #define round_up(x, y) x)-1) | __round_mask(x, y))+1) #define round_down(x, y) ((x) & ~__round_mask(x, y)) +/** + * upper_32_bits - return bits 32-63 of a number + * @n: the number we're accessing + * + * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress + * the "right shift count >= width of type" warning when that quantity is + * 32-bits. + */ +#define upper_32_bits(n) ((u32)(((n) >> 16) >> 16)) + +/** + * lower_32_bits - return bits 0-31 of a number + * @n: the number we're accessing + */ +#define lower_32_bits(n) ((u32)(n)) + +/* Inspired from ALIGN_*_KERNEL */ +#define __ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask)) +#define __ALIGN(x, a) __ALIGN_MASK(x, (typeof(x))(a) - 1) +#define ALIGN_DOWN(x, a) __ALIGN((x) - ((a) - 1), (a)) + #define current_gfp_context(k) 0 #define synchronize_rcu() diff --git a/tools/include/linux/printk.h b/tools/include/linux/printk.h new file mode 100644 index ..515ebdc47e6e --- /dev/null +++ b/tools/include/linux/printk.h @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_LINUX_KERNEL_PRINTK_H_ +#define _TOOLS_LINUX_KERNEL_PRINTK_H_ + +#include +#include +#include + +#define printk(fmt, ...) fprintf(stdout, fmt, ##__VA_ARGS__) +#define pr_info printk +#define pr_notice printk +#define pr_cont printk + +#define pr_warn(fmt, ...) fprintf(stderr, fmt, ##__VA_ARGS__) +#define pr_err pr_warn +#define pr_alert pr_warn +#define pr_emerg pr_warn +#define pr_crit pr_warn + +/* + * Dummy printk for disabled debugging statements to use whilst maintaining + * gcc's format checking. + */ +#define no_printk(fmt, ...)\ +({ \ + if (0) \ + printk(fmt, ##__VA_ARGS__); \ + 0; \ +}) + +/* pr_devel() should produce zero code unless DEBUG is defined */ +#ifdef DEBUG +#define pr_devel(fmt, ...) printk +#else +#define pr_devel(fmt, ...) no_printk +#endif + +#define pr_debug pr_devel + +#endif /* _TOOLS_LINUX_KERNEL_PRINTK_H_ */ -- 2.25.4
[RFC PATCH v2 13/13] objtool: arm64: Enable stack validation for arm64
From: Raphael Gault Add build option to run stack validation at compile time. When requiring stack validation, jump tables are disabled as it simplifies objtool analysis (without having to introduce unreliable artifacs). In local testing, this does not appear to significaly affect final binary size nor system performance. Signed-off-by: Raphael Gault Signed-off-by: Julien Thierry --- arch/arm64/Kconfig | 1 + arch/arm64/Makefile | 4 2 files changed, 5 insertions(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1f212b47a48a..928323c03318 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -187,6 +187,7 @@ config ARM64 select MMU_GATHER_RCU_TABLE_FREE select HAVE_RSEQ select HAVE_STACKPROTECTOR + select HAVE_STACK_VALIDATION select HAVE_SYSCALL_TRACEPOINTS select HAVE_KPROBES select HAVE_KRETPROBES diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 5b84aec31ed3..b819fb2e8eda 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -136,6 +136,10 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_REGS),y) CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif +ifeq ($(CONFIG_STACK_VALIDATION),y) +KBUILD_CFLAGS += -fno-jump-tables +endif + # Default value head-y := arch/arm64/kernel/head.o -- 2.25.4
[RFC PATCH v2 10/13] objtool: arm64: Accept padding in code sections
The compiler can introduce some '0' words in code sections to pad the end of functions. Similar to load literal functions, record these zero words to remove the "struct instruction" created for them. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index b4631d79f13f..592276c199eb 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -385,8 +385,23 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, switch (aarch64_get_insn_class(insn)) { case AARCH64_INSN_CLS_UNKNOWN: - WARN("can't decode instruction at %s:0x%lx", sec->name, offset); - return -1; + { + /* +* There are a few reasons we might have non-valid opcodes in +* code sections: +* - For load literal, assembler can generate the data to be +* loaded in the code section +* - Compiler/assembler can generate zeroes to pad function that +* do not end on 8-byte alignment +*/ + /* Compiler might put zeroes as padding */ + if (record_invalid_insn(sec, offset, insn == 0x0)) + return -1; + + *type = INSN_OTHER; + + break; + } case AARCH64_INSN_CLS_DP_IMM: /* Mov register to and from SP are aliases of add_imm */ if (aarch64_insn_is_add_imm(insn) || -- 2.25.4
[RFC PATCH v2 08/13] objtool: arm64: Decode load/store instructions
Decode load/store operations and create corresponding stack_ops for operations targetting SP or FP. Operations storing/loading multiple registers are split into separate stack_ops storing single registers. Operations modifying the base register get an additional stack_op for the register update. Since the atomic register(s) load/store + base register update gets split into multiple operations, to make sure objtool always sees a valid stack, consider store instruction to perform stack allocations (i.e. modifying the base pointer before the storing) and loads de-allocations (i.e. modifying the base pointer after the load). Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 148 ++ 1 file changed, 148 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 3008dcbb5e64..4e086d2251f5 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -105,6 +105,48 @@ int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg) return -1; } +static struct stack_op *arm_make_store_op(enum aarch64_insn_register base, + enum aarch64_insn_register reg, + int offset) +{ + struct stack_op *op; + + op = calloc(1, sizeof(*op)); + if (!op) { + WARN("calloc failed"); + return NULL; + } + op->dest.type = OP_DEST_REG_INDIRECT; + op->dest.reg = base; + op->dest.offset = offset; + op->src.type = OP_SRC_REG; + op->src.reg = reg; + op->src.offset = 0; + + return op; +} + +static struct stack_op *arm_make_load_op(enum aarch64_insn_register base, +enum aarch64_insn_register reg, +int offset) +{ + struct stack_op *op; + + op = calloc(1, sizeof(*op)); + if (!op) { + WARN("calloc failed"); + return NULL; + } + op->dest.type = OP_DEST_REG; + op->dest.reg = reg; + op->dest.offset = 0; + op->src.type = OP_SRC_REG_INDIRECT; + op->src.reg = base; + op->src.offset = offset; + + return op; +} + static struct stack_op *arm_make_add_op(enum aarch64_insn_register dest, enum aarch64_insn_register src, int val) @@ -125,6 +167,101 @@ static struct stack_op *arm_make_add_op(enum aarch64_insn_register dest, return op; } +static int arm_decode_load_store(u32 insn, enum insn_type *type, +unsigned long *immediate, +struct list_head *ops_list) +{ + enum aarch64_insn_register base; + enum aarch64_insn_register rt; + struct stack_op *op; + int size; + int offset; + + *type = INSN_OTHER; + + if (aarch64_insn_is_store_single(insn) || + aarch64_insn_is_load_single(insn)) + size = 1 << ((insn & GENMASK(31, 30)) >> 30); + else + size = 4 << ((insn >> 31) & 1); + + if (aarch64_insn_is_store_imm(insn) || aarch64_insn_is_load_imm(insn)) + *immediate = size * aarch64_insn_decode_immediate(AARCH64_INSN_IMM_12, + insn); + else if (aarch64_insn_is_store_pre(insn) || +aarch64_insn_is_load_pre(insn) || +aarch64_insn_is_store_post(insn) || +aarch64_insn_is_load_post(insn)) + *immediate = sign_extend(aarch64_insn_decode_immediate(AARCH64_INSN_IMM_9, + insn), +9); + else if (aarch64_insn_is_stp(insn) || aarch64_insn_is_ldp(insn) || +aarch64_insn_is_stp_pre(insn) || +aarch64_insn_is_ldp_pre(insn) || +aarch64_insn_is_stp_post(insn) || +aarch64_insn_is_ldp_post(insn)) + *immediate = size * sign_extend(aarch64_insn_decode_immediate(AARCH64_INSN_IMM_7, + insn), + 7); + else + return 1; + + base = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, insn); + if (base != AARCH64_INSN_REG_FP && base != AARCH64_INSN_REG_SP) + return 0; + + offset = *immediate; + + if (aarch64_insn_is_store_pre(insn) || aarch64_insn_is_stp_pre(insn) || + aarch64_insn_is_store_post(insn) || aarch64_insn_is_stp_post(insn)) { + op = arm_make_add_op(base, base, *immediate); + list_add_tail(&op->li
[RFC PATCH v2 8/8] arm64: insn: Add load/store decoding helpers
Provide some function to group different load/store instructions. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/insn.h | 28 1 file changed, 28 insertions(+) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 388aa22eacb1..71de52d1532f 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -412,6 +412,34 @@ static inline bool aarch64_insn_is_barrier(u32 insn) aarch64_insn_is_pssbb(insn); } +static inline bool aarch64_insn_is_store_single(u32 insn) +{ + return aarch64_insn_is_store_imm(insn) || + aarch64_insn_is_store_pre(insn) || + aarch64_insn_is_store_post(insn); +} + +static inline bool aarch64_insn_is_store_pair(u32 insn) +{ + return aarch64_insn_is_stp(insn) || + aarch64_insn_is_stp_pre(insn) || + aarch64_insn_is_stp_post(insn); +} + +static inline bool aarch64_insn_is_load_single(u32 insn) +{ + return aarch64_insn_is_load_imm(insn) || + aarch64_insn_is_load_pre(insn) || + aarch64_insn_is_load_post(insn); +} + +static inline bool aarch64_insn_is_load_pair(u32 insn) +{ + return aarch64_insn_is_ldp(insn) || + aarch64_insn_is_ldp_pre(insn) || + aarch64_insn_is_ldp_post(insn); +} + enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); bool aarch64_insn_uses_literal(u32 insn); bool aarch64_insn_is_branch(u32 insn); -- 2.25.4
[RFC PATCH v2 05/13] objtool: arm64: Decode add/sub instructions
Decode aarch64 additions and substractions and create stack_ops for instructions interacting with SP or FP. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 94 +++ 1 file changed, 94 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 3ec0254f7306..54eeb8704a42 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -23,6 +23,13 @@ #include "../../../arch/arm64/lib/insn.c" +static unsigned long sign_extend(unsigned long x, int nbits) +{ + unsigned long sign_bit = (x >> (nbits - 1)) & 1; + + return ((~0UL + (sign_bit ^ 1)) << nbits) | x; +} + bool arch_callee_saved_reg(unsigned char reg) { switch (reg) { @@ -98,6 +105,61 @@ int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg) return -1; } +static struct stack_op *arm_make_add_op(enum aarch64_insn_register dest, + enum aarch64_insn_register src, + int val) +{ + struct stack_op *op; + + op = calloc(1, sizeof(*op)); + if (!op) { + WARN("calloc failed"); + return NULL; + } + op->dest.type = OP_DEST_REG; + op->dest.reg = dest; + op->src.reg = src; + op->src.type = val != 0 ? OP_SRC_ADD : OP_SRC_REG; + op->src.offset = val; + + return op; +} + +static int arm_decode_add_sub_imm(u32 instr, bool set_flags, + enum insn_type *type, + unsigned long *immediate, + struct list_head *ops_list) +{ + u32 rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, instr); + u32 rn = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, instr); + + *type = INSN_OTHER; + *immediate = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_12, instr); + + if (instr & AARCH64_INSN_LSL_12) + *immediate <<= 12; + + if ((!set_flags && rd == AARCH64_INSN_REG_SP) || + rd == AARCH64_INSN_REG_FP || + rn == AARCH64_INSN_REG_FP || + rn == AARCH64_INSN_REG_SP) { + struct stack_op *op; + int value; + + if (aarch64_insn_is_subs_imm(instr) || aarch64_insn_is_sub_imm(instr)) + value = -*immediate; + else + value = *immediate; + + op = arm_make_add_op(rd, rn, value); + if (!op) + return -1; + list_add_tail(&op->list, ops_list); + } + + return 0; +} + int arch_decode_instruction(const struct elf *elf, const struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, @@ -121,6 +183,38 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case AARCH64_INSN_CLS_UNKNOWN: WARN("can't decode instruction at %s:0x%lx", sec->name, offset); return -1; + case AARCH64_INSN_CLS_DP_IMM: + /* Mov register to and from SP are aliases of add_imm */ + if (aarch64_insn_is_add_imm(insn) || + aarch64_insn_is_sub_imm(insn)) + return arm_decode_add_sub_imm(insn, false, type, immediate, + ops_list); + else if (aarch64_insn_is_adds_imm(insn) || +aarch64_insn_is_subs_imm(insn)) + return arm_decode_add_sub_imm(insn, true, type, immediate, + ops_list); + else + *type = INSN_OTHER; + break; + case AARCH64_INSN_CLS_DP_REG: + if (aarch64_insn_is_mov_reg(insn)) { + enum aarch64_insn_register rd; + enum aarch64_insn_register rm; + + rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, insn); + rm = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RM, insn); + if (rd == AARCH64_INSN_REG_FP || rm == AARCH64_INSN_REG_FP) { + struct stack_op *op; + + op = arm_make_add_op(rd, rm, 0); + if (!op) + return -1; + list_add_tail(&op->list, ops_list); + break; + } + } + *type = INSN_OTHER; + break; default: *type = INSN_OTHER; break; -- 2.25.4
[RFC PATCH v2 04/13] objtool: arm64: Add base definition for arm64 backend
Provide needed definitions for a new architecture instruction decoder. No proper decoding is done yet. Signed-off-by: Julien Thierry --- tools/objtool/Makefile| 5 + tools/objtool/arch/arm64/Build| 8 ++ tools/objtool/arch/arm64/decode.c | 130 ++ .../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| 21 +++ tools/objtool/sync-check.sh | 5 + 9 files changed, 219 insertions(+) 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 diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index 92ce4fce7bc7..d5cfbec87c02 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -47,6 +47,11 @@ ifeq ($(SRCARCH),x86) SUBCMD_ORC := y endif +ifeq ($(SRCARCH),arm64) + SUBCMD_CHECK := y + CFLAGS += -Wno-nested-externs +endif + export SUBCMD_CHECK SUBCMD_ORC export srctree OUTPUT CFLAGS SRCARCH AWK include $(srctree)/tools/build/Makefile.include diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build new file mode 100644 index ..f3de3a50d541 --- /dev/null +++ b/tools/objtool/arch/arm64/Build @@ -0,0 +1,8 @@ +objtool-y += special.o +objtool-y += decode.o + +objtool-y += libhweight.o + +$(OUTPUT)arch/arm64/libhweight.o: ../lib/hweight.c FORCE + $(call rule_mkdir) + $(call if_changed_dep,cc_o_c) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c new file mode 100644 index ..3ec0254f7306 --- /dev/null +++ b/tools/objtool/arch/arm64/decode.c @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include +#include +#include + +#include + +#include +#include +#include +#include + +#include + +/* Hack needed to avoid depending on debug-monitors.h */ +#define AARCH64_BREAK_FAULT0xBAD + +/* Hack needed to avoid depending on kprobes.h */ +#ifndef __kprobes +#define __kprobes +#endif + +#include "../../../arch/arm64/lib/insn.c" + +bool arch_callee_saved_reg(unsigned char reg) +{ + switch (reg) { + case AARCH64_INSN_REG_19: + case AARCH64_INSN_REG_20: + case AARCH64_INSN_REG_21: + case AARCH64_INSN_REG_22: + case AARCH64_INSN_REG_23: + case AARCH64_INSN_REG_24: + case AARCH64_INSN_REG_25: + case AARCH64_INSN_REG_26: + case AARCH64_INSN_REG_27: + case AARCH64_INSN_REG_28: + case AARCH64_INSN_REG_FP: + case AARCH64_INSN_REG_LR: + return true; + default: + return false; + } +} + +void arch_initial_func_cfi_state(struct cfi_init_state *state) +{ + int i; + + for (i = 0; i < CFI_NUM_REGS; i++) { + state->regs[i].base = CFI_UNDEFINED; + state->regs[i].offset = 0; + } + + /* initial CFA (call frame address) */ + state->cfa.base = CFI_SP; + state->cfa.offset = 0; +} + +unsigned long arch_dest_reloc_offset(int addend) +{ + return addend; +} + +unsigned long arch_jump_destination(struct instruction *insn) +{ + return insn->offset + insn->immediate; +} + +const char *arch_nop_insn(int len) +{ + static u32 nop = 0; + + if (len != AARCH64_INSN_SIZE) + WARN("invalid NOP size: %d\n", len); + + if (!nop) + nop = aarch64_insn_gen_nop(); + + return (const char*)&nop; +} + +static int is_arm64(const struct elf *elf) +{ + switch (elf->ehdr.e_machine) { + case EM_AARCH64: //0xB7 + return 1; + default: + WARN("unexpected ELF machine type %x", +elf->ehdr.e_machine); + return 0; + } +} + +int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg) +{ + return -1; +} + +int arch_decode_instruction(const struct elf *elf, const struct section *sec, + unsigned long offset, unsigned int maxlen, + unsigned int *len, enum insn_type *type, + unsigned long *immediate, + struct list_head *ops_list) +{ + u32 insn; + + if (!is_arm64(elf)) + return -1; + + if (maxlen < AARCH64_INSN_SIZE) + return 0; + + *len = AARCH64_INSN_SIZE; + *immediate = 0; + +
[RFC PATCH v2 00/13] objtool: add base support for arm64
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
[RFC PATCH v2 11/13] objtool: arm64: Handle supported relocations in alternatives
Based on get_alt_insn() in arch/arm64/kernel/alternative.c, arm64 alternative code adapts offsets for static branches and adrp instructions. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/special.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/objtool/arch/arm64/special.c b/tools/objtool/arch/arm64/special.c index 45f283283091..a70b91e8bd7d 100644 --- a/tools/objtool/arch/arm64/special.c +++ b/tools/objtool/arch/arm64/special.c @@ -10,7 +10,11 @@ bool arch_support_alt_relocation(struct special_alt *special_alt, struct instruction *insn, struct reloc *reloc) { - return false; + u32 opcode = *(u32 *)(insn->sec->data->d_buf + insn->offset); + + return aarch64_insn_is_branch_imm(opcode) || + aarch64_insn_is_adrp(opcode) || + !aarch64_insn_uses_literal(opcode); } -- 2.25.4
[RFC PATCH v2 7/8] arm64: insn: Add some opcodes to instruction decoder
Add decoding capability for some instructions that objtool will need to decode. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/insn.h | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 185f52ef0228..388aa22eacb1 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -296,6 +296,12 @@ __AARCH64_INSN_FUNCS(adr, 0x9F00, 0x1000) __AARCH64_INSN_FUNCS(adrp, 0x9F00, 0x9000) __AARCH64_INSN_FUNCS(prfm, 0x3FC0, 0x3980) __AARCH64_INSN_FUNCS(prfm_lit, 0xFF00, 0xD800) +__AARCH64_INSN_FUNCS(store_imm,0x3FC0, 0x3900) +__AARCH64_INSN_FUNCS(load_imm, 0x3FC0, 0x3940) +__AARCH64_INSN_FUNCS(store_pre,0x3FE00C00, 0x38000C00) +__AARCH64_INSN_FUNCS(load_pre, 0x3FE00C00, 0x38400C00) +__AARCH64_INSN_FUNCS(store_post, 0x3FE00C00, 0x38000400) +__AARCH64_INSN_FUNCS(load_post,0x3FE00C00, 0x38400400) __AARCH64_INSN_FUNCS(str_reg, 0x3FE0EC00, 0x38206800) __AARCH64_INSN_FUNCS(ldadd,0x3F20FC00, 0x3820) __AARCH64_INSN_FUNCS(ldr_reg, 0x3FE0EC00, 0x38606800) @@ -304,6 +310,8 @@ __AARCH64_INSN_FUNCS(ldrsw_lit, 0xFF00, 0x9800) __AARCH64_INSN_FUNCS(exclusive,0x3F80, 0x0800) __AARCH64_INSN_FUNCS(load_ex, 0x3F40, 0x0840) __AARCH64_INSN_FUNCS(store_ex, 0x3F40, 0x0800) +__AARCH64_INSN_FUNCS(stp, 0x7FC0, 0x2900) +__AARCH64_INSN_FUNCS(ldp, 0x7FC0, 0x2940) __AARCH64_INSN_FUNCS(stp_post, 0x7FC0, 0x2880) __AARCH64_INSN_FUNCS(ldp_post, 0x7FC0, 0x28C0) __AARCH64_INSN_FUNCS(stp_pre, 0x7FC0, 0x2980) @@ -336,6 +344,7 @@ __AARCH64_INSN_FUNCS(rev64, 0x7C00, 0x5AC00C00) __AARCH64_INSN_FUNCS(and, 0x7F20, 0x0A00) __AARCH64_INSN_FUNCS(bic, 0x7F20, 0x0A20) __AARCH64_INSN_FUNCS(orr, 0x7F20, 0x2A00) +__AARCH64_INSN_FUNCS(mov_reg, 0x7FE0FFE0, 0x2A0003E0) __AARCH64_INSN_FUNCS(orn, 0x7F20, 0x2A20) __AARCH64_INSN_FUNCS(eor, 0x7F20, 0x4A00) __AARCH64_INSN_FUNCS(eon, 0x7F20, 0x4A20) -- 2.25.4
[RFC PATCH v2 6/8] arm64: insn: Add barrier encodings
Create necessary functions to encode/decode aarch64 barrier instructions. DSB needs special case handling as it has multiple encodings. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/insn.h | 22 ++ 1 file changed, 22 insertions(+) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index b55b629c5eab..185f52ef0228 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -370,6 +370,14 @@ __AARCH64_INSN_FUNCS(eret_auth,0xFBFF, 0xD69F0BFF) __AARCH64_INSN_FUNCS(mrs, 0xFFF0, 0xD530) __AARCH64_INSN_FUNCS(msr_imm, 0xFFF8F01F, 0xD500401F) __AARCH64_INSN_FUNCS(msr_reg, 0xFFF0, 0xD510) +__AARCH64_INSN_FUNCS(dmb, 0xF0FF, 0xD50330BF) +__AARCH64_INSN_FUNCS(dsb_base, 0xF0FF, 0xD503309F) +__AARCH64_INSN_FUNCS(dsb_nxs, 0xF3FF, 0xD503323F) +__AARCH64_INSN_FUNCS(isb, 0xF0FF, 0xD50330DF) +__AARCH64_INSN_FUNCS(sb, 0x, 0xD50330FF) +__AARCH64_INSN_FUNCS(clrex,0xF0FF, 0xD503305F) +__AARCH64_INSN_FUNCS(ssbb, 0x, 0xD503309F) +__AARCH64_INSN_FUNCS(pssbb,0x, 0xD503349F) #undef __AARCH64_INSN_FUNCS @@ -381,6 +389,20 @@ static inline bool aarch64_insn_is_adr_adrp(u32 insn) return aarch64_insn_is_adr(insn) || aarch64_insn_is_adrp(insn); } +static inline bool aarch64_insn_is_dsb(u32 insn) +{ + return (aarch64_insn_is_dsb_base(insn) && (insn & 0xb00)) || + aarch64_insn_is_dsb_nxs(insn); +} + +static inline bool aarch64_insn_is_barrier(u32 insn) +{ + return aarch64_insn_is_dmb(insn) || aarch64_insn_is_dsb(insn) || + aarch64_insn_is_isb(insn) || aarch64_insn_is_sb(insn) || + aarch64_insn_is_clrex(insn) || aarch64_insn_is_ssbb(insn) || + aarch64_insn_is_pssbb(insn); +} + enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); bool aarch64_insn_uses_literal(u32 insn); bool aarch64_insn_is_branch(u32 insn); -- 2.25.4
[RFC PATCH v2 3/8] arm64: insn: Reduce header dependencies of instruction decoder
The instruction encoder/decoder depends on alternative headers only for single macro definitions that could be part of the instruction decoder. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/alternative-macros.h | 3 --- arch/arm64/include/asm/insn.h | 3 ++- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h index 5df500dcc627..c01edf4d988d 100644 --- a/arch/arm64/include/asm/alternative-macros.h +++ b/arch/arm64/include/asm/alternative-macros.h @@ -6,9 +6,6 @@ #define ARM64_CB_PATCH ARM64_NCAPS -/* A64 instructions are always 32 bits. */ -#defineAARCH64_INSN_SIZE 4 - #ifndef __ASSEMBLY__ #include diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 14aa2f3aebfe..ffcdeac80026 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -10,7 +10,8 @@ #include #include -#include +/* A64 instructions are always 32 bits. */ +#define AARCH64_INSN_SIZE 4 #ifndef __ASSEMBLY__ /* -- 2.25.4
[RFC PATCH v2 5/8] arm64: insn: Add SVE instruction class
SVE has been public for some time now. Let the decoder acknowledge its existence. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/insn.h | 1 + arch/arm64/lib/insn.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index ffcdeac80026..b55b629c5eab 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -31,6 +31,7 @@ */ enum aarch64_insn_encoding_class { AARCH64_INSN_CLS_UNKNOWN, /* UNALLOCATED */ + AARCH64_INSN_CLS_SVE, /* SVE instructions */ AARCH64_INSN_CLS_DP_IMM,/* Data processing - immediate */ AARCH64_INSN_CLS_DP_REG,/* Data processing - register */ AARCH64_INSN_CLS_DP_FPSIMD, /* Data processing - SIMD and FP */ diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c index 6ff8826ae7ea..b506a4b1e38c 100644 --- a/arch/arm64/lib/insn.c +++ b/arch/arm64/lib/insn.c @@ -23,7 +23,7 @@ static const int aarch64_insn_encoding_class[] = { AARCH64_INSN_CLS_UNKNOWN, AARCH64_INSN_CLS_UNKNOWN, - AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_SVE, AARCH64_INSN_CLS_UNKNOWN, AARCH64_INSN_CLS_LDST, AARCH64_INSN_CLS_DP_REG, -- 2.25.4
[RFC PATCH v2 4/8] arm64: Move instruction encoder/decoder under lib/
Aarch64 instruction set encoding and decoding logic can prove useful for some features/tools both part of the kernel and outside the kernel. Isolate the function dealing only with encoding/decoding instructions, with minimal dependency on kernel utilities in order to be able to reuse that code. Code was only moved, no code should have been added, removed nor modifier. Signed-off-by: Julien Thierry --- arch/arm64/kernel/Makefile| 2 +- arch/arm64/lib/Makefile | 6 +++--- arch/arm64/{kernel => lib}/insn.c | 0 3 files changed, 4 insertions(+), 4 deletions(-) rename arch/arm64/{kernel => lib}/insn.c (100%) diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 790af8c69338..027f06cb75ff 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -13,7 +13,7 @@ CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE) obj-y := debug-monitors.o entry.o irq.o fpsimd.o \ entry-common.o entry-fpsimd.o process.o ptrace.o \ setup.o signal.o sys.o stacktrace.o time.o traps.o \ - io.o vdso.o hyp-stub.o psci.o cpu_ops.o insn.o \ + io.o vdso.o hyp-stub.o psci.o cpu_ops.o \ return_address.o cpuinfo.o cpu_errata.o \ cpufeature.o alternative.o cacheinfo.o \ smp.o smp_spin_table.o topology.o smccc-call.o \ diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile index d31e1169d9b8..9cd83908717d 100644 --- a/arch/arm64/lib/Makefile +++ b/arch/arm64/lib/Makefile @@ -1,9 +1,9 @@ # SPDX-License-Identifier: GPL-2.0 lib-y := clear_user.o delay.o copy_from_user.o\ copy_to_user.o copy_in_user.o copy_page.o\ - clear_page.o csum.o memchr.o memcpy.o memmove.o \ - memset.o memcmp.o strcmp.o strncmp.o strlen.o\ - strnlen.o strchr.o strrchr.o tishift.o + clear_page.o csum.o insn.o memchr.o memcpy.o \ + memmove.o memset.o memcmp.o strcmp.o strncmp.o \ + strlen.o strnlen.o strchr.o strrchr.o tishift.o ifeq ($(CONFIG_KERNEL_MODE_NEON), y) obj-$(CONFIG_XOR_BLOCKS) += xor-neon.o diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/lib/insn.c similarity index 100% rename from arch/arm64/kernel/insn.c rename to arch/arm64/lib/insn.c -- 2.25.4
[RFC PATCH v2 2/8] arm64: Move aarch32 condition check functions
The functions to check condition flags for aarch32 execution is only used to emulate aarch32 instructions. Move them from the instruction encoding/decoding code to the trap handling files. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/insn.h| 2 - arch/arm64/include/asm/probes.h | 2 +- arch/arm64/include/asm/traps.h | 3 + arch/arm64/kernel/insn.c | 98 --- arch/arm64/kernel/probes/simulate-insn.c | 1 + arch/arm64/kernel/traps.c| 99 +++- 6 files changed, 103 insertions(+), 102 deletions(-) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 7437b7e7e7eb..14aa2f3aebfe 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -499,8 +499,6 @@ u32 aarch32_insn_extract_reg_num(u32 insn, int offset); u32 aarch32_insn_mcr_extract_opc2(u32 insn); u32 aarch32_insn_mcr_extract_crm(u32 insn); -typedef bool (pstate_check_t)(unsigned long); -extern pstate_check_t * const aarch32_opcode_cond_checks[16]; #endif /* __ASSEMBLY__ */ #endif /* __ASM_INSN_H */ diff --git a/arch/arm64/include/asm/probes.h b/arch/arm64/include/asm/probes.h index 006946745352..462ca74a94ac 100644 --- a/arch/arm64/include/asm/probes.h +++ b/arch/arm64/include/asm/probes.h @@ -7,7 +7,7 @@ #ifndef _ARM_PROBES_H #define _ARM_PROBES_H -#include +#include typedef u32 probe_opcode_t; typedef void (probes_handler_t) (u32 opcode, long addr, struct pt_regs *); diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h index 54f32a0675df..6f33ff55a9f0 100644 --- a/arch/arm64/include/asm/traps.h +++ b/arch/arm64/include/asm/traps.h @@ -100,4 +100,7 @@ static inline u32 arm64_ras_serror_get_severity(u32 esr) bool arm64_is_fatal_ras_serror(struct pt_regs *regs, unsigned int esr); void __noreturn arm64_serror_panic(struct pt_regs *regs, u32 esr); + +typedef bool (pstate_check_t)(unsigned long); +extern pstate_check_t * const aarch32_opcode_cond_checks[16]; #endif diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 952e7d6fe60e..6ff8826ae7ea 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -1289,104 +1289,6 @@ u32 aarch32_insn_mcr_extract_crm(u32 insn) return insn & CRM_MASK; } -static bool __kprobes __check_eq(unsigned long pstate) -{ - return (pstate & PSR_Z_BIT) != 0; -} - -static bool __kprobes __check_ne(unsigned long pstate) -{ - return (pstate & PSR_Z_BIT) == 0; -} - -static bool __kprobes __check_cs(unsigned long pstate) -{ - return (pstate & PSR_C_BIT) != 0; -} - -static bool __kprobes __check_cc(unsigned long pstate) -{ - return (pstate & PSR_C_BIT) == 0; -} - -static bool __kprobes __check_mi(unsigned long pstate) -{ - return (pstate & PSR_N_BIT) != 0; -} - -static bool __kprobes __check_pl(unsigned long pstate) -{ - return (pstate & PSR_N_BIT) == 0; -} - -static bool __kprobes __check_vs(unsigned long pstate) -{ - return (pstate & PSR_V_BIT) != 0; -} - -static bool __kprobes __check_vc(unsigned long pstate) -{ - return (pstate & PSR_V_BIT) == 0; -} - -static bool __kprobes __check_hi(unsigned long pstate) -{ - pstate &= ~(pstate >> 1); /* PSR_C_BIT &= ~PSR_Z_BIT */ - return (pstate & PSR_C_BIT) != 0; -} - -static bool __kprobes __check_ls(unsigned long pstate) -{ - pstate &= ~(pstate >> 1); /* PSR_C_BIT &= ~PSR_Z_BIT */ - return (pstate & PSR_C_BIT) == 0; -} - -static bool __kprobes __check_ge(unsigned long pstate) -{ - pstate ^= (pstate << 3);/* PSR_N_BIT ^= PSR_V_BIT */ - return (pstate & PSR_N_BIT) == 0; -} - -static bool __kprobes __check_lt(unsigned long pstate) -{ - pstate ^= (pstate << 3);/* PSR_N_BIT ^= PSR_V_BIT */ - return (pstate & PSR_N_BIT) != 0; -} - -static bool __kprobes __check_gt(unsigned long pstate) -{ - /*PSR_N_BIT ^= PSR_V_BIT */ - unsigned long temp = pstate ^ (pstate << 3); - - temp |= (pstate << 1); /*PSR_N_BIT |= PSR_Z_BIT */ - return (temp & PSR_N_BIT) == 0; -} - -static bool __kprobes __check_le(unsigned long pstate) -{ - /*PSR_N_BIT ^= PSR_V_BIT */ - unsigned long temp = pstate ^ (pstate << 3); - - temp |= (pstate << 1); /*PSR_N_BIT |= PSR_Z_BIT */ - return (temp & PSR_N_BIT) != 0; -} - -static bool __kprobes __check_al(unsigned long pstate) -{ - return true; -} - -/* - * Note that the ARMv8 ARM calls condition code 0b "nv", but states that - * it behaves identically to 0b1110 ("al"). - */ -pstate_check_t * const aarch32_opcode_cond_checks[16] = { - __check_eq, __check_ne, __check_cs, __check_cc, - __check_mi, __check_pl, __check_vs, __check_vc, - __check_hi, __check_ls, __check_ge, __check_lt, - __chec
[RFC PATCH v2 1/8] arm64: Move patching utilities out of instruction encoding/decoding
Files insn.[c|h] containt some functions used for instruction patching. In order to reuse the instruction encoder/decoder, move the patching utilities to their own file. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/insn.h | 5 - arch/arm64/include/asm/patching.h | 13 +++ arch/arm64/kernel/Makefile| 2 +- arch/arm64/kernel/ftrace.c| 1 + arch/arm64/kernel/insn.c | 149 +- arch/arm64/kernel/jump_label.c| 1 + arch/arm64/kernel/patching.c | 148 + arch/arm64/kernel/traps.c | 1 + 8 files changed, 168 insertions(+), 152 deletions(-) create mode 100644 arch/arm64/include/asm/patching.h create mode 100644 arch/arm64/kernel/patching.c diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 4ebb9c054ccc..7437b7e7e7eb 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -379,8 +379,6 @@ static inline bool aarch64_insn_is_adr_adrp(u32 insn) return aarch64_insn_is_adr(insn) || aarch64_insn_is_adrp(insn); } -int aarch64_insn_read(void *addr, u32 *insnp); -int aarch64_insn_write(void *addr, u32 insn); enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); bool aarch64_insn_uses_literal(u32 insn); bool aarch64_insn_is_branch(u32 insn); @@ -487,9 +485,6 @@ u32 aarch64_insn_gen_prefetch(enum aarch64_insn_register base, s32 aarch64_get_branch_offset(u32 insn); u32 aarch64_set_branch_offset(u32 insn, s32 offset); -int aarch64_insn_patch_text_nosync(void *addr, u32 insn); -int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt); - s32 aarch64_insn_adrp_get_offset(u32 insn); u32 aarch64_insn_adrp_set_offset(u32 insn, s32 offset); diff --git a/arch/arm64/include/asm/patching.h b/arch/arm64/include/asm/patching.h new file mode 100644 index ..6bf5adc56295 --- /dev/null +++ b/arch/arm64/include/asm/patching.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef__ASM_PATCHING_H +#define__ASM_PATCHING_H + +#include + +int aarch64_insn_read(void *addr, u32 *insnp); +int aarch64_insn_write(void *addr, u32 insn); + +int aarch64_insn_patch_text_nosync(void *addr, u32 insn); +int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt); + +#endif /* __ASM_PATCHING_H */ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index ed65576ce710..790af8c69338 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -17,7 +17,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \ return_address.o cpuinfo.o cpu_errata.o \ cpufeature.o alternative.o cacheinfo.o \ smp.o smp_spin_table.o topology.o smccc-call.o \ - syscall.o proton-pack.o idreg-override.o + syscall.o proton-pack.o idreg-override.o patching.o targets+= efi-entry.o diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index 86a5cf9bc19a..fd7993f0c9c4 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -15,6 +15,7 @@ #include #include #include +#include #ifdef CONFIG_DYNAMIC_FTRACE /* diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 6c0de2f60ea9..952e7d6fe60e 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -7,21 +7,14 @@ */ #include #include -#include -#include -#include -#include -#include -#include +#include +#include #include -#include -#include #include -#include +#include #include #include -#include #define AARCH64_INSN_SF_BITBIT(31) #define AARCH64_INSN_N_BIT BIT(22) @@ -83,81 +76,6 @@ bool aarch64_insn_is_branch_imm(u32 insn) aarch64_insn_is_bcond(insn)); } -static DEFINE_RAW_SPINLOCK(patch_lock); - -static bool is_exit_text(unsigned long addr) -{ - /* discarded with init text/data */ - return system_state < SYSTEM_RUNNING && - addr >= (unsigned long)__exittext_begin && - addr < (unsigned long)__exittext_end; -} - -static bool is_image_text(unsigned long addr) -{ - return core_kernel_text(addr) || is_exit_text(addr); -} - -static void __kprobes *patch_map(void *addr, int fixmap) -{ - unsigned long uintaddr = (uintptr_t) addr; - bool image = is_image_text(uintaddr); - struct page *page; - - if (image) - page = phys_to_page(__pa_symbol(addr)); - else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) - page = vmalloc_to_page(addr); - else - return addr; - - BUG_ON(!page); - return (void *)set_fixmap_offset(fixmap, page_to_phys(page) + - (uintaddr & ~PAGE_MASK)); -} - -static void __kprobes patch_unmap(int fixmap) -
[RFC PATCH v2 0/8] arm64: Prepare instruction decoder for objtool
To support arm64, objtool will need to be able to decode aarch64 instructions. This patch series adds some instruction definitions needed by objtool and moves out encoding/decoding functionalities that do not rely on kernel code in order. Changes since v1[1]: - Split the isolation of instruction encoder/decoder capabilities in several steps as suggested by Mark R. - Exclude dsb encoding where CRm != 0b0x00 - Support dsb FEAT_XS encoding - Support previously missing barriers [1] https://lkml.org/lkml/2021/1/20/791 Thanks, Julien --> Julien Thierry (8): arm64: Move patching utilities out of instruction encoding/decoding arm64: Move aarch32 condition check functions arm64: insn: Reduce header dependencies of instruction decoder arm64: Move instruction encoder/decoder under lib/ arm64: insn: Add SVE instruction class arm64: insn: Add barrier encodings arm64: insn: Add some opcodes to instruction decoder arm64: insn: Add load/store decoding helpers arch/arm64/include/asm/alternative-macros.h | 3 - arch/arm64/include/asm/insn.h | 70 +- arch/arm64/include/asm/patching.h | 13 + arch/arm64/include/asm/probes.h | 2 +- arch/arm64/include/asm/traps.h | 3 + arch/arm64/kernel/Makefile | 4 +- arch/arm64/kernel/ftrace.c | 1 + arch/arm64/kernel/jump_label.c | 1 + arch/arm64/kernel/patching.c| 148 arch/arm64/kernel/probes/simulate-insn.c| 1 + arch/arm64/kernel/traps.c | 100 +++- arch/arm64/lib/Makefile | 6 +- arch/arm64/{kernel => lib}/insn.c | 249 +--- 13 files changed, 338 insertions(+), 263 deletions(-) create mode 100644 arch/arm64/include/asm/patching.h create mode 100644 arch/arm64/kernel/patching.c rename arch/arm64/{kernel => lib}/insn.c (86%) -- 2.25.4
Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/
On 2/3/21 12:12 PM, Mark Rutland wrote: On Wed, Feb 03, 2021 at 09:26:45AM +0100, Julien Thierry wrote: On 2/2/21 11:17 AM, Mark Rutland wrote: On Wed, Jan 20, 2021 at 06:17:41PM +0100, Julien Thierry wrote: Aarch64 instruction set encoding and decoding logic can prove useful for some features/tools both part of the kernel and outside the kernel. Isolate the function dealing only with encoding/decoding instructions, with minimal dependency on kernel utilities in order to be able to reuse that code. Code was only moved, no code should have been added, removed nor modifier. Signed-off-by: Julien Thierry This looks sound, but the diff is a little hard to check. Yes, I was expecting this change to be hard to digest. Would it be possible to split this into two patches, where: 1) Refactoring definitions into insn.h and insn.c, leaving those files in their current locations. I'm not quite sure I understand the suggestions. After this patch insn.h and insn.c still contain some definitions that can't really be used outside of kernel code which is why I split them into insn.* and aarch64-insn.* (the latter containing what could be used by tools). Sorry; I hadn't appreciated what was getting left behind. With the series applied I see that's some kernel patching logic, and AArch32 insn bits. How about we invert the move, and split those bits out of insn.c first, then move the rest in one go, i.e. 1) Factor the patching bits out into new patching.{c,h} files. 2) Move insn.c to arch/arm64/lib/ 3) Split insn.* into aarch64-insn.* and aarch32-insn.* ... if that makes any sense? We might not even need to split the aarch32 bits out given they all have an aarch32_* prefix. Yes, that approach sounds good. I'll if the aarchxx-insn is necessary but as you say, all in the same file shouldn't cause trouble. Thanks, -- Julien Thierry
Re: [RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings
On 2/2/21 12:15 PM, Mark Rutland wrote: On Wed, Jan 20, 2021 at 06:17:43PM +0100, Julien Thierry wrote: Create necessary functions to encode/decode aarch64 data/instruction barriers. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/aarch64-insn.h | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h index 200bee726172..d0fee47bbe6e 100644 --- a/arch/arm64/include/asm/aarch64-insn.h +++ b/arch/arm64/include/asm/aarch64-insn.h @@ -379,6 +379,9 @@ __AARCH64_INSN_FUNCS(eret_auth, 0xFBFF, 0xD69F0BFF) __AARCH64_INSN_FUNCS(mrs, 0xFFF0, 0xD530) __AARCH64_INSN_FUNCS(msr_imm, 0xFFF8F01F, 0xD500401F) __AARCH64_INSN_FUNCS(msr_reg, 0xFFF0, 0xD510) +__AARCH64_INSN_FUNCS(dmb, 0xF0FF, 0xD50330BF) +__AARCH64_INSN_FUNCS(dsb, 0xF0FF, 0xD503309F) +__AARCH64_INSN_FUNCS(isb, 0xF0FF, 0xD50330DF) These match the encodings in ARM DDI 0487G.a, with a couple of caveats for DSB. Per section C6.2.82 on page C6-1000, when CRm != 0x00, the instruction isn't considered a DSB. I believe per the "barriers" decode table on page C4-289 that here "0x00" is actually a binary string and 'x' is a "don't care" -- I've raised a ticket to get the documentation clarified. I suspect we need to write a function to handle that. Ah, I did miss that part. Thanks for pointing it out (and for clarifying it's probably not hexa, but the binary string makes sense since it's a 4 bits field) There's also a secondary encoding for DSB with FEAT_XS, which we don't currently use but might want to add. Ah, yes, had to pick up a newer version of the Arm ARM! I'll add it. #undef__AARCH64_INSN_FUNCS @@ -390,6 +393,12 @@ static inline bool aarch64_insn_is_adr_adrp(u32 insn) return aarch64_insn_is_adr(insn) || aarch64_insn_is_adrp(insn); } +static inline bool aarch64_insn_is_barrier(u32 insn) +{ + return aarch64_insn_is_dmb(insn) || aarch64_insn_is_dsb(insn) || + aarch64_insn_is_isb(insn); +} I assume this is meant to match the barriers instruction class, as per the table on page C4-289? That also contains CLREX, SB, SSBB, and PSSBB, and it might be worth adding them at the same time. Yes, I have to admit I only added the ones that objtool saw and complained about "unreachable instruction" (mostly barriers after ret/eret). I'll add them as well. Thanks, -- Julien Thierry
Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/
Hi Mark, On 2/2/21 11:17 AM, Mark Rutland wrote: Hi Julien, On Wed, Jan 20, 2021 at 06:17:41PM +0100, Julien Thierry wrote: Aarch64 instruction set encoding and decoding logic can prove useful for some features/tools both part of the kernel and outside the kernel. Isolate the function dealing only with encoding/decoding instructions, with minimal dependency on kernel utilities in order to be able to reuse that code. Code was only moved, no code should have been added, removed nor modifier. Signed-off-by: Julien Thierry This looks sound, but the diff is a little hard to check. Yes, I was expecting this change to be hard to digest. Would it be possible to split this into two patches, where: 1) Refactoring definitions into insn.h and insn.c, leaving those files in their current locations. I'm not quite sure I understand the suggestions. After this patch insn.h and insn.c still contain some definitions that can't really be used outside of kernel code which is why I split them into insn.* and aarch64-insn.* (the latter containing what could be used by tools). Or do you suggest that I still create the aarch64-insn.* files next to the insn.* files? 2) Moving insn.h and insn.c out to arch/arm64/lib/, updating includes > With that, the second patch might be easier for git to identify as a rename (if using `git format-patch -M -C`), and it'd be easier to see that there are no unintended changes. But it is more a split than a rename (at least in this patch). But I'm happy to do this in another way. Thanks, -- Julien Thierry
Re: [RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64
On 2/3/21 12:01 AM, Nick Desaulniers wrote: On Tue, Feb 2, 2021 at 12:57 AM Julien Thierry wrote: On 2/2/21 12:17 AM, Nick Desaulniers wrote: On Mon, Feb 1, 2021 at 1:44 PM Josh Poimboeuf 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 wrote: On Wed, Jan 27, 2021 at 02:15:57PM -0800, Nick Desaulniers wrote: From: Raphael Gault 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 [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 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.) The main issue is that the tables for arm64 have more indirection than x86. I wonder if PAC or BTI also make this slightly more complex? PAC at least has implications for unwinders, IIUC. On x86, the dispatching jump instruction fetches the target address from a contiguous array of addresses based on a given offset. So the list of potential targets of the jump is neatly organized in a table (and sure, before link time these are just relocation, but still processable). On arm64 (with GCC at least), what is stored in a table is an array of candidate offsets from the jump instruction. And because arm64 is limited to 32bit instructions, the encoding often requires multiple instructions to compute the target address: ldr<*> x_offset, [x_offsets_table, x_index, ...] // load offset adr x_dest_base, // load target branch for offset 0 add x_dest, x_target_base, x_offset, ... // compute final address br x_dest// jump Where this gets trickier is that (with GCC) the offsets stored in the table might or might not be signed constants (and this can be seen in GCC intermediate representations, but I do not believe this information is output in the final object file). And on top of that, GCC might decide to use offsets that are seen as unsigned during intermediate representation as signed offset by sign extending them in the add instruction. So, to handle this we'd have to track the different operation done with the offset, from the load to the final jump, decoding the instructions and deducing the potential target instructions from the table of offsets. But that is error prone as we don't really know how many instructions can be between the ones doing the address computation, and I remember some messy case of a jump table inside a jump table where tracking the instruction touching one or the other offset would need a lot of corner case handling. And this of course is just for GCC, I haven't looked at what it all looks like on Clang's end. Sure, but this is what production unwinders do, and they don't require compiler plugins, right? I don't doubt unwinders can be made simpler with changes to toolchain output; please work with your compiler vendor on making such changes rather than relying on compiler plugins to do so. I think there is a small confusion. The plugin nor the data it generates is not to be used by a kernel unwinder. It's here to allow objtool to assess whether the code being checked can be unwound (?) reliably (not omitting functions). Part of this is checking that a branch/jump in a function does not end up in some code that is not related to the function without setting up a call frame. This is about static validation rather than functionality. 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. Sorry, I will need to make that clearer. The next patch explains it a bit [1] Basically, for simplicity, the plugin creates a new section containing Right, this takes a focus on simplicity, at the cost of alienating a toolchain. Ard'
Re: [RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64
On 2/2/21 12:17 AM, Nick Desaulniers wrote: On Mon, Feb 1, 2021 at 1:44 PM Josh Poimboeuf 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 wrote: On Wed, Jan 27, 2021 at 02:15:57PM -0800, Nick Desaulniers wrote: From: Raphael Gault 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 [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 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.) The main issue is that the tables for arm64 have more indirection than x86. On x86, the dispatching jump instruction fetches the target address from a contiguous array of addresses based on a given offset. So the list of potential targets of the jump is neatly organized in a table (and sure, before link time these are just relocation, but still processable). On arm64 (with GCC at least), what is stored in a table is an array of candidate offsets from the jump instruction. And because arm64 is limited to 32bit instructions, the encoding often requires multiple instructions to compute the target address: ldr<*> x_offset, [x_offsets_table, x_index, ...] // load offset adr x_dest_base, // load target branch for offset 0 add x_dest, x_target_base, x_offset, ... // compute final address br x_dest// jump Where this gets trickier is that (with GCC) the offsets stored in the table might or might not be signed constants (and this can be seen in GCC intermediate representations, but I do not believe this information is output in the final object file). And on top of that, GCC might decide to use offsets that are seen as unsigned during intermediate representation as signed offset by sign extending them in the add instruction. So, to handle this we'd have to track the different operation done with the offset, from the load to the final jump, decoding the instructions and deducing the potential target instructions from the table of offsets. But that is error prone as we don't really know how many instructions can be between the ones doing the address computation, and I remember some messy case of a jump table inside a jump table where tracking the instruction touching one or the other offset would need a lot of corner case handling. And this of course is just for GCC, I haven't looked at what it all looks like on Clang's end. 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. Sorry, I will need to make that clearer. The next patch explains it a bit [1] Basically, for simplicity, the plugin creates a new section containing tables (one per jump table) of references to the jump targets, similar to what x86 has, except that in this case this table isn't actually used by runtime code and is discarded at link time. I only chose this to minimize what needed to be changed in objtool and because the format seemed simple enough. But I'm open on some alternative, whether it's a -fjump-table-info option added to compilers with a different format to do the links. The important requirement is to be able to know all the candidate targets for a "br " instruction. [1] https://lkml.org/lkml/2021/1/20/910 Thanks, -- Julien Thierry
Re: [RFC PATCH 00/17] objtool: add base support for arm64
On 1/21/21 12:08 PM, Ard Biesheuvel wrote: On Thu, 21 Jan 2021 at 11:26, Julien Thierry wrote: Hi Ard, On 1/21/21 10:03 AM, Ard Biesheuvel wrote: Hello Julien, On Wed, 20 Jan 2021 at 18:38, Julien Thierry 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. 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. That means that every actor maintaining their own stable version of the kernel have to do their own audit when they do backports (assuming the audit would be done for upstream) to be able to provide a safe livepatching feature in their kernel. 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. I was not familiar with the shadow call stack. If I understand correctly that would be a stack of return addresses of function currently on the call stack, is that correct? That would indeed be a simpler approach, however I guess the instrumentation has a cost. Is the instrumentation also available with GCC? And is this instrumentation efficient enough to be suitable for production builds? I am not aware of any plans to enable this in GCC, but the Clang implementation is definitely intended for production use (it's a CFI feature for ROP/JOP mitigation) 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). I don't know how feasible it will be to get it into GCC if people decide to go with that. Also, now that I think about it, it will probably come with similar limitations as stackframes where the unwinder would need to know when/where the shadow call stack is unavailable for some reason and the stack trace is not reliable. (it might be a bit simpler to audit than stack frame setting and maybe have less limitations, but I guess there will still be cases where we can't rely on it) -- Julien Thierry
Re: [RFC PATCH 00/17] objtool: add base support for arm64
Hi Ard, On 1/21/21 10:03 AM, Ard Biesheuvel wrote: Hello Julien, On Wed, 20 Jan 2021 at 18:38, Julien Thierry 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. I'm not familiar with toolcahin code models, but would this approach be able to validate assembly code (either inline or in assembly files?) 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. I was not familiar with the shadow call stack. If I understand correctly that would be a stack of return addresses of function currently on the call stack, is that correct? That would indeed be a simpler approach, however I guess the instrumentation has a cost. Is the instrumentation also available with GCC? And is this instrumentation efficient enough to be suitable for production builds? If we can rely on shadow call stack to implement the reliable unwinder, I guess this could be the way to go. 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) I understand the concern and appreciate the suggestion. I guess this does need some thorough discussions for the right approach. Thanks, -- Julien Thierry
[RFC PATCH 15/17] objtool: arm64: Handle supported relocations in alternatives
Based on get_alt_insn() in arch/arm64/kernel/alternative.c, arm64 alternative code adapts offsets for static branches and adrp instructions. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/special.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/objtool/arch/arm64/special.c b/tools/objtool/arch/arm64/special.c index c9c3e0bfd581..d47e5590ed60 100644 --- a/tools/objtool/arch/arm64/special.c +++ b/tools/objtool/arch/arm64/special.c @@ -30,7 +30,11 @@ bool arch_support_alt_relocation(struct special_alt *special_alt, struct instruction *insn, struct reloc *reloc) { - return false; + u32 opcode = *(u32 *)(insn->sec->data->d_buf + insn->offset); + + return aarch64_insn_is_branch_imm(opcode) || + aarch64_insn_is_adrp(opcode) || + !aarch64_insn_uses_literal(opcode); } static struct section *get_switch_table_info_section(struct objtool_file *file) -- 2.25.4
[RFC PATCH 09/17] objtool: arm64: Decode LDR instructions
Load literal instructions can generate constants inside code sections. Record the locations of the constants in order to be able to remove their corresponding "struct instruction". Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c| 86 tools/objtool/arch/x86/decode.c | 5 ++ tools/objtool/check.c| 3 + tools/objtool/include/objtool/arch.h | 3 + 4 files changed, 97 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 1087ede67bcd..b4d4d5b051b0 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -30,6 +30,73 @@ static unsigned long sign_extend(unsigned long x, int nbits) return ((~0UL + (sign_bit ^ 1)) << nbits) | x; } +struct insn_loc { + const struct section *sec; + unsigned long offset; + struct hlist_node hnode; + bool ignorable; +}; + +DEFINE_HASHTABLE(invalid_insns, 16); + +static int record_invalid_insn(const struct section *sec, + unsigned long offset, + bool ignore) +{ + struct insn_loc *loc; + struct hlist_head *l; + + l = &invalid_insns[hash_min(offset, HASH_BITS(invalid_insns))]; + if (!hlist_empty(l)) { + loc = hlist_entry(l->first, struct insn_loc, hnode); + loc->ignorable |= ignore; + return 0; + } + + loc = malloc(sizeof(*loc)); + if (!loc) { + WARN("malloc failed"); + return -1; + } + + loc->sec = sec; + loc->offset = offset; + loc->ignorable = ignore; + + hash_add(invalid_insns, &loc->hnode, loc->offset); + + return 0; +} + +int arch_post_process_instructions(struct objtool_file *file) +{ + struct hlist_node *tmp; + struct insn_loc *loc; + unsigned int bkt; + int res = 0; + + hash_for_each_safe(invalid_insns, bkt, tmp, loc, hnode) { + struct instruction *insn; + + insn = find_insn(file, (struct section *) loc->sec, loc->offset); + if (insn) { + if (loc->ignorable) { + list_del(&insn->list); + hash_del(&insn->hash); + free(insn); + } else { + WARN_FUNC("can't decode instruction", insn->sec, insn->offset); + return -1; + } + } + + hash_del(&loc->hnode); + free(loc); + } + + return res; +} + bool arch_callee_saved_reg(unsigned char reg) { switch (reg) { @@ -359,6 +426,25 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case AARCH64_INSN_CLS_LDST: if (arm_decode_load_store(insn, type, immediate, ops_list)) break; + if (aarch64_insn_is_ldr_lit(insn)) { + long pc_offset; + + pc_offset = insn & GENMASK(23, 5); + /* Sign extend and multiply by 4 */ + pc_offset = (pc_offset << (64 - 23)); + pc_offset = ((pc_offset >> (64 - 23)) >> 5) << 2; + + if (record_invalid_insn(sec, offset + pc_offset, true)) + return -1; + + /* 64-bit literal */ + if (insn & BIT(30)) { + if (record_invalid_insn(sec, + offset + pc_offset + 4, + true)) + return -1; + } + } *type = INSN_OTHER; break; default: diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 6baa22732ca6..e76d987ce3c7 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -549,6 +549,11 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, return 0; } +int arch_post_process_instructions(struct objtool_file *file) +{ + return 0; +} + void arch_initial_func_cfi_state(struct cfi_init_state *state) { int i; diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 270b507e7098..d902697a388e 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -319,6 +319,9 @@ static int decode_instructions(struct objtool_file *file) if (stats) printf("nr_insns: %lu\n", nr_insns); + if (arch_post_process_instructions(file)) + return -1; + return 0;
[RFC PATCH 00/17] objtool: add base support for arm64
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. 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
[RFC PATCH 01/17] tools: Add some generic functions and headers
These will be needed to be able to use arm64 instruction decoder in userland tools. Signed-off-by: Julien Thierry --- tools/include/asm-generic/bitops/__ffs.h | 11 +++ tools/include/linux/kernel.h | 21 + tools/include/linux/printk.h | 40 3 files changed, 72 insertions(+) create mode 100644 tools/include/linux/printk.h diff --git a/tools/include/asm-generic/bitops/__ffs.h b/tools/include/asm-generic/bitops/__ffs.h index 9d1310519497..963f8a22212f 100644 --- a/tools/include/asm-generic/bitops/__ffs.h +++ b/tools/include/asm-generic/bitops/__ffs.h @@ -42,4 +42,15 @@ static __always_inline unsigned long __ffs(unsigned long word) return num; } +static inline unsigned long __ffs64(u64 word) +{ +#if BITS_PER_LONG == 32 + if (((u32)word) == 0UL) + return __ffs((u32)(word >> 32)) + 32; +#elif BITS_PER_LONG != 64 +#error BITS_PER_LONG not 32 or 64 +#endif + return __ffs((unsigned long)word); +} + #endif /* _TOOLS_LINUX_ASM_GENERIC_BITOPS___FFS_H_ */ diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h index a7e54a08fb54..e748982ed5c1 100644 --- a/tools/include/linux/kernel.h +++ b/tools/include/linux/kernel.h @@ -114,6 +114,27 @@ int scnprintf_pad(char * buf, size_t size, const char * fmt, ...); #define round_up(x, y) x)-1) | __round_mask(x, y))+1) #define round_down(x, y) ((x) & ~__round_mask(x, y)) +/** + * upper_32_bits - return bits 32-63 of a number + * @n: the number we're accessing + * + * A basic shift-right of a 64- or 32-bit quantity. Use this to suppress + * the "right shift count >= width of type" warning when that quantity is + * 32-bits. + */ +#define upper_32_bits(n) ((u32)(((n) >> 16) >> 16)) + +/** + * lower_32_bits - return bits 0-31 of a number + * @n: the number we're accessing + */ +#define lower_32_bits(n) ((u32)(n)) + +/* Inspired from ALIGN_*_KERNEL */ +#define __ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask)) +#define __ALIGN(x, a) __ALIGN_MASK(x, (typeof(x))(a) - 1) +#define ALIGN_DOWN(x, a) __ALIGN((x) - ((a) - 1), (a)) + #define current_gfp_context(k) 0 #define synchronize_rcu() diff --git a/tools/include/linux/printk.h b/tools/include/linux/printk.h new file mode 100644 index ..515ebdc47e6e --- /dev/null +++ b/tools/include/linux/printk.h @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_LINUX_KERNEL_PRINTK_H_ +#define _TOOLS_LINUX_KERNEL_PRINTK_H_ + +#include +#include +#include + +#define printk(fmt, ...) fprintf(stdout, fmt, ##__VA_ARGS__) +#define pr_info printk +#define pr_notice printk +#define pr_cont printk + +#define pr_warn(fmt, ...) fprintf(stderr, fmt, ##__VA_ARGS__) +#define pr_err pr_warn +#define pr_alert pr_warn +#define pr_emerg pr_warn +#define pr_crit pr_warn + +/* + * Dummy printk for disabled debugging statements to use whilst maintaining + * gcc's format checking. + */ +#define no_printk(fmt, ...)\ +({ \ + if (0) \ + printk(fmt, ##__VA_ARGS__); \ + 0; \ +}) + +/* pr_devel() should produce zero code unless DEBUG is defined */ +#ifdef DEBUG +#define pr_devel(fmt, ...) printk +#else +#define pr_devel(fmt, ...) no_printk +#endif + +#define pr_debug pr_devel + +#endif /* _TOOLS_LINUX_KERNEL_PRINTK_H_ */ -- 2.25.4
[RFC PATCH 07/17] objtool: arm64: Decode other system instructions
Decode ERET, BRK and NOPs Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 924121b4b466..a4a587c400a1 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -223,6 +223,13 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, /* Remaining branch opcodes are conditional */ *type = INSN_JUMP_CONDITIONAL; *immediate = aarch64_get_branch_offset(insn); + } else if (aarch64_insn_is_eret(insn)) { + *type = INSN_CONTEXT_SWITCH; + } else if (aarch64_insn_is_steppable_hint(insn)) { + *type = INSN_NOP; + } else if (aarch64_insn_is_brk(insn)) { + *immediate = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_16, insn); + *type = INSN_BUG; } else { *type = INSN_OTHER; } -- 2.25.4
[RFC PATCH 05/17] objtool: arm64: Decode add/sub instructions
Decode aarch64 additions and substractions and create stack_ops for instructions interacting with SP or FP. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 84 +++ 1 file changed, 84 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 8ae822f553ca..0f312dd1b146 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -23,6 +23,13 @@ #include "../../../arch/arm64/lib/aarch64-insn.c" +static unsigned long sign_extend(unsigned long x, int nbits) +{ + unsigned long sign_bit = (x >> (nbits - 1)) & 1; + + return ((~0UL + (sign_bit ^ 1)) << nbits) | x; +} + bool arch_callee_saved_reg(unsigned char reg) { switch (reg) { @@ -98,6 +105,53 @@ int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg) return -1; } +static struct stack_op *arm_make_add_op(enum aarch64_insn_register dest, + enum aarch64_insn_register src, + int val) +{ + struct stack_op *op; + + op = calloc(1, sizeof(*op)); + op->dest.type = OP_DEST_REG; + op->dest.reg = dest; + op->src.reg = src; + op->src.type = val != 0 ? OP_SRC_ADD : OP_SRC_REG; + op->src.offset = val; + + return op; +} + +static void arm_decode_add_sub_imm(u32 instr, bool set_flags, + enum insn_type *type, + unsigned long *immediate, + struct list_head *ops_list) +{ + u32 rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, instr); + u32 rn = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, instr); + + *type = INSN_OTHER; + *immediate = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_12, instr); + + if (instr & AARCH64_INSN_LSL_12) + *immediate <<= 12; + + if ((!set_flags && rd == AARCH64_INSN_REG_SP) || + rd == AARCH64_INSN_REG_FP || + rn == AARCH64_INSN_REG_FP || + rn == AARCH64_INSN_REG_SP) { + struct stack_op *op; + int value; + + if (aarch64_insn_is_subs_imm(instr) || aarch64_insn_is_sub_imm(instr)) + value = -*immediate; + else + value = *immediate; + + op = arm_make_add_op(rd, rn, value); + list_add_tail(&op->list, ops_list); + } +} + int arch_decode_instruction(const struct elf *elf, const struct section *sec, unsigned long offset, unsigned int maxlen, unsigned int *len, enum insn_type *type, @@ -121,6 +175,36 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, case AARCH64_INSN_CLS_UNKNOWN: WARN("can't decode instruction at %s:0x%lx", sec->name, offset); return -1; + case AARCH64_INSN_CLS_DP_IMM: + /* Mov register to and from SP are aliases of add_imm */ + if (aarch64_insn_is_add_imm(insn) || + aarch64_insn_is_sub_imm(insn)) + arm_decode_add_sub_imm(insn, false, type, immediate, + ops_list); + else if (aarch64_insn_is_adds_imm(insn) || +aarch64_insn_is_subs_imm(insn)) + arm_decode_add_sub_imm(insn, true, type, immediate, + ops_list); + else + *type = INSN_OTHER; + break; + case AARCH64_INSN_CLS_DP_REG: + if (aarch64_insn_is_mov_reg(insn)) { + enum aarch64_insn_register rd; + enum aarch64_insn_register rm; + + rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, insn); + rm = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RM, insn); + if (rd == AARCH64_INSN_REG_FP || rm == AARCH64_INSN_REG_FP) { + struct stack_op *op; + + op = arm_make_add_op(rd, rm, 0); + list_add_tail(&op->list, ops_list); + break; + } + } + *type = INSN_OTHER; + break; default: *type = INSN_OTHER; break; -- 2.25.4
[RFC PATCH 03/17] tools: bug: Remove duplicate definition
Under tools, bug.h only defines BUILD_BUG_ON_ZERO() which is already defined in build_bug.h. This prevents a file to include both headers at the same time. Have bug.h include build_bug.h instead. Signed-off-by: Julien Thierry --- tools/include/linux/bug.h | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/include/linux/bug.h b/tools/include/linux/bug.h index 85f80258a15f..548be7cffa8e 100644 --- a/tools/include/linux/bug.h +++ b/tools/include/linux/bug.h @@ -2,10 +2,6 @@ #ifndef _TOOLS_PERF_LINUX_BUG_H #define _TOOLS_PERF_LINUX_BUG_H -/* Force a compilation error if condition is true, but also produce a - result (of value 0 and type size_t), so the expression can be used - e.g. in a structure initializer (or where-ever else comma expressions - aren't permitted). */ -#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); })) +#include #endif /* _TOOLS_PERF_LINUX_BUG_H */ -- 2.25.4
[RFC PATCH 06/17] objtool: arm64: Decode jump and call related instructions
Decode branch, branch and link (aarch64's call) and return instructions. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index 0f312dd1b146..924121b4b466 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -205,6 +205,28 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, } *type = INSN_OTHER; break; + case AARCH64_INSN_CLS_BR_SYS: + if (aarch64_insn_is_ret(insn) && + aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, insn) == AARCH64_INSN_REG_LR) { + *type = INSN_RETURN; + } else if (aarch64_insn_is_bl(insn)) { + *type = INSN_CALL; + *immediate = aarch64_get_branch_offset(insn); + } else if (aarch64_insn_is_blr(insn)) { + *type = INSN_CALL_DYNAMIC; + } else if (aarch64_insn_is_b(insn)) { + *type = INSN_JUMP_UNCONDITIONAL; + *immediate = aarch64_get_branch_offset(insn); + } else if (aarch64_insn_is_br(insn)) { + *type = INSN_JUMP_DYNAMIC; + } else if (aarch64_insn_is_branch_imm(insn)) { + /* Remaining branch opcodes are conditional */ + *type = INSN_JUMP_CONDITIONAL; + *immediate = aarch64_get_branch_offset(insn); + } else { + *type = INSN_OTHER; + } + break; default: *type = INSN_OTHER; break; -- 2.25.4
[RFC PATCH 02/17] tools: arm64: Make aarch64 instruction decoder available to tools
Add aarch64 encoder/decoder implementation under tools/ as well as the necessary arm64 headers. Signed-off-by: Julien Thierry --- tools/arch/arm64/include/asm/aarch64-insn.h | 551 +++ tools/arch/arm64/lib/aarch64-insn.c | 1425 +++ 2 files changed, 1976 insertions(+) create mode 100644 tools/arch/arm64/include/asm/aarch64-insn.h create mode 100644 tools/arch/arm64/lib/aarch64-insn.c diff --git a/tools/arch/arm64/include/asm/aarch64-insn.h b/tools/arch/arm64/include/asm/aarch64-insn.h new file mode 100644 index ..b202d6e2e47e --- /dev/null +++ b/tools/arch/arm64/include/asm/aarch64-insn.h @@ -0,0 +1,551 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef__ASM_AARCH64_INSN_H +#define__ASM_AARCH64_INSN_H + +#include +#include + + +/* A64 instructions are always 32 bits. */ +#defineAARCH64_INSN_SIZE 4 + +/* + * BRK instruction encoding + * The #imm16 value should be placed at bits[20:5] within BRK ins + */ +#define AARCH64_BREAK_MON 0xd420 + +/* + * BRK instruction for provoking a fault on purpose + * Unlike kgdb, #imm16 value with unallocated handler is used for faulting. + */ +#define AARCH64_BREAK_FAULT(AARCH64_BREAK_MON | (FAULT_BRK_IMM << 5)) + +#ifndef __ASSEMBLY__ +/* + * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a + * Section C3.1 "A64 instruction index by encoding": + * AArch64 main encoding table + * Bit position + * 28 27 26 25 Encoding Group + * 0 0 - -Unallocated + * 1 0 0 -Data processing, immediate + * 1 0 1 -Branch, exception generation and system instructions + * - 1 - 0Loads and stores + * - 1 0 1Data processing - register + * 0 1 1 1Data processing - SIMD and floating point + * 1 1 1 1Data processing - SIMD and floating point + * "-" means "don't care" + */ +enum aarch64_insn_encoding_class { + AARCH64_INSN_CLS_UNKNOWN, /* UNALLOCATED */ + AARCH64_INSN_CLS_SVE, /* SVE instructions */ + AARCH64_INSN_CLS_DP_IMM,/* Data processing - immediate */ + AARCH64_INSN_CLS_DP_REG,/* Data processing - register */ + AARCH64_INSN_CLS_DP_FPSIMD, /* Data processing - SIMD and FP */ + AARCH64_INSN_CLS_LDST, /* Loads and stores */ + AARCH64_INSN_CLS_BR_SYS,/* Branch, exception generation and +* system instructions */ +}; + +enum aarch64_insn_hint_cr_op { + AARCH64_INSN_HINT_NOP = 0x0 << 5, + AARCH64_INSN_HINT_YIELD = 0x1 << 5, + AARCH64_INSN_HINT_WFE = 0x2 << 5, + AARCH64_INSN_HINT_WFI = 0x3 << 5, + AARCH64_INSN_HINT_SEV = 0x4 << 5, + AARCH64_INSN_HINT_SEVL = 0x5 << 5, + + AARCH64_INSN_HINT_XPACLRI= 0x07 << 5, + AARCH64_INSN_HINT_PACIA_1716 = 0x08 << 5, + AARCH64_INSN_HINT_PACIB_1716 = 0x0A << 5, + AARCH64_INSN_HINT_AUTIA_1716 = 0x0C << 5, + AARCH64_INSN_HINT_AUTIB_1716 = 0x0E << 5, + AARCH64_INSN_HINT_PACIAZ = 0x18 << 5, + AARCH64_INSN_HINT_PACIASP= 0x19 << 5, + AARCH64_INSN_HINT_PACIBZ = 0x1A << 5, + AARCH64_INSN_HINT_PACIBSP= 0x1B << 5, + AARCH64_INSN_HINT_AUTIAZ = 0x1C << 5, + AARCH64_INSN_HINT_AUTIASP= 0x1D << 5, + AARCH64_INSN_HINT_AUTIBZ = 0x1E << 5, + AARCH64_INSN_HINT_AUTIBSP= 0x1F << 5, + + AARCH64_INSN_HINT_ESB = 0x10 << 5, + AARCH64_INSN_HINT_PSB = 0x11 << 5, + AARCH64_INSN_HINT_TSB = 0x12 << 5, + AARCH64_INSN_HINT_CSDB = 0x14 << 5, + + AARCH64_INSN_HINT_BTI = 0x20 << 5, + AARCH64_INSN_HINT_BTIC = 0x22 << 5, + AARCH64_INSN_HINT_BTIJ = 0x24 << 5, + AARCH64_INSN_HINT_BTIJC = 0x26 << 5, +}; + +enum aarch64_insn_imm_type { + AARCH64_INSN_IMM_ADR, + AARCH64_INSN_IMM_26, + AARCH64_INSN_IMM_19, + AARCH64_INSN_IMM_16, + AARCH64_INSN_IMM_14, + AARCH64_INSN_IMM_12, + AARCH64_INSN_IMM_9, + AARCH64_INSN_IMM_7, + AARCH64_INSN_IMM_6, + AARCH64_INSN_IMM_S, + AARCH64_INSN_IMM_R, + AARCH64_INSN_IMM_N, + AARCH64_INSN_IMM_MAX +}; + +enum aarch64_insn_register_type { + AARCH64_INSN_REGTYPE_RT, + AARCH64_INSN_REGTYPE_RN, + AARCH64_INSN_REGTYPE_RT2, + AARCH64_INSN_REGTYPE_RM, + AARCH64_INSN_REGTYPE_RD, + AARCH64_INSN_REGTYPE_RA, + AARCH64_INSN_REGTYPE_RS, +}; + +enum aarch64_insn_register { + AARCH64_INSN_REG_0 = 0, + AARCH64_INSN_REG_1 = 1, + AARCH64_INSN_REG_2 = 2, + AARCH64_INSN_REG_3 = 3, + AARCH64_INSN_REG_4 = 4, + AARCH64_INSN_REG_5
[RFC PATCH 08/17] objtool: arm64: Decode load/store instructions
Decode load/store operations and create corresponding stack_ops for operations targetting SP or FP. Operations storing/loading multiple registers are split into separate stack_ops storing single registers. Operations modifying the base register get an additional stack_op for the register update. Since the atomic register(s) load/store + base register update gets split into multiple operations, to make sure objtool always sees a valid stack, consider store instruction to perform stack allocations (i.e. modifying the base pointer before the storing) and loads de-allocations (i.e. modifying the base pointer after the load). Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 127 ++ 1 file changed, 127 insertions(+) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index a4a587c400a1..1087ede67bcd 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -105,6 +105,40 @@ int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg) return -1; } +static struct stack_op *arm_make_store_op(enum aarch64_insn_register base, + enum aarch64_insn_register reg, + int offset) +{ + struct stack_op *op; + + op = calloc(1, sizeof(*op)); + op->dest.type = OP_DEST_REG_INDIRECT; + op->dest.reg = base; + op->dest.offset = offset; + op->src.type = OP_SRC_REG; + op->src.reg = reg; + op->src.offset = 0; + + return op; +} + +static struct stack_op *arm_make_load_op(enum aarch64_insn_register base, +enum aarch64_insn_register reg, +int offset) +{ + struct stack_op *op; + + op = calloc(1, sizeof(*op)); + op->dest.type = OP_DEST_REG; + op->dest.reg = reg; + op->dest.offset = 0; + op->src.type = OP_SRC_REG_INDIRECT; + op->src.reg = base; + op->src.offset = offset; + + return op; +} + static struct stack_op *arm_make_add_op(enum aarch64_insn_register dest, enum aarch64_insn_register src, int val) @@ -121,6 +155,94 @@ static struct stack_op *arm_make_add_op(enum aarch64_insn_register dest, return op; } +static bool arm_decode_load_store(u32 insn, enum insn_type *type, + unsigned long *immediate, + struct list_head *ops_list) +{ + enum aarch64_insn_register base; + enum aarch64_insn_register rt; + struct stack_op *op; + int size; + int offset; + + *type = INSN_OTHER; + + if (aarch64_insn_is_store_single(insn) || + aarch64_insn_is_load_single(insn)) + size = 1 << ((insn & GENMASK(31, 30)) >> 30); + else + size = 4 << ((insn >> 31) & 1); + + if (aarch64_insn_is_store_imm(insn) || aarch64_insn_is_load_imm(insn)) + *immediate = size * aarch64_insn_decode_immediate(AARCH64_INSN_IMM_12, + insn); + else if (aarch64_insn_is_store_pre(insn) || +aarch64_insn_is_load_pre(insn) || +aarch64_insn_is_store_post(insn) || +aarch64_insn_is_load_post(insn)) + *immediate = sign_extend(aarch64_insn_decode_immediate(AARCH64_INSN_IMM_9, + insn), +9); + else if (aarch64_insn_is_stp(insn) || aarch64_insn_is_ldp(insn) || +aarch64_insn_is_stp_pre(insn) || +aarch64_insn_is_ldp_pre(insn) || +aarch64_insn_is_stp_post(insn) || +aarch64_insn_is_ldp_post(insn)) + *immediate = size * sign_extend(aarch64_insn_decode_immediate(AARCH64_INSN_IMM_7, + insn), + 7); + else + return false; + + base = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RN, insn); + if (base != AARCH64_INSN_REG_FP && base != AARCH64_INSN_REG_SP) + return true; + + offset = *immediate; + + if (aarch64_insn_is_store_pre(insn) || aarch64_insn_is_stp_pre(insn) || + aarch64_insn_is_store_post(insn) || aarch64_insn_is_stp_post(insn)) { + op = arm_make_add_op(base, base, *immediate); + list_add_tail(&op->list, ops_list); + + if (aarch64_insn_is_store_post(insn) || aarch64_insn_is_stp_post(insn)) + offset = -*immediate; + else + offset = 0; +
[RFC PATCH 04/17] objtool: arm64: Add base definition for arm64 backend
Provide needed definitions for a new architecture instruction decoder. No proper decoding is done yet. Signed-off-by: Julien Thierry --- tools/objtool/Makefile| 5 + tools/objtool/arch/arm64/Build| 8 ++ tools/objtool/arch/arm64/decode.c | 130 ++ .../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| 21 +++ tools/objtool/sync-check.sh | 5 + 9 files changed, 219 insertions(+) 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 diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index 92ce4fce7bc7..d5cfbec87c02 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -47,6 +47,11 @@ ifeq ($(SRCARCH),x86) SUBCMD_ORC := y endif +ifeq ($(SRCARCH),arm64) + SUBCMD_CHECK := y + CFLAGS += -Wno-nested-externs +endif + export SUBCMD_CHECK SUBCMD_ORC export srctree OUTPUT CFLAGS SRCARCH AWK include $(srctree)/tools/build/Makefile.include diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build new file mode 100644 index ..f3de3a50d541 --- /dev/null +++ b/tools/objtool/arch/arm64/Build @@ -0,0 +1,8 @@ +objtool-y += special.o +objtool-y += decode.o + +objtool-y += libhweight.o + +$(OUTPUT)arch/arm64/libhweight.o: ../lib/hweight.c FORCE + $(call rule_mkdir) + $(call if_changed_dep,cc_o_c) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c new file mode 100644 index ..8ae822f553ca --- /dev/null +++ b/tools/objtool/arch/arm64/decode.c @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include +#include +#include + +/* Hack needed to avoid depending on brk-imm.h */ +#define FAULT_BRK_IMM 0x100 + +#include + +#include +#include +#include +#include + +#include + +/* Hack needed to avoid depending on kprobes.h */ +#ifndef __kprobes +#define __kprobes +#endif + +#include "../../../arch/arm64/lib/aarch64-insn.c" + +bool arch_callee_saved_reg(unsigned char reg) +{ + switch (reg) { + case AARCH64_INSN_REG_19: + case AARCH64_INSN_REG_20: + case AARCH64_INSN_REG_21: + case AARCH64_INSN_REG_22: + case AARCH64_INSN_REG_23: + case AARCH64_INSN_REG_24: + case AARCH64_INSN_REG_25: + case AARCH64_INSN_REG_26: + case AARCH64_INSN_REG_27: + case AARCH64_INSN_REG_28: + case AARCH64_INSN_REG_FP: + case AARCH64_INSN_REG_LR: + return true; + default: + return false; + } +} + +void arch_initial_func_cfi_state(struct cfi_init_state *state) +{ + int i; + + for (i = 0; i < CFI_NUM_REGS; i++) { + state->regs[i].base = CFI_UNDEFINED; + state->regs[i].offset = 0; + } + + /* initial CFA (call frame address) */ + state->cfa.base = CFI_SP; + state->cfa.offset = 0; +} + +unsigned long arch_dest_reloc_offset(int addend) +{ + return addend; +} + +unsigned long arch_jump_destination(struct instruction *insn) +{ + return insn->offset + insn->immediate; +} + +const char *arch_nop_insn(int len) +{ + static u32 nop = 0; + + if (len != AARCH64_INSN_SIZE) + WARN("invalid NOP size: %d\n", len); + + if (!nop) + nop = aarch64_insn_gen_nop(); + + return (const char*)&nop; +} + +static int is_arm64(const struct elf *elf) +{ + switch (elf->ehdr.e_machine) { + case EM_AARCH64: //0xB7 + return 1; + default: + WARN("unexpected ELF machine type %x", +elf->ehdr.e_machine); + return 0; + } +} + +int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg) +{ + return -1; +} + +int arch_decode_instruction(const struct elf *elf, const struct section *sec, + unsigned long offset, unsigned int maxlen, + unsigned int *len, enum insn_type *type, + unsigned long *immediate, + struct list_head *ops_list) +{ + u32 insn; + + if (!is_arm64(elf)) + return -1; + + if (maxlen < AARCH64_INSN_SIZE) + return 0; + + *len = AARCH64_INSN_SIZE; + *immediate = 0; + + insn = *(u32 *)(sec->d
[RFC PATCH 10/17] objtool: arm64: Accept padding in code sections
The compiler can introduce some '0' words in code sections to pad the end of functions. Similar to load literal functions, record these zero words to remove the "struct instruction" created for them. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/decode.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c index b4d4d5b051b0..ed5ef0b52bbe 100644 --- a/tools/objtool/arch/arm64/decode.c +++ b/tools/objtool/arch/arm64/decode.c @@ -362,8 +362,23 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec, switch (aarch64_get_insn_class(insn)) { case AARCH64_INSN_CLS_UNKNOWN: - WARN("can't decode instruction at %s:0x%lx", sec->name, offset); - return -1; + { + /* +* There are a few reasons we might have non-valid opcodes in +* code sections: +* - For load literal, assembler can generate the data to be +* loaded in the code section +* - Compiler/assembler can generate zeroes to pad function that +* do not end on 8-byte alignment +*/ + /* Compiler might put zeroes as padding */ + if (record_invalid_insn(sec, offset, insn == 0x0)) + return -1; + + *type = INSN_OTHER; + + break; + } case AARCH64_INSN_CLS_DP_IMM: /* Mov register to and from SP are aliases of add_imm */ if (aarch64_insn_is_add_imm(insn) || -- 2.25.4
[RFC PATCH 12/17] gcc-plugins: objtool: Add plugin to detect switch table on arm64
From: Raphael Gault 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 [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 --- arch/arm64/Kconfig| 1 + scripts/Makefile.gcc-plugins | 2 + scripts/gcc-plugins/Kconfig | 4 + .../arm64_switch_table_detection_plugin.c | 85 +++ 4 files changed, 92 insertions(+) create mode 100644 scripts/gcc-plugins/arm64_switch_table_detection_plugin.c diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 05e17351e4f3..93a320cc8e03 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -100,6 +100,7 @@ config ARM64 select DMA_DIRECT_REMAP select EDAC_SUPPORT select FRAME_POINTER + select GCC_PLUGIN_SWITCH_TABLES if STACK_VALIDATION select GENERIC_ALLOCATOR select GENERIC_ARCH_TOPOLOGY select GENERIC_CLOCKEVENTS_BROADCAST diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins index 952e46876329..8af322311f6b 100644 --- a/scripts/Makefile.gcc-plugins +++ b/scripts/Makefile.gcc-plugins @@ -46,6 +46,8 @@ ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK endif export DISABLE_ARM_SSP_PER_TASK_PLUGIN +gcc-plugin-$(CONFIG_GCC_PLUGIN_SWITCH_TABLES) += arm64_switch_table_detection_plugin.so + # All the plugin CFLAGS are collected here in case a build target needs to # filter them out of the KBUILD_CFLAGS. GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y)) diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig index ab9eb4cbe33a..76efbb97d223 100644 --- a/scripts/gcc-plugins/Kconfig +++ b/scripts/gcc-plugins/Kconfig @@ -104,4 +104,8 @@ config GCC_PLUGIN_ARM_SSP_PER_TASK bool depends on GCC_PLUGINS && ARM +config GCC_PLUGIN_SWITCH_TABLES + bool + depends on GCC_PLUGINS && ARM64 + endif diff --git a/scripts/gcc-plugins/arm64_switch_table_detection_plugin.c b/scripts/gcc-plugins/arm64_switch_table_detection_plugin.c new file mode 100644 index ..60ef00ff2c5b --- /dev/null +++ b/scripts/gcc-plugins/arm64_switch_table_detection_plugin.c @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include "gcc-common.h" + +__visible int plugin_is_GPL_compatible; + +#define GEN_QUAD(rtx) assemble_integer_with_op(".quad ", rtx) + +/* + * Create an array of metadata for each jump table found in the rtl. + * The metadata contains: + * - A reference to first instruction part of the RTL expanded into an + * acutal jump + * - The number of entries in the table of offsets + * - A reference to each possible jump target + * + * Separate each entry with a null quad word. + */ +static unsigned int arm64_switchtbl_rtl_execute(void) +{ + rtx_insn *insn; + rtx_insn *labelp = NULL; + rtx_jump_table_data *tablep = NULL; + section *swt_sec; + section *curr_sec = current_function_section(); + + swt_sec = get_section(".discard.switch_table_info", + SECTION_DEBUG | SECTION_EXCLUDE, NULL); + + for (insn = get_insns(); insn; insn = NEXT_INSN(insn)) { + /* +* Find a tablejump_p INSN (using a dispatch table) +*/ + if (!tablejump_p(insn, &labelp, &tablep)) + continue; + + if (labelp && tablep) { + rtx_code_label *label_to_jump; + rtvec jump_labels = tablep->get_labels(); + int nr_labels = GET_NUM_ELEM(jump_labels); + int i; + + label_to_jump = gen_label_rtx(); + SET_LABEL_KIND(label_to_jump, LABEL_NORMAL); + emit_label_before(label_to_jump, insn); + LABEL_PRESERVE_P(label_to_jump) = 1; + + switch_to_section(swt_sec); + GEN_QUAD(GEN_INT(0)); // mark separation between rela tables + GEN_QUAD(gen_rtx_LABEL_REF(Pmode, label_to_jump)); + GEN_QUAD(GEN_INT(nr_labels)); + for (i = 0; i < nr_labels; i++) + GEN_QUAD(gen_rtx_LABEL_REF(Pmode, + label_ref_label(RTVEC_ELT(jump_labels, i; + switch_to_section(curr_sec); + delete_insn(label_to_jump); +
[RFC PATCH 13/17] objtool: arm64: Implement functions to add switch tables alternatives
This patch implements the functions required to identify and add as alternatives all the possible destinations of the switch table. This implementation relies on the new plugin introduced previously which records information about the switch-table in a .discard.switch_table_information section. Signed-off-by: Raphael Gault [J.T.: Update arch implementation to new prototypes, Update switch table information section name, Do some clean up, Use the offset sign information, Use the newly added rela to find the corresponding jump instruction] Signed-off-by: Julien Thierry --- .../objtool/arch/arm64/include/arch/special.h | 2 + tools/objtool/arch/arm64/special.c| 85 +++ 2 files changed, 87 insertions(+) diff --git a/tools/objtool/arch/arm64/include/arch/special.h b/tools/objtool/arch/arm64/include/arch/special.h index a82a9b3e51df..b96bcee308cf 100644 --- a/tools/objtool/arch/arm64/include/arch/special.h +++ b/tools/objtool/arch/arm64/include/arch/special.h @@ -3,6 +3,8 @@ #ifndef _ARM64_ARCH_SPECIAL_H #define _ARM64_ARCH_SPECIAL_H +#include + #define EX_ENTRY_SIZE 8 #define EX_ORIG_OFFSET 0 #define EX_NEW_OFFSET 4 diff --git a/tools/objtool/arch/arm64/special.c b/tools/objtool/arch/arm64/special.c index 45f283283091..396b9c5feebd 100644 --- a/tools/objtool/arch/arm64/special.c +++ b/tools/objtool/arch/arm64/special.c @@ -1,6 +1,26 @@ // SPDX-License-Identifier: GPL-2.0-or-later +#include +#include + +#include + #include +#include +#include + +/* + * The arm64_switch_table_detection_plugin generate an array of elements + * described by the following structure. + * Each jump table found in the compilation unit is associated with one of + * entries of the array. + */ +struct switch_table_info { + u64 padding; + u64 jump_ref; + u64 nb_entries; + u64 dest_relocations[]; +} __attribute__((__packed__)); void arch_handle_alternative(unsigned short feature, struct special_alt *alt) { @@ -14,8 +34,73 @@ bool arch_support_alt_relocation(struct special_alt *special_alt, } +/* + * Aarch64 jump tables are just arrays of offsets (of varying size/signess) + * representing the potential destination from a base address loaded by an adr + * instruction. + * + * Sadly, extracting the actual offset might require to consider multiple + * instructions and decoding them to understand what they do. To make life + * easier, the gcc plugin will generate a list of relocation entries for + * each jump table target, conforming to the format expected by + * add_jump_table(). + * + * Aarch64 branches to jump tables are composed of multiple instructions: + * + * ldr x_offset, [x_offsets_table, x_index, ...] + * adr x_dest_base, + * add x_dest, x_target_base, x_offset, ... + * br x_dest + * + * The arm64_switch_table_detection_plugin will make the connection between + * the instruction setting x_offsets_table (jump_ref) and the list of + * relocations. + */ struct reloc *arch_find_switch_table(struct objtool_file *file, struct instruction *insn) { + struct switch_table_info *sti; + struct section *table_info_sec; + void *sti_sec_start; + struct reloc *text_reloc; + + table_info_sec = find_section_by_name(file->elf, + ".discard.switch_table_info"); + if (!table_info_sec) + goto try_c_jmptbl; + + sti_sec_start = table_info_sec->data->d_buf; + sti = sti_sec_start; + + while ((char *)sti - (char *)sti_sec_start < table_info_sec->len) { + struct reloc *target_reloc = find_reloc_by_dest(file->elf, + table_info_sec, + (char *)&sti->jump_ref - (char *)sti_sec_start); + + if (!target_reloc) { + WARN("Malformed switch table entry"); + return NULL; + } + + if (target_reloc->sym->sec == insn->sec && + target_reloc->addend == insn->offset) + return find_reloc_by_dest(file->elf, table_info_sec, + (char *)&sti->dest_relocations[0] - (char *)sti_sec_start); + + /* Get next jump table entry */ + sti = (struct switch_table_info *) (&sti->dest_relocations[0] + sti->nb_entries); + } + +try_c_jmptbl: + text_reloc = find_reloc_by_dest(file->elf, insn->sec, insn->offset); + if (!text_reloc || text_reloc->sym->type != STT_SECTION || + !text_reloc->sym->sec->rodata) + return NULL; + + /* Handle C jump tables */ + if (!strcmp(text_reloc-&
[RFC PATCH 16/17] objtool: arm64: Ignore replacement section for alternative callback
ARM64_CB_PATCH doesn't have static replacement instructions. Skip trying to validate the alternative section. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/special.c | 12 tools/objtool/check.c | 3 +++ 2 files changed, 15 insertions(+) diff --git a/tools/objtool/arch/arm64/special.c b/tools/objtool/arch/arm64/special.c index d47e5590ed60..aff8577e71e9 100644 --- a/tools/objtool/arch/arm64/special.c +++ b/tools/objtool/arch/arm64/special.c @@ -24,6 +24,18 @@ struct switch_table_info { void arch_handle_alternative(unsigned short feature, struct special_alt *alt) { + if (alt->orig_len && !alt->new_len) { + /* +* ARM64_CB_PATCH has no alternative instruction. +* a callback is called at alternative replacement time +* to dynamically change the original instructions. +* +* ARM64_CB_PATCH is the last ARM64 feature, it's value changes +* every time a new feature is added. So the orig/alt region +* length are used to detect those alternatives +*/ + alt->skip_alt = true; + } } bool arch_support_alt_relocation(struct special_alt *special_alt, diff --git a/tools/objtool/check.c b/tools/objtool/check.c index d902697a388e..8840af09f843 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1182,6 +1182,9 @@ static int add_special_section_alts(struct objtool_file *file) continue; } + if (special_alt->skip_alt && !special_alt->new_len) + continue; + ret = handle_group_alt(file, special_alt, orig_insn, &new_insn); if (ret) -- 2.25.4
[RFC PATCH 14/17] objtool: arm64: Cache section with switch table information
Section ".discard.switch_table_info", created by the gcc plugin will be looked up for every dynamic jump in the object file while the section might not even exist. Cache the result of the first lookup. Signed-off-by: Julien Thierry --- tools/objtool/arch/arm64/special.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/objtool/arch/arm64/special.c b/tools/objtool/arch/arm64/special.c index 396b9c5feebd..c9c3e0bfd581 100644 --- a/tools/objtool/arch/arm64/special.c +++ b/tools/objtool/arch/arm64/special.c @@ -33,6 +33,19 @@ bool arch_support_alt_relocation(struct special_alt *special_alt, return false; } +static struct section *get_switch_table_info_section(struct objtool_file *file) +{ + static bool first = true; + static struct section *info_section = NULL; + + if (first) { + first = false; + info_section = find_section_by_name(file->elf, + ".discard.switch_table_info"); + } + + return info_section; +} /* * Aarch64 jump tables are just arrays of offsets (of varying size/signess) @@ -64,8 +77,7 @@ struct reloc *arch_find_switch_table(struct objtool_file *file, void *sti_sec_start; struct reloc *text_reloc; - table_info_sec = find_section_by_name(file->elf, - ".discard.switch_table_info"); + table_info_sec = get_switch_table_info_section(file); if (!table_info_sec) goto try_c_jmptbl; -- 2.25.4
[RFC PATCH 17/17] objtool: arm64: Enable stack validation for arm64
From: Raphael Gault Add build option to run stack validation at compile time. Signed-off-by: Raphael Gault Signed-off-by: Julien Thierry --- arch/arm64/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 93a320cc8e03..3f297d61b56b 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -185,6 +185,7 @@ config ARM64 select MMU_GATHER_RCU_TABLE_FREE select HAVE_RSEQ select HAVE_STACKPROTECTOR + select HAVE_STACK_VALIDATION if CC_IS_GCC select HAVE_SYSCALL_TRACEPOINTS select HAVE_KPROBES select HAVE_KRETPROBES -- 2.25.4
[RFC PATCH 11/17] efi: libstub: Ignore relocations for .discard sections
EFI stub cannot have absolute relocations in sections affecting the execution flow. However, for sections that get discarded at link time, it doesn't really matter if they have absolute relocations. Ignore the relocation associated with such sections. Signed-off-by: Julien Thierry --- drivers/firmware/efi/libstub/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 8a94388e38b3..70e9c7f45d30 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -133,7 +133,7 @@ $(obj)/%.stub.o: $(obj)/%.o FORCE # quiet_cmd_stubcopy = STUBCPY $@ cmd_stubcopy = \ - $(STRIP) --strip-debug -o $@ $<;\ + $(STRIP) --strip-debug --remove-relocations=".discard.*" -o $@ $<; \ if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); then\ echo "$@: absolute symbol references not allowed in the EFI stub" >&2; \ /bin/false; \ -- 2.25.4
Re: Live patching on ARM64
Hi, On 1/19/21 4:19 PM, Madhavan T. Venkataraman wrote: Sorry for the late reply. The last RFC for arm64 support in objtool is a bit old because it was preferable to split things into smaller series. I touched it much lately, so I'm picking it back up and will try to get a git branch into shape on a recent mainline (a few things need fixing since the last time I rebased it). I'll update you once I have something at least usable/presentable. Cheers, I just sent some series the arm64 objtool support: - https://lkml.org/lkml/2021/1/20/791 - https://lore.kernel.org/linux-arm-kernel/20210120173800.1660730-1-jthie...@redhat.com/T/#t There are still some things missing, so if you want to investigate a more complete state I have a branch: $ git clone https://github.com/julien-thierry/linux.git -b objtoolxarm64-latest Let me know if there are any questions related to it. Cheers, -- Julien Thierry
[RFC PATCH 4/5] arm64: aarch64-insn: Add some opcodes to instruction decoder
Add decoding capability for some instructions that objtool will need to decode. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/aarch64-insn.h | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h index d0fee47bbe6e..3a0e0ad51f5b 100644 --- a/arch/arm64/include/asm/aarch64-insn.h +++ b/arch/arm64/include/asm/aarch64-insn.h @@ -305,6 +305,12 @@ __AARCH64_INSN_FUNCS(adr, 0x9F00, 0x1000) __AARCH64_INSN_FUNCS(adrp, 0x9F00, 0x9000) __AARCH64_INSN_FUNCS(prfm, 0x3FC0, 0x3980) __AARCH64_INSN_FUNCS(prfm_lit, 0xFF00, 0xD800) +__AARCH64_INSN_FUNCS(store_imm,0x3FC0, 0x3900) +__AARCH64_INSN_FUNCS(load_imm, 0x3FC0, 0x3940) +__AARCH64_INSN_FUNCS(store_pre,0x3FE00C00, 0x38000C00) +__AARCH64_INSN_FUNCS(load_pre, 0x3FE00C00, 0x38400C00) +__AARCH64_INSN_FUNCS(store_post, 0x3FE00C00, 0x38000400) +__AARCH64_INSN_FUNCS(load_post,0x3FE00C00, 0x38400400) __AARCH64_INSN_FUNCS(str_reg, 0x3FE0EC00, 0x38206800) __AARCH64_INSN_FUNCS(ldadd,0x3F20FC00, 0x3820) __AARCH64_INSN_FUNCS(ldr_reg, 0x3FE0EC00, 0x38606800) @@ -313,6 +319,8 @@ __AARCH64_INSN_FUNCS(ldrsw_lit, 0xFF00, 0x9800) __AARCH64_INSN_FUNCS(exclusive,0x3F80, 0x0800) __AARCH64_INSN_FUNCS(load_ex, 0x3F40, 0x0840) __AARCH64_INSN_FUNCS(store_ex, 0x3F40, 0x0800) +__AARCH64_INSN_FUNCS(stp, 0x7FC0, 0x2900) +__AARCH64_INSN_FUNCS(ldp, 0x7FC0, 0x2940) __AARCH64_INSN_FUNCS(stp_post, 0x7FC0, 0x2880) __AARCH64_INSN_FUNCS(ldp_post, 0x7FC0, 0x28C0) __AARCH64_INSN_FUNCS(stp_pre, 0x7FC0, 0x2980) @@ -345,6 +353,7 @@ __AARCH64_INSN_FUNCS(rev64, 0x7C00, 0x5AC00C00) __AARCH64_INSN_FUNCS(and, 0x7F20, 0x0A00) __AARCH64_INSN_FUNCS(bic, 0x7F20, 0x0A20) __AARCH64_INSN_FUNCS(orr, 0x7F20, 0x2A00) +__AARCH64_INSN_FUNCS(mov_reg, 0x7FE0FFE0, 0x2A0003E0) __AARCH64_INSN_FUNCS(orn, 0x7F20, 0x2A20) __AARCH64_INSN_FUNCS(eor, 0x7F20, 0x4A00) __AARCH64_INSN_FUNCS(eon, 0x7F20, 0x4A20) -- 2.25.4
[RFC PATCH 5/5] arm64: Add load/store decoding helpers
Provide some function to group different load/store instructions. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/aarch64-insn.h | 28 +++ 1 file changed, 28 insertions(+) diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h index 3a0e0ad51f5b..f1b5b652c400 100644 --- a/arch/arm64/include/asm/aarch64-insn.h +++ b/arch/arm64/include/asm/aarch64-insn.h @@ -408,6 +408,34 @@ static inline bool aarch64_insn_is_barrier(u32 insn) aarch64_insn_is_isb(insn); } +static inline bool aarch64_insn_is_store_single(u32 insn) +{ + return aarch64_insn_is_store_imm(insn) || + aarch64_insn_is_store_pre(insn) || + aarch64_insn_is_store_post(insn); +} + +static inline bool aarch64_insn_is_store_pair(u32 insn) +{ + return aarch64_insn_is_stp(insn) || + aarch64_insn_is_stp_pre(insn) || + aarch64_insn_is_stp_post(insn); +} + +static inline bool aarch64_insn_is_load_single(u32 insn) +{ + return aarch64_insn_is_load_imm(insn) || + aarch64_insn_is_load_pre(insn) || + aarch64_insn_is_load_post(insn); +} + +static inline bool aarch64_insn_is_load_pair(u32 insn) +{ + return aarch64_insn_is_ldp(insn) || + aarch64_insn_is_ldp_pre(insn) || + aarch64_insn_is_ldp_post(insn); +} + enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); bool aarch64_insn_uses_literal(u32 insn); bool aarch64_insn_is_branch(u32 insn); -- 2.25.4
[RFC PATCH 0/5] arm64: Prepare instruction decoder for objtool
To support arm64, objtool will need to be able to decode aarch64 instructions. This patch series adds some instruction definitions needed by objtool and moves out encoding/decoding functionalities that do not rely on kernel code in order. I'll post the start of the arm64 objtool backend shortly. Thanks, Julien --> Julien Thierry (5): arm64: Move instruction encoder/decoder under lib/ arm64: aarch64-insn: Add SVE instruction class arm64: aarch64-insn: Add barrier encodings arm64: aarch64-insn: Add some opcodes to instruction decoder arm64: Add load/store decoding helpers arch/arm64/include/asm/aarch64-insn.h | 552 +++ arch/arm64/include/asm/alternative-macros.h |3 - arch/arm64/include/asm/alternative.h|1 + arch/arm64/include/asm/debug-monitors.h | 14 +- arch/arm64/include/asm/ftrace.h |2 +- arch/arm64/include/asm/insn.h | 476 --- arch/arm64/include/asm/jump_label.h |2 +- arch/arm64/include/asm/uprobes.h|2 +- arch/arm64/kernel/insn.c| 1416 +- arch/arm64/lib/Makefile |2 +- arch/arm64/lib/aarch64-insn.c | 1426 +++ 11 files changed, 1985 insertions(+), 1911 deletions(-) create mode 100644 arch/arm64/include/asm/aarch64-insn.h create mode 100644 arch/arm64/lib/aarch64-insn.c -- 2.25.4
[RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/
Aarch64 instruction set encoding and decoding logic can prove useful for some features/tools both part of the kernel and outside the kernel. Isolate the function dealing only with encoding/decoding instructions, with minimal dependency on kernel utilities in order to be able to reuse that code. Code was only moved, no code should have been added, removed nor modifier. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/aarch64-insn.h | 505 +++ arch/arm64/include/asm/alternative-macros.h |3 - arch/arm64/include/asm/alternative.h|1 + arch/arm64/include/asm/debug-monitors.h | 14 +- arch/arm64/include/asm/ftrace.h |2 +- arch/arm64/include/asm/insn.h | 476 --- arch/arm64/include/asm/jump_label.h |2 +- arch/arm64/include/asm/uprobes.h|2 +- arch/arm64/kernel/insn.c| 1416 +- arch/arm64/lib/Makefile |2 +- arch/arm64/lib/aarch64-insn.c | 1426 +++ 11 files changed, 1938 insertions(+), 1911 deletions(-) create mode 100644 arch/arm64/include/asm/aarch64-insn.h create mode 100644 arch/arm64/lib/aarch64-insn.c diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h new file mode 100644 index ..cead3be1a2d0 --- /dev/null +++ b/arch/arm64/include/asm/aarch64-insn.h @@ -0,0 +1,505 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef__ASM_AARCH64_INSN_H +#define__ASM_AARCH64_INSN_H + +#include +#include + +#include + +/* A64 instructions are always 32 bits. */ +#defineAARCH64_INSN_SIZE 4 + +/* + * BRK instruction encoding + * The #imm16 value should be placed at bits[20:5] within BRK ins + */ +#define AARCH64_BREAK_MON 0xd420 + +/* + * BRK instruction for provoking a fault on purpose + * Unlike kgdb, #imm16 value with unallocated handler is used for faulting. + */ +#define AARCH64_BREAK_FAULT(AARCH64_BREAK_MON | (FAULT_BRK_IMM << 5)) + +#ifndef __ASSEMBLY__ +/* + * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a + * Section C3.1 "A64 instruction index by encoding": + * AArch64 main encoding table + * Bit position + * 28 27 26 25 Encoding Group + * 0 0 - -Unallocated + * 1 0 0 -Data processing, immediate + * 1 0 1 -Branch, exception generation and system instructions + * - 1 - 0Loads and stores + * - 1 0 1Data processing - register + * 0 1 1 1Data processing - SIMD and floating point + * 1 1 1 1Data processing - SIMD and floating point + * "-" means "don't care" + */ +enum aarch64_insn_encoding_class { + AARCH64_INSN_CLS_UNKNOWN, /* UNALLOCATED */ + AARCH64_INSN_CLS_DP_IMM,/* Data processing - immediate */ + AARCH64_INSN_CLS_DP_REG,/* Data processing - register */ + AARCH64_INSN_CLS_DP_FPSIMD, /* Data processing - SIMD and FP */ + AARCH64_INSN_CLS_LDST, /* Loads and stores */ + AARCH64_INSN_CLS_BR_SYS,/* Branch, exception generation and +* system instructions */ +}; + +enum aarch64_insn_hint_cr_op { + AARCH64_INSN_HINT_NOP = 0x0 << 5, + AARCH64_INSN_HINT_YIELD = 0x1 << 5, + AARCH64_INSN_HINT_WFE = 0x2 << 5, + AARCH64_INSN_HINT_WFI = 0x3 << 5, + AARCH64_INSN_HINT_SEV = 0x4 << 5, + AARCH64_INSN_HINT_SEVL = 0x5 << 5, + + AARCH64_INSN_HINT_XPACLRI= 0x07 << 5, + AARCH64_INSN_HINT_PACIA_1716 = 0x08 << 5, + AARCH64_INSN_HINT_PACIB_1716 = 0x0A << 5, + AARCH64_INSN_HINT_AUTIA_1716 = 0x0C << 5, + AARCH64_INSN_HINT_AUTIB_1716 = 0x0E << 5, + AARCH64_INSN_HINT_PACIAZ = 0x18 << 5, + AARCH64_INSN_HINT_PACIASP= 0x19 << 5, + AARCH64_INSN_HINT_PACIBZ = 0x1A << 5, + AARCH64_INSN_HINT_PACIBSP= 0x1B << 5, + AARCH64_INSN_HINT_AUTIAZ = 0x1C << 5, + AARCH64_INSN_HINT_AUTIASP= 0x1D << 5, + AARCH64_INSN_HINT_AUTIBZ = 0x1E << 5, + AARCH64_INSN_HINT_AUTIBSP= 0x1F << 5, + + AARCH64_INSN_HINT_ESB = 0x10 << 5, + AARCH64_INSN_HINT_PSB = 0x11 << 5, + AARCH64_INSN_HINT_TSB = 0x12 << 5, + AARCH64_INSN_HINT_CSDB = 0x14 << 5, + + AARCH64_INSN_HINT_BTI = 0x20 << 5, + AARCH64_INSN_HINT_BTIC = 0x22 << 5, + AARCH64_INSN_HINT_BTIJ = 0x24 << 5, + AARCH64_INSN_HINT_BTIJC = 0x26 << 5, +}; + +enum aarch64_insn_imm_type { + AARCH64_INSN_IMM_ADR, + AARCH64_INSN_IMM_26, + AARCH64_INSN_IMM_19, + AARCH64_INSN_IMM_16, + AARCH64_INSN_
[RFC PATCH 2/5] arm64: aarch64-insn: Add SVE instruction class
SVE has been public for some time now. Let the decoder acknowledge its existence. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/aarch64-insn.h | 1 + arch/arm64/lib/aarch64-insn.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h index cead3be1a2d0..200bee726172 100644 --- a/arch/arm64/include/asm/aarch64-insn.h +++ b/arch/arm64/include/asm/aarch64-insn.h @@ -40,6 +40,7 @@ */ enum aarch64_insn_encoding_class { AARCH64_INSN_CLS_UNKNOWN, /* UNALLOCATED */ + AARCH64_INSN_CLS_SVE, /* SVE instructions */ AARCH64_INSN_CLS_DP_IMM,/* Data processing - immediate */ AARCH64_INSN_CLS_DP_REG,/* Data processing - register */ AARCH64_INSN_CLS_DP_FPSIMD, /* Data processing - SIMD and FP */ diff --git a/arch/arm64/lib/aarch64-insn.c b/arch/arm64/lib/aarch64-insn.c index df233a7790dc..95d9143aa9c6 100644 --- a/arch/arm64/lib/aarch64-insn.c +++ b/arch/arm64/lib/aarch64-insn.c @@ -17,7 +17,7 @@ static const int aarch64_insn_encoding_class[] = { AARCH64_INSN_CLS_UNKNOWN, AARCH64_INSN_CLS_UNKNOWN, - AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_SVE, AARCH64_INSN_CLS_UNKNOWN, AARCH64_INSN_CLS_LDST, AARCH64_INSN_CLS_DP_REG, -- 2.25.4
[RFC PATCH 3/5] arm64: aarch64-insn: Add barrier encodings
Create necessary functions to encode/decode aarch64 data/instruction barriers. Signed-off-by: Julien Thierry --- arch/arm64/include/asm/aarch64-insn.h | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm64/include/asm/aarch64-insn.h b/arch/arm64/include/asm/aarch64-insn.h index 200bee726172..d0fee47bbe6e 100644 --- a/arch/arm64/include/asm/aarch64-insn.h +++ b/arch/arm64/include/asm/aarch64-insn.h @@ -379,6 +379,9 @@ __AARCH64_INSN_FUNCS(eret_auth, 0xFBFF, 0xD69F0BFF) __AARCH64_INSN_FUNCS(mrs, 0xFFF0, 0xD530) __AARCH64_INSN_FUNCS(msr_imm, 0xFFF8F01F, 0xD500401F) __AARCH64_INSN_FUNCS(msr_reg, 0xFFF0, 0xD510) +__AARCH64_INSN_FUNCS(dmb, 0xF0FF, 0xD50330BF) +__AARCH64_INSN_FUNCS(dsb, 0xF0FF, 0xD503309F) +__AARCH64_INSN_FUNCS(isb, 0xF0FF, 0xD50330DF) #undef __AARCH64_INSN_FUNCS @@ -390,6 +393,12 @@ static inline bool aarch64_insn_is_adr_adrp(u32 insn) return aarch64_insn_is_adr(insn) || aarch64_insn_is_adrp(insn); } +static inline bool aarch64_insn_is_barrier(u32 insn) +{ + return aarch64_insn_is_dmb(insn) || aarch64_insn_is_dsb(insn) || + aarch64_insn_is_isb(insn); +} + enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); bool aarch64_insn_uses_literal(u32 insn); bool aarch64_insn_is_branch(u32 insn); -- 2.25.4
Re: Live patching on ARM64
Hi Madhavan, On 1/17/21 6:25 PM, Madhavan T. Venkataraman wrote: On 1/15/21 6:33 AM, Mark Rutland wrote: It looks like the most recent work in this area has been from the following folks: Mark Brown and Mark Rutland: Kernel changes to providing reliable stack traces. Julien Thierry: Providing ARM64 support in objtool. Torsten Duwe: Ftrace with regs. IIRC that's about right. I'm also trying to make arm64 patch-safe (more on that below), and there's a long tail of work there for anyone interested. OK. I apologize if I have missed anyone else who is working on Live Patching for ARM64. Do let me know. Is there any work I can help with? Any areas that need investigation, any code that needs to be written, any work that needs to be reviewed, any testing that needs to done? You folks are probably super busy and would not mind an extra hand. One general thing that I believe we'll need to do is to rework code to be patch-safe (which implies being noinstr-safe too). For example, we'll need to rework the instruction patching code such that this cannot end up patching itself (or anything that has instrumented it) in an unsafe way. OK. Once we have objtool it should be possible to identify those cases automatically. Currently I'm aware that we'll need to do something in at least the following places: * The entry code -- I'm currently chipping away at this. OK. * The insn framework (which is used by some patching code), since the bulk of it lives in arch/arm64/kernel/insn.c and isn't marked noinstr. We can probably shift the bulk of the aarch64_insn_gen_*() and aarch64_get_*() helpers into a header as __always_inline functions, which would allow them to be used in noinstr code. As those are typically invoked with a number of constant arguments that the compiler can fold, this /might/ work out as an optimization if the compiler can elide the error paths. * The alternatives code, since we call instrumentable and patchable functions between updating instructions and performing all the necessary maintenance. There are a number of cases within __apply_alternatives(), e.g. - test_bit() - cpus_have_cap() - pr_info_once() - lm_alias() - alt_cb, if the callback is not marked as noinstr, or if it calls instrumentable code (e.g. from the insn framework). - clean_dcache_range_nopatch(), as read_sanitised_ftr_reg() and related code can be instrumented. This might need some underlying rework elsewhere (e.g. in the cpufeature code, or atomics framework). OK. So on the kernel side, maybe a first step would be to try to headerize the insn generation code as __always_inline, and see whether that looks ok? With that out of the way it'd be a bit easier to rework patching code depending on the insn framework. OK. I have an understanding of some of the above already. I will come up to speed on the others. I will email you any questions I might have. I'm not sure about the objtool side, so I'll leave that to Julien and co to answer. Sorry for the late reply. The last RFC for arm64 support in objtool is a bit old because it was preferable to split things into smaller series. I touched it much lately, so I'm picking it back up and will try to get a git branch into shape on a recent mainline (a few things need fixing since the last time I rebased it). I'll update you once I have something at least usable/presentable. Cheers, -- Julien Thierry
[tip: objtool/core] objtool: Make SP memory operation match PUSH/POP semantics
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 201ef5a974e24112953b74cc9f33dcfc4cbcc1cb Gitweb: https://git.kernel.org/tip/201ef5a974e24112953b74cc9f33dcfc4cbcc1cb Author:Julien Thierry AuthorDate:Wed, 14 Oct 2020 08:38:02 +01:00 Committer: Josh Poimboeuf CommitterDate: Wed, 13 Jan 2021 18:13:10 -06:00 objtool: Make SP memory operation match PUSH/POP semantics Architectures without PUSH/POP instructions will always access the stack though memory operations (SRC/DEST_INDIRECT). Make those operations have the same effect on the CFA as PUSH/POP, with no stack pointer modification. Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Signed-off-by: Josh Poimboeuf --- tools/objtool/check.c | 20 1 file changed, 20 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 00d00f9..270adc3 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2065,6 +2065,14 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; case OP_SRC_REG_INDIRECT: + if (!cfi->drap && op->dest.reg == cfa->base && + op->dest.reg == CFI_BP) { + + /* mov disp(%rsp), %rbp */ + cfa->base = CFI_SP; + cfa->offset = cfi->stack_size; + } + if (cfi->drap && op->src.reg == CFI_BP && op->src.offset == cfi->drap_offset) { @@ -2086,6 +2094,12 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov disp(%rbp), %reg */ /* mov disp(%rsp), %reg */ restore_reg(cfi, op->dest.reg); + + } else if (op->src.reg == CFI_SP && + op->src.offset == regs[op->dest.reg].offset + cfi->stack_size) { + + /* mov disp(%rsp), %reg */ + restore_reg(cfi, op->dest.reg); } break; @@ -2163,6 +2177,12 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov reg, disp(%rsp) */ save_reg(cfi, op->src.reg, CFI_CFA, op->dest.offset - cfi->cfa.offset); + + } else if (op->dest.reg == CFI_SP) { + + /* mov reg, disp(%rsp) */ + save_reg(cfi, op->src.reg, CFI_CFA, +op->dest.offset - cfi->stack_size); } break;
[tip: objtool/core] objtool: Fully validate the stack frame
The following commit has been merged into the objtool/core branch of tip: Commit-ID: fb084fde0c8106bc86df243411751c3421c07c08 Gitweb: https://git.kernel.org/tip/fb084fde0c8106bc86df243411751c3421c07c08 Author:Julien Thierry AuthorDate:Wed, 14 Oct 2020 08:38:00 +01:00 Committer: Josh Poimboeuf CommitterDate: Wed, 13 Jan 2021 18:13:09 -06:00 objtool: Fully validate the stack frame A valid stack frame should contain both the return address and the previous frame pointer value. On x86, the return value is placed on the stack by the calling instructions. On other architectures, the callee needs to explicitly save the return address on the stack. Add the necessary checks to verify a function properly sets up all the elements of the stack frame. Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Signed-off-by: Josh Poimboeuf --- tools/objtool/check.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 5f8d3ee..88210b0 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1733,12 +1733,20 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state return false; } +static bool check_reg_frame_pos(const struct cfi_reg *reg, + int expected_offset) +{ + return reg->base == CFI_CFA && + reg->offset == expected_offset; +} + static bool has_valid_stack_frame(struct insn_state *state) { struct cfi_state *cfi = &state->cfi; - if (cfi->cfa.base == CFI_BP && cfi->regs[CFI_BP].base == CFI_CFA && - cfi->regs[CFI_BP].offset == -16) + if (cfi->cfa.base == CFI_BP && + check_reg_frame_pos(&cfi->regs[CFI_BP], -cfi->cfa.offset) && + check_reg_frame_pos(&cfi->regs[CFI_RA], -cfi->cfa.offset + 8)) return true; if (cfi->drap && cfi->regs[CFI_BP].base == CFI_BP) @@ -1867,8 +1875,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, case OP_SRC_REG: if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP && cfa->base == CFI_SP && - regs[CFI_BP].base == CFI_CFA && - regs[CFI_BP].offset == -cfa->offset) { + check_reg_frame_pos(®s[CFI_BP], -cfa->offset)) { /* mov %rsp, %rbp */ cfa->base = op->dest.reg;
[tip: objtool/core] objtool: Support addition to set CFA base
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 468af56a7bbaa626da5a4578bedc930d731fba13 Gitweb: https://git.kernel.org/tip/468af56a7bbaa626da5a4578bedc930d731fba13 Author:Julien Thierry AuthorDate:Wed, 14 Oct 2020 08:38:01 +01:00 Committer: Josh Poimboeuf CommitterDate: Wed, 13 Jan 2021 18:13:10 -06:00 objtool: Support addition to set CFA base On arm64, the compiler can set the frame pointer either with a move operation or with and add operation like: add (SP + constant), BP For a simple move operation, the CFA base is changed from SP to BP. Handle also changing the CFA base when the frame pointer is set with an addition instruction. Signed-off-by: Julien Thierry Acked-by: Peter Zijlstra (Intel) Signed-off-by: Josh Poimboeuf --- tools/objtool/check.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 88210b0..00d00f9 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1960,6 +1960,17 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; } + if (!cfi->drap && op->src.reg == CFI_SP && + op->dest.reg == CFI_BP && cfa->base == CFI_SP && + check_reg_frame_pos(®s[CFI_BP], -cfa->offset + op->src.offset)) { + + /* lea disp(%rsp), %rbp */ + cfa->base = CFI_BP; + cfa->offset -= op->src.offset; + cfi->bp_scratch = false; + break; + } + if (op->src.reg == CFI_SP && cfa->base == CFI_SP) { /* drap: lea disp(%rsp), %drap */
[PATCH v3 0/3] objtool: Extend CFA updating/checking
Hi, The following patches are the result of limitation found on the CFA management code when trying to validate arm64 frames. I tried to keep things simple and not contradict current CFA management logic nor introduce too many corner cases. Changes since V2[1]: - Simplify cfa offset checking for BP and return address [1] https://lkml.org/lkml/2020/9/28/354 Thanks, Julien Julien Thierry (3): objtool: check: Fully validate the stack frame objtool: check: Support addition to set CFA base objtool: check: Make SP memory operation match PUSH/POP semantics tools/objtool/check.c | 46 +++ 1 file changed, 42 insertions(+), 4 deletions(-) -- 2.25.4
[PATCH v3 1/3] objtool: check: Fully validate the stack frame
A valid stack frame should contain both the return address and the previous frame pointer value. On x86, the return value is placed on the stack by the calling instructions. On other architectures, the callee need to explicitly save the return address on the stack. Add the necessary checks to verify a function properly sets up all the elements of the stack frame. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index f50ffa243c72..87f10e726a75 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1671,12 +1671,20 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state return false; } +static bool check_reg_frame_pos(const struct cfi_reg *reg, + int expected_offset) +{ + return reg->base == CFI_CFA && + reg->offset == expected_offset; +} + static bool has_valid_stack_frame(struct insn_state *state) { struct cfi_state *cfi = &state->cfi; - if (cfi->cfa.base == CFI_BP && cfi->regs[CFI_BP].base == CFI_CFA && - cfi->regs[CFI_BP].offset == -16) + if (cfi->cfa.base == CFI_BP && + check_reg_frame_pos(&cfi->regs[CFI_BP], -cfi->cfa.offset) && + check_reg_frame_pos(&cfi->regs[CFI_RA], -cfi->cfa.offset + 8)) return true; if (cfi->drap && cfi->regs[CFI_BP].base == CFI_BP) @@ -1805,8 +1813,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, case OP_SRC_REG: if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP && cfa->base == CFI_SP && - regs[CFI_BP].base == CFI_CFA && - regs[CFI_BP].offset == -cfa->offset) { + check_reg_frame_pos(®s[CFI_BP], -cfa->offset)) { /* mov %rsp, %rbp */ cfa->base = op->dest.reg; -- 2.25.4
[PATCH v3 3/3] objtool: check: Make SP memory operation match PUSH/POP semantics
Architectures without PUSH/POP instructions will always access the stack though memory operations (SRC/DEST_INDIRECT). Make those operations have the same effect on the CFA as PUSH/POP, with no stack pointer modification. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 20 1 file changed, 20 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 815aeb770930..32ee9902ade2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2003,6 +2003,14 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; case OP_SRC_REG_INDIRECT: + if (!cfi->drap && op->dest.reg == cfa->base && + op->dest.reg == CFI_BP) { + + /* mov disp(%rsp), %rbp */ + cfa->base = CFI_SP; + cfa->offset = cfi->stack_size; + } + if (cfi->drap && op->src.reg == CFI_BP && op->src.offset == cfi->drap_offset) { @@ -2024,6 +2032,12 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov disp(%rbp), %reg */ /* mov disp(%rsp), %reg */ restore_reg(cfi, op->dest.reg); + + } else if (op->src.reg == CFI_SP && + op->src.offset == regs[op->dest.reg].offset + cfi->stack_size) { + + /* mov disp(%rsp), %reg */ + restore_reg(cfi, op->dest.reg); } break; @@ -2101,6 +2115,12 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov reg, disp(%rsp) */ save_reg(cfi, op->src.reg, CFI_CFA, op->dest.offset - cfi->cfa.offset); + + } else if (op->dest.reg == CFI_SP) { + + /* mov reg, disp(%rsp) */ + save_reg(cfi, op->src.reg, CFI_CFA, +op->dest.offset - cfi->stack_size); } break; -- 2.25.4
[PATCH v3 2/3] objtool: check: Support addition to set CFA base
On arm64, the compiler can set the frame pointer either with a move operation or with and add operation like: add (SP + constant), BP For a simple move operation, the CFA base is changed from SP to BP. Handle also changing the CFA base when the frame pointer is set with an addition instruction. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 87f10e726a75..815aeb770930 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1898,6 +1898,17 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; } + if (!cfi->drap && op->src.reg == CFI_SP && + op->dest.reg == CFI_BP && cfa->base == CFI_SP && + check_reg_frame_pos(®s[CFI_BP], -cfa->offset + op->src.offset)) { + + /* lea disp(%rsp), %rbp */ + cfa->base = CFI_BP; + cfa->offset -= op->src.offset; + cfi->bp_scratch = false; + break; + } + if (op->src.reg == CFI_SP && cfa->base == CFI_SP) { /* drap: lea disp(%rsp), %drap */ -- 2.25.4
Re: [PATCH v2 1/3] objtool: check: Fully validate the stack frame
On 10/12/20 4:35 PM, Josh Poimboeuf wrote: On Mon, Oct 12, 2020 at 11:21:49AM +0100, Julien Thierry wrote: On 9/29/20 8:18 PM, Josh Poimboeuf wrote: "Stack frame" has more than one meaning now, I suppose. i.e. it could also include the callee-saved registers and any other stack space allocated by the function. Would "call frame" be clearer? CALL_FRAME_BP_OFFSET CALL_FRAME_RA_OFFSET ? I would've thought that the call-frame could include the stackframe + other callee saved regs. Hm, probably so. Whereas stackframe tends to used for the caller's frame pointer + return address (i.e. what allows unwinding). Unless I'm getting lost with things. I've always seen "stack frame" used to indicate the function's entire stack. And if call frame is associated with the region starting from the stack pointer at the parent call point (since this is what CFA is), then it shouldn't be associated with the framepointer + return address structure since this could be anywhere on the call frame (not at a fixed offset) as long as the new frame pointer points to the structure. I suppose "call frame" and "stack frame" probably mean the same thing, in which case neither is appropriate here... In fact, maybe we could forget the concept of a frame (or even a struct) here. If cfa.base is CFI_BP, then is regs[CFI_BP].offset always the same as -cfa.offset? i.e. could the BP checks could it just be a simple regs[CFI_BP].offset == -cfa.offset check? I guess that makes sense. If the above was no true it would mean that BP is not pointing to the unwind information. And then is RA at regs[CFI_BP].offset + 8? In the case of aarch64, the saved frame pointer and return address appear in the same order as on x86_64. So that would work. If that can make things simpler for now I can go with that. Thanks for the suggestion. -- Julien Thierry
Re: [PATCH v2 1/3] objtool: check: Fully validate the stack frame
On 9/29/20 8:18 PM, Josh Poimboeuf wrote: On Mon, Sep 28, 2020 at 10:36:29AM +0100, Julien Thierry wrote: +++ b/tools/objtool/arch/x86/include/cfi_regs.h @@ -22,4 +22,7 @@ #define CFI_RA16 #define CFI_NUM_REGS 17 A few more naming nitpicks: +#define STACKFRAME_BP_OFFSET -16 +#define STACKFRAME_RA_OFFSET -8 "Stack frame" has more than one meaning now, I suppose. i.e. it could also include the callee-saved registers and any other stack space allocated by the function. Would "call frame" be clearer? CALL_FRAME_BP_OFFSET CALL_FRAME_RA_OFFSET ? I would've thought that the call-frame could include the stackframe + other callee saved regs. Whereas stackframe tends to used for the caller's frame pointer + return address (i.e. what allows unwinding). Unless I'm getting lost with things. And if call frame is associated with the region starting from the stack pointer at the parent call point (since this is what CFA is), then it shouldn't be associated with the framepointer + return address structure since this could be anywhere on the call frame (not at a fixed offset) as long as the new frame pointer points to the structure. +++ b/tools/objtool/cfi.h @@ -35,4 +35,6 @@ struct cfi_state { bool end; }; +#define STACKFRAME_SIZE 16 CALL_FRAME_SIZE ? I'm sort of contradicting my previous comment here, but even though this value may be generic, it's also very much intertwined with the CALL_FRAME_{BP|RA}_OFFSET values. So I get the feeling it really belongs in the arch-specific cfi_regs.h next to the other defines after all. Agreed. -- Julien Thierry
[PATCH v2 3/3] objtool: check: Make SP memory operation match PUSH/POP semantics
Architectures without PUSH/POP instructions will always access the stack though memory operations (SRC/DEST_INDIRECT). Make those operations have the same effect on the CFA as PUSH/POP, with no stack pointer modification. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 20 1 file changed, 20 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 9f7a14a24a65..724293fd1f59 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2008,6 +2008,14 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; case OP_SRC_REG_INDIRECT: + if (!cfi->drap && op->dest.reg == cfa->base && + op->dest.reg == CFI_BP) { + + /* mov disp(%rsp), %rbp */ + cfa->base = CFI_SP; + cfa->offset = cfi->stack_size; + } + if (cfi->drap && op->src.reg == CFI_BP && op->src.offset == cfi->drap_offset) { @@ -2029,6 +2037,12 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov disp(%rbp), %reg */ /* mov disp(%rsp), %reg */ restore_reg(cfi, op->dest.reg); + + } else if (op->src.reg == CFI_SP && + op->src.offset == regs[op->dest.reg].offset + cfi->stack_size) { + + /* mov disp(%rsp), %reg */ + restore_reg(cfi, op->dest.reg); } break; @@ -2106,6 +2120,12 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov reg, disp(%rsp) */ save_reg(cfi, op->src.reg, CFI_CFA, op->dest.offset - cfi->cfa.offset); + + } else if (op->dest.reg == CFI_SP) { + + /* mov reg, disp(%rsp) */ + save_reg(cfi, op->src.reg, CFI_CFA, +op->dest.offset - cfi->stack_size); } break; -- 2.25.4
[PATCH v2 1/3] objtool: check: Fully validate the stack frame
A valid stack frame should contain both the return address and the previous frame pointer value. On x86, the return value is placed on the stack by the calling instructions. On other architectures, the callee need to explicitly save the return address on the stack. Add the necessary checks to verify a function properly sets up all the elements of the stack frame. Signed-off-by: Julien Thierry --- tools/objtool/arch/x86/include/cfi_regs.h | 3 +++ tools/objtool/cfi.h | 2 ++ tools/objtool/check.c | 21 + 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/tools/objtool/arch/x86/include/cfi_regs.h b/tools/objtool/arch/x86/include/cfi_regs.h index 79bc517efba8..4e443b49d0d3 100644 --- a/tools/objtool/arch/x86/include/cfi_regs.h +++ b/tools/objtool/arch/x86/include/cfi_regs.h @@ -22,4 +22,7 @@ #define CFI_RA 16 #define CFI_NUM_REGS 17 +#define STACKFRAME_BP_OFFSET -16 +#define STACKFRAME_RA_OFFSET -8 + #endif /* _OBJTOOL_CFI_REGS_H */ diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h index c7c59c6a44ee..2691b6ce4fcd 100644 --- a/tools/objtool/cfi.h +++ b/tools/objtool/cfi.h @@ -35,4 +35,6 @@ struct cfi_state { bool end; }; +#define STACKFRAME_SIZE16 + #endif /* _OBJTOOL_CFI_H */ diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 2df9f769412e..50b3a4504db1 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1668,12 +1668,24 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state return false; } +static bool check_reg_frame_pos(const struct cfi_reg *reg, int stackframe_start, + int expected_offset) +{ + return reg->base == CFI_CFA && + reg->offset == stackframe_start + expected_offset; +} + static bool has_valid_stack_frame(struct insn_state *state) { struct cfi_state *cfi = &state->cfi; - if (cfi->cfa.base == CFI_BP && cfi->regs[CFI_BP].base == CFI_CFA && - cfi->regs[CFI_BP].offset == -16) + if (cfi->cfa.base == CFI_BP && cfi->cfa.offset >= STACKFRAME_SIZE && + check_reg_frame_pos(&cfi->regs[CFI_BP], + -cfi->cfa.offset + STACKFRAME_SIZE, + STACKFRAME_BP_OFFSET) && + check_reg_frame_pos(&cfi->regs[CFI_RA], + -cfi->cfa.offset + STACKFRAME_SIZE, + STACKFRAME_RA_OFFSET)) return true; if (cfi->drap && cfi->regs[CFI_BP].base == CFI_BP) @@ -1802,8 +1814,9 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, case OP_SRC_REG: if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP && cfa->base == CFI_SP && - regs[CFI_BP].base == CFI_CFA && - regs[CFI_BP].offset == -cfa->offset) { + check_reg_frame_pos(®s[CFI_BP], + -cfa->offset + STACKFRAME_SIZE, + STACKFRAME_BP_OFFSET)) { /* mov %rsp, %rbp */ cfa->base = op->dest.reg; -- 2.25.4
[PATCH v2 2/3] objtool: check: Support addition to set CFA base
On arm64, the compiler can set the frame pointer either with a move operation or with and add operation like: add (SP + constant), BP For a simple move operation, the CFA base is changed from SP to BP. Handle also changing the CFA base when the frame pointer is set with an addition instruction. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 13 + 1 file changed, 13 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 50b3a4504db1..9f7a14a24a65 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1901,6 +1901,19 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; } + if (!cfi->drap && op->src.reg == CFI_SP && + op->dest.reg == CFI_BP && cfa->base == CFI_SP && + check_reg_frame_pos(®s[CFI_BP], + -cfa->offset + op->src.offset + STACKFRAME_SIZE, + STACKFRAME_BP_OFFSET)) { + + /* lea disp(%rsp), %rbp */ + cfa->base = CFI_BP; + cfa->offset -= op->src.offset; + cfi->bp_scratch = false; + break; + } + if (op->src.reg == CFI_SP && cfa->base == CFI_SP) { /* drap: lea disp(%rsp), %drap */ -- 2.25.4
[PATCH v2 0/3] objtool: Extend CFA updating/checking
Hi, The following patches are the result of limitation found on the CFA management code when trying to validate arm64 frames. I tried to keep things simple and not contradict current CFA management logic nor introduce too many corner cases. Changes since V1[1]: - Minor cleanups/rewording from discussion with Josh [1] https://www.spinics.net/lists/kernel/msg3662146.html Thanks, Julien --> Julien Thierry (3): objtool: check: Fully validate the stack frame objtool: check: Support addition to set CFA base objtool: check: Make SP memory operation match PUSH/POP semantics tools/objtool/arch/x86/include/cfi_regs.h | 3 ++ tools/objtool/cfi.h | 2 + tools/objtool/check.c | 54 +-- 3 files changed, 55 insertions(+), 4 deletions(-) -- 2.25.4
[tip: objtool/core] objtool: Ignore unreachable fake jumps
The following commit has been merged into the objtool/core branch of tip: Commit-ID: fb136219f0e2b417d84e67b2a3adc1f933997d04 Gitweb: https://git.kernel.org/tip/fb136219f0e2b417d84e67b2a3adc1f933997d04 Author:Julien Thierry AuthorDate:Tue, 15 Sep 2020 08:53:17 +01:00 Committer: Josh Poimboeuf CommitterDate: Fri, 18 Sep 2020 12:04:00 -05:00 objtool: Ignore unreachable fake jumps It is possible for alternative code to unconditionally jump out of the alternative region. In such a case, if a fake jump is added at the end of the alternative instructions, the fake jump will never be reached. Since the fake jump is just a mean to make sure code validation does not go beyond the set of alternatives, reaching it is not a requirement. Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/check.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index fd2edab..cd7c669 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2648,6 +2648,9 @@ static bool ignore_unreachable_insn(struct instruction *insn) !strcmp(insn->sec->name, ".altinstr_aux")) return true; + if (insn->type == INSN_JUMP_UNCONDITIONAL && insn->offset == FAKE_JUMP_OFFSET) + return true; + if (!insn->func) return false;
[tip: objtool/core] objtool: Remove useless tests before save_reg()
The following commit has been merged into the objtool/core branch of tip: Commit-ID: f4f803984c3685f416a74e9e2fa7d39bdafbe02b Gitweb: https://git.kernel.org/tip/f4f803984c3685f416a74e9e2fa7d39bdafbe02b Author:Julien Thierry AuthorDate:Tue, 15 Sep 2020 08:53:16 +01:00 Committer: Josh Poimboeuf CommitterDate: Fri, 18 Sep 2020 12:02:27 -05:00 objtool: Remove useless tests before save_reg() save_reg already checks that the register being saved does not already have a saved state. Remove redundant checks before processing a register storing operation. Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/check.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 4e2f703..fd2edab 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2030,7 +2030,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* drap: push %rbp */ cfi->stack_size = 0; - } else if (regs[op->src.reg].base == CFI_UNDEFINED) { + } else { /* drap: push %reg */ save_reg(cfi, op->src.reg, CFI_BP, -cfi->stack_size); @@ -2059,9 +2059,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* save drap offset so we know when to restore it */ cfi->drap_offset = op->dest.offset; - } - - else if (regs[op->src.reg].base == CFI_UNDEFINED) { + } else { /* drap: mov reg, disp(%rbp) */ save_reg(cfi, op->src.reg, CFI_BP, op->dest.offset);
[tip: objtool/core] objtool: Handle calling non-function symbols in other sections
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 2b232a22d8225df419a92ca69ddeeb4e5fe902f7 Gitweb: https://git.kernel.org/tip/2b232a22d8225df419a92ca69ddeeb4e5fe902f7 Author:Julien Thierry AuthorDate:Tue, 15 Sep 2020 08:53:18 +01:00 Committer: Josh Poimboeuf CommitterDate: Mon, 21 Sep 2020 10:17:36 -05:00 objtool: Handle calling non-function symbols in other sections Relocation for a call destination could point to a symbol that has type STT_NOTYPE. Lookup such a symbol when no function is available. Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/check.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index cd7c669..a4796e3 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -815,6 +815,17 @@ static void remove_insn_ops(struct instruction *insn) } } +static struct symbol *find_call_destination(struct section *sec, unsigned long offset) +{ + struct symbol *call_dest; + + call_dest = find_func_by_offset(sec, offset); + if (!call_dest) + call_dest = find_symbol_by_offset(sec, offset); + + return call_dest; +} + /* * Find the destination instructions for all calls. */ @@ -832,9 +843,7 @@ static int add_call_destinations(struct objtool_file *file) insn->offset, insn->len); if (!reloc) { dest_off = arch_jump_destination(insn); - insn->call_dest = find_func_by_offset(insn->sec, dest_off); - if (!insn->call_dest) - insn->call_dest = find_symbol_by_offset(insn->sec, dest_off); + insn->call_dest = find_call_destination(insn->sec, dest_off); if (insn->ignore) continue; @@ -852,8 +861,8 @@ static int add_call_destinations(struct objtool_file *file) } else if (reloc->sym->type == STT_SECTION) { dest_off = arch_dest_reloc_offset(reloc->addend); - insn->call_dest = find_func_by_offset(reloc->sym->sec, - dest_off); + insn->call_dest = find_call_destination(reloc->sym->sec, + dest_off); if (!insn->call_dest) { WARN_FUNC("can't find call dest symbol at %s+0x%lx", insn->sec, insn->offset,
Re: [PATCH 1/3] objtool: check: Fully validate the stack frame
On 9/18/20 9:56 PM, Josh Poimboeuf wrote: On Tue, Sep 15, 2020 at 09:12:02AM +0100, Julien Thierry wrote: A valid stack frame should contain both the return address and the previous frame pointer value. On x86, the return value is placed on the stack by the calling instructions. On other architectures, the callee need to explicitly save the return value on the stack. s/return value/return address/g Add the necessary checks to verify a function properly sets up the all s/the all/all the/ the elements of the stack frame. Signed-off-by: Julien Thierry --- tools/objtool/arch/x86/include/cfi_regs.h | 4 tools/objtool/check.c | 17 + 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/tools/objtool/arch/x86/include/cfi_regs.h b/tools/objtool/arch/x86/include/cfi_regs.h index 79bc517efba8..19b75b8b8439 100644 --- a/tools/objtool/arch/x86/include/cfi_regs.h +++ b/tools/objtool/arch/x86/include/cfi_regs.h @@ -22,4 +22,8 @@ #define CFI_RA16 #define CFI_NUM_REGS 17 +#define CFA_SIZE 16 If I remember correctly, CFA (stolen from DWARF) is "Caller Frame Address". It's the stack address of the caller, before the call. Ok, so maybe I'm mixing Call Frame and Stack Frame (frame pointer + return address). I get the feeling CFA_SIZE is the wrong name, because CFA is an address, and its size isn't 16 bytes. But I'm not quite sure what this is supposed to represent. Is it supposed to be the size of the frame pointer + return address? Isn't that always going to be 16 bytes for both arches? For arm64 and x86_64 it is. Maybe it is a bit early to consider it might differ for other arches (e.g. 32bit arches?). +#define CFA_BP_OFFSET -16 +#define CFA_RA_OFFSET -8 + #endif /* _OBJTOOL_CFI_REGS_H */ diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 500f63b3dcff..7db6761d28c2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1669,12 +1669,20 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state return false; } +static bool check_reg_frame_pos(const struct cfi_reg *reg, int cfa_start, + int expected_offset) +{ + return reg->base == CFI_CFA && + reg->offset == cfa_start + expected_offset; +} + static bool has_valid_stack_frame(struct insn_state *state) { struct cfi_state *cfi = &state->cfi; - if (cfi->cfa.base == CFI_BP && cfi->regs[CFI_BP].base == CFI_CFA && - cfi->regs[CFI_BP].offset == -16) + if (cfi->cfa.base == CFI_BP && cfi->cfa.offset >= CFA_SIZE && Why '>=' rather than '=='? Because on arm64 the stack frame is not necessarily the first thing put on the stack by the callee. The callee is free to create the stack frame where it wants (on its part of the stack of course) as long as it appropriately sets the frame pointer before making a call. You could have something like: ++ || || ++> f1() called || | some callee| | saved regs | || ++ | RA | ++ | BP/FP | ++> Set new BP/FP value + check_reg_frame_pos(&cfi->regs[CFI_BP], -cfi->cfa.offset + CFA_SIZE, CFA_BP_OFFSET) && + check_reg_frame_pos(&cfi->regs[CFI_RA], -cfi->cfa.offset + CFA_SIZE, CFA_RA_OFFSET)) Isn't '-cfi->cfa.offset + CFA_SIZE' always going to be zero? For x86 yes, for arm64 it cfa.offset can be > 16. -- Julien Thierry
Re: [PATCH 3/3] objtool: check: Make SP memory operation match PUSH/POP semantics
On 9/18/20 10:43 PM, Josh Poimboeuf wrote: On Tue, Sep 15, 2020 at 09:12:04AM +0100, Julien Thierry wrote: Architectures without PUSH/POP instructions will always access the stack though memory operations (SRC/DEST_INDIRECT). Make those operations have the same effect on the CFA as PUSH/POP, with no stack pointer modification. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 17 + 1 file changed, 17 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index f45991c2db41..7ff87fa3caec 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2005,6 +2005,13 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; case OP_SRC_REG_INDIRECT: + if (!cfi->drap && op->dest.reg == cfa->base) { && op->dest.reg == CFI_BP ? Does it matter? My unstandig was that the register used to point to the CFA is getting overwritten, so we need to fallback to something known which is the offset from the stack pointer. Was that not the case? + + /* mov disp(%rsp), %rbp */ + cfa->base = CFI_SP; + cfa->offset = cfi->stack_size; + } + if (cfi->drap && op->src.reg == CFI_BP && op->src.offset == cfi->drap_offset) { @@ -2026,6 +2033,11 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov disp(%rbp), %reg */ /* mov disp(%rsp), %reg */ restore_reg(cfi, op->dest.reg); + } else if (op->src.reg == CFI_SP && An empty line above the else would help readability. + op->src.offset == regs[op->dest.reg].offset + cfi->stack_size) { + + /* mov disp(%rsp), %reg */ + restore_reg(cfi, op->dest.reg); } break; @@ -2103,6 +2115,11 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov reg, disp(%rsp) */ save_reg(cfi, op->src.reg, CFI_CFA, op->dest.offset - cfi->cfa.offset); + } else if (op->dest.reg == CFI_SP) { Same here. I'll add those. + + /* mov reg, disp(%rsp) */ + save_reg(cfi, op->src.reg, CFI_CFA, + op->dest.offset - cfi->stack_size); } break; -- 2.21.3 -- Julien Thierry
Re: [PATCH 3/3] objtool: check: Handle calling non-function symbols in other sections
On 9/18/20 9:07 PM, Josh Poimboeuf wrote: On Tue, Sep 15, 2020 at 08:53:18AM +0100, Julien Thierry wrote: Relocation for a call destination could point to a symbol that has type STT_NOTYPE. Then shouldn't the callee be changed to STT_FUNC? Not if it's a code symbol that does not follow standard calling convention. It's really the same case as the !reloc, except this time it's in a different .text section. In arm64 there are different sections that are used (.text for basic code, .idmap.text for code mapped in a manner where virtual address == physical address, .hyp.text for kvm priviledged code, .tramp.text for trampolines...). There aren't many cases, but some symbols reference symbols in other sections, but the symbol being called isn't a proper function. -- Julien Thierry
[PATCH 0/3] objtool: Extend CFA updating/checking
Hi, The following patches are the result of limitation found on the CFA management code when trying to validate arm64 frames. I tried to keep things simple and not contradict current CFA management logic nor introduce too many corner cases. The patches apply on top of the cleanup series[1] I sent previously. [1] https://lkml.org/lkml/2020/9/15/199 Thanks, Julien --> Julien Thierry (3): objtool: check: Fully validate the stack frame objtool: check: Support addition to set CFA base objtool: check: Make SP memory operation match PUSH/POP semantics tools/objtool/arch/x86/include/cfi_regs.h | 4 ++ tools/objtool/check.c | 47 +-- 2 files changed, 47 insertions(+), 4 deletions(-) -- 2.21.3
[PATCH 1/3] objtool: check: Fully validate the stack frame
A valid stack frame should contain both the return address and the previous frame pointer value. On x86, the return value is placed on the stack by the calling instructions. On other architectures, the callee need to explicitly save the return value on the stack. Add the necessary checks to verify a function properly sets up the all the elements of the stack frame. Signed-off-by: Julien Thierry --- tools/objtool/arch/x86/include/cfi_regs.h | 4 tools/objtool/check.c | 17 + 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/tools/objtool/arch/x86/include/cfi_regs.h b/tools/objtool/arch/x86/include/cfi_regs.h index 79bc517efba8..19b75b8b8439 100644 --- a/tools/objtool/arch/x86/include/cfi_regs.h +++ b/tools/objtool/arch/x86/include/cfi_regs.h @@ -22,4 +22,8 @@ #define CFI_RA 16 #define CFI_NUM_REGS 17 +#define CFA_SIZE 16 +#define CFA_BP_OFFSET -16 +#define CFA_RA_OFFSET -8 + #endif /* _OBJTOOL_CFI_REGS_H */ diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 500f63b3dcff..7db6761d28c2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1669,12 +1669,20 @@ static bool has_modified_stack_frame(struct instruction *insn, struct insn_state return false; } +static bool check_reg_frame_pos(const struct cfi_reg *reg, int cfa_start, + int expected_offset) +{ + return reg->base == CFI_CFA && + reg->offset == cfa_start + expected_offset; +} + static bool has_valid_stack_frame(struct insn_state *state) { struct cfi_state *cfi = &state->cfi; - if (cfi->cfa.base == CFI_BP && cfi->regs[CFI_BP].base == CFI_CFA && - cfi->regs[CFI_BP].offset == -16) + if (cfi->cfa.base == CFI_BP && cfi->cfa.offset >= CFA_SIZE && + check_reg_frame_pos(&cfi->regs[CFI_BP], -cfi->cfa.offset + CFA_SIZE, CFA_BP_OFFSET) && + check_reg_frame_pos(&cfi->regs[CFI_RA], -cfi->cfa.offset + CFA_SIZE, CFA_RA_OFFSET)) return true; if (cfi->drap && cfi->regs[CFI_BP].base == CFI_BP) @@ -1803,8 +1811,9 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, case OP_SRC_REG: if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP && cfa->base == CFI_SP && - regs[CFI_BP].base == CFI_CFA && - regs[CFI_BP].offset == -cfa->offset) { + check_reg_frame_pos(®s[CFI_BP], + -cfa->offset + CFA_SIZE, + CFA_BP_OFFSET)) { /* mov %rsp, %rbp */ cfa->base = op->dest.reg; -- 2.21.3
[PATCH 3/3] objtool: check: Make SP memory operation match PUSH/POP semantics
Architectures without PUSH/POP instructions will always access the stack though memory operations (SRC/DEST_INDIRECT). Make those operations have the same effect on the CFA as PUSH/POP, with no stack pointer modification. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 17 + 1 file changed, 17 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index f45991c2db41..7ff87fa3caec 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2005,6 +2005,13 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; case OP_SRC_REG_INDIRECT: + if (!cfi->drap && op->dest.reg == cfa->base) { + + /* mov disp(%rsp), %rbp */ + cfa->base = CFI_SP; + cfa->offset = cfi->stack_size; + } + if (cfi->drap && op->src.reg == CFI_BP && op->src.offset == cfi->drap_offset) { @@ -2026,6 +2033,11 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov disp(%rbp), %reg */ /* mov disp(%rsp), %reg */ restore_reg(cfi, op->dest.reg); + } else if (op->src.reg == CFI_SP && + op->src.offset == regs[op->dest.reg].offset + cfi->stack_size) { + + /* mov disp(%rsp), %reg */ + restore_reg(cfi, op->dest.reg); } break; @@ -2103,6 +2115,11 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* mov reg, disp(%rsp) */ save_reg(cfi, op->src.reg, CFI_CFA, op->dest.offset - cfi->cfa.offset); + } else if (op->dest.reg == CFI_SP) { + + /* mov reg, disp(%rsp) */ + save_reg(cfi, op->src.reg, CFI_CFA, +op->dest.offset - cfi->stack_size); } break; -- 2.21.3
[PATCH 2/3] objtool: check: Support addition to set CFA base
Instead of "mov SP, BP", a compiler could simply set BP to SP + constant. Handle changing the CFA base in such cases. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 13 + 1 file changed, 13 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 7db6761d28c2..f45991c2db41 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1898,6 +1898,19 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, break; } + if (!cfi->drap && op->src.reg == CFI_SP && + op->dest.reg == CFI_BP && cfa->base == CFI_SP && + check_reg_frame_pos(®s[CFI_BP], + -cfa->offset + op->src.offset + CFA_SIZE, + CFA_BP_OFFSET)) { + + /* lea disp(%rsp), %rbp */ + cfa->base = CFI_BP; + cfa->offset -= op->src.offset; + cfi->bp_scratch = false; + break; + } + if (op->src.reg == CFI_SP && cfa->base == CFI_SP) { /* drap: lea disp(%rsp), %drap */ -- 2.21.3
[PATCH 0/3] objtool: Miscellaneous cleanup/fixes
Hi, These patches provide some simple cleanup or lift small limitations found while working on the arm64 port. They should apply on current tip/objtool/core branch Cheers, Julien --> Julien Thierry (3): objtool: check: Remove useless tests objtool: check: Ignore unreachable fake jumps objtool: check: Handle calling non-function symbols in other sections tools/objtool/check.c | 29 - 1 file changed, 20 insertions(+), 9 deletions(-) -- 2.21.3
[PATCH 2/3] objtool: check: Ignore unreachable fake jumps
It is possible for alternative code to unconditionally jump out of the alternative region. In such a case, if a fake jump is added at the end of the alternative instructions, the fake jump will never be reached. Since the fake jump is just a mean to make sure code validation does not go beyond the set of alternatives, reaching it is not a requirement. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index fd2edab8e672..cd7c6698d316 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2648,6 +2648,9 @@ static bool ignore_unreachable_insn(struct instruction *insn) !strcmp(insn->sec->name, ".altinstr_aux")) return true; + if (insn->type == INSN_JUMP_UNCONDITIONAL && insn->offset == FAKE_JUMP_OFFSET) + return true; + if (!insn->func) return false; -- 2.21.3
[PATCH 3/3] objtool: check: Handle calling non-function symbols in other sections
Relocation for a call destination could point to a symbol that has type STT_NOTYPE. Lookup such a symbol when no function is available. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index cd7c6698d316..500f63b3dcff 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -815,6 +815,17 @@ static void remove_insn_ops(struct instruction *insn) } } +static struct symbol *find_call_destination(struct section *sec, unsigned long offset) +{ + struct symbol *call_dest; + + call_dest = find_func_by_offset(sec, offset); + if (!call_dest) + call_dest = find_symbol_by_offset(sec, offset); + + return call_dest; +} + /* * Find the destination instructions for all calls. */ @@ -832,9 +843,7 @@ static int add_call_destinations(struct objtool_file *file) insn->offset, insn->len); if (!reloc) { dest_off = arch_jump_destination(insn); - insn->call_dest = find_func_by_offset(insn->sec, dest_off); - if (!insn->call_dest) - insn->call_dest = find_symbol_by_offset(insn->sec, dest_off); + insn->call_dest = find_call_destination(insn->sec, dest_off); if (insn->ignore) continue; @@ -852,8 +861,9 @@ static int add_call_destinations(struct objtool_file *file) } else if (reloc->sym->type == STT_SECTION) { dest_off = arch_dest_reloc_offset(reloc->addend); - insn->call_dest = find_func_by_offset(reloc->sym->sec, - dest_off); + insn->call_dest = find_call_destination(reloc->sym->sec, + dest_off); + if (!insn->call_dest) { WARN_FUNC("can't find call dest symbol at %s+0x%lx", insn->sec, insn->offset, -- 2.21.3
[PATCH 1/3] objtool: check: Remove useless tests
save_reg already checks that the register being saved does not already have a saved state. Remove redundant checks before processing a register storing operation. Signed-off-by: Julien Thierry --- tools/objtool/check.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 4e2f703b6a25..fd2edab8e672 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2030,7 +2030,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* drap: push %rbp */ cfi->stack_size = 0; - } else if (regs[op->src.reg].base == CFI_UNDEFINED) { + } else { /* drap: push %reg */ save_reg(cfi, op->src.reg, CFI_BP, -cfi->stack_size); @@ -2059,9 +2059,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi, /* save drap offset so we know when to restore it */ cfi->drap_offset = op->dest.offset; - } - - else if (regs[op->src.reg].base == CFI_UNDEFINED) { + } else { /* drap: mov reg, disp(%rbp) */ save_reg(cfi, op->src.reg, CFI_BP, op->dest.offset); -- 2.21.3
[tip: objtool/core] objtool: Decode unwind hint register depending on architecture
The following commit has been merged into the objtool/core branch of tip: Commit-ID: edea9e6bcbeaa41718b022a8b99ffddef2330bbc Gitweb: https://git.kernel.org/tip/edea9e6bcbeaa41718b022a8b99ffddef2330bbc Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:28 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Decode unwind hint register depending on architecture The set of registers that can be included in an unwind hint and their encoding will depend on the architecture. Have arch specific code to decode that register. Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/arch.h| 2 ++- tools/objtool/arch/x86/decode.c | 37 - tools/objtool/check.c | 27 +--- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index b18c5f6..4a84c30 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -88,4 +88,6 @@ unsigned long arch_dest_reloc_offset(int addend); const char *arch_nop_insn(int len); +int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg); + #endif /* _ARCH_H */ diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 1967370..cde9c36 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -15,6 +15,7 @@ #include "../../elf.h" #include "../../arch.h" #include "../../warn.h" +#include static unsigned char op_to_cfi_reg[][2] = { {CFI_AX, CFI_R8}, @@ -583,3 +584,39 @@ const char *arch_nop_insn(int len) return nops[len-1]; } + +int arch_decode_hint_reg(struct instruction *insn, u8 sp_reg) +{ + struct cfi_reg *cfa = &insn->cfi.cfa; + + switch (sp_reg) { + case ORC_REG_UNDEFINED: + cfa->base = CFI_UNDEFINED; + break; + case ORC_REG_SP: + cfa->base = CFI_SP; + break; + case ORC_REG_BP: + cfa->base = CFI_BP; + break; + case ORC_REG_SP_INDIRECT: + cfa->base = CFI_SP_INDIRECT; + break; + case ORC_REG_R10: + cfa->base = CFI_R10; + break; + case ORC_REG_R13: + cfa->base = CFI_R13; + break; + case ORC_REG_DI: + cfa->base = CFI_DI; + break; + case ORC_REG_DX: + cfa->base = CFI_DX; + break; + default: + return -1; + } + + return 0; +} diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 95c6e0d..4e2f703 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1367,32 +1367,7 @@ static int read_unwind_hints(struct objtool_file *file) insn->hint = true; - switch (hint->sp_reg) { - case ORC_REG_UNDEFINED: - cfa->base = CFI_UNDEFINED; - break; - case ORC_REG_SP: - cfa->base = CFI_SP; - break; - case ORC_REG_BP: - cfa->base = CFI_BP; - break; - case ORC_REG_SP_INDIRECT: - cfa->base = CFI_SP_INDIRECT; - break; - case ORC_REG_R10: - cfa->base = CFI_R10; - break; - case ORC_REG_R13: - cfa->base = CFI_R13; - break; - case ORC_REG_DI: - cfa->base = CFI_DI; - break; - case ORC_REG_DX: - cfa->base = CFI_DX; - break; - default: + if (arch_decode_hint_reg(insn, hint->sp_reg)) { WARN_FUNC("unsupported unwind_hint sp base reg %d", insn->sec, insn->offset, hint->sp_reg); return -1;
[tip: objtool/core] objtool: Only include valid definitions depending on source file type
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 5567c6c39f3404e4492c18c0c1abff5556684f6e Gitweb: https://git.kernel.org/tip/5567c6c39f3404e4492c18c0c1abff5556684f6e Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:26 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Only include valid definitions depending on source file type Header include/linux/objtool.h contains both C and assembly definition that are visible regardless of the file including them. Place definition under conditional __ASSEMBLY__. Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- include/linux/objtool.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/linux/objtool.h b/include/linux/objtool.h index 358175c..15e9997 100644 --- a/include/linux/objtool.h +++ b/include/linux/objtool.h @@ -3,6 +3,8 @@ #define _LINUX_OBJTOOL_H #ifdef CONFIG_STACK_VALIDATION + +#ifndef __ASSEMBLY__ /* * This macro marks the given function's stack frame as "non-standard", which * tells objtool to ignore the function when doing stack metadata validation. @@ -15,6 +17,8 @@ static void __used __section(.discard.func_stack_frame_non_standard) \ *__func_stack_frame_non_standard_##func = func +#else /* __ASSEMBLY__ */ + /* * This macro indicates that the following intra-function call is valid. * Any non-annotated intra-function call will cause objtool to issue a warning. @@ -25,6 +29,8 @@ .long 999b; \ .popsection; +#endif /* __ASSEMBLY__ */ + #else /* !CONFIG_STACK_VALIDATION */ #define STACK_FRAME_NON_STANDARD(func)
[tip: objtool/core] objtool: Move ORC logic out of check()
The following commit has been merged into the objtool/core branch of tip: Commit-ID: d44becb9decf4438d1e555b1428634964d2e5764 Gitweb: https://git.kernel.org/tip/d44becb9decf4438d1e555b1428634964d2e5764 Author:Julien Thierry AuthorDate:Tue, 25 Aug 2020 13:47:40 +01:00 Committer: Josh Poimboeuf CommitterDate: Tue, 01 Sep 2020 17:19:11 -05:00 objtool: Move ORC logic out of check() Now that the objtool_file can be obtained outside of the check function, orc generation builtin no longer requires check to explicitly call its orc related functions. Signed-off-by: Julien Thierry Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf --- tools/objtool/builtin-check.c | 10 +- tools/objtool/builtin-orc.c | 21 - tools/objtool/check.c | 18 +- tools/objtool/objtool.h | 2 +- tools/objtool/weak.c | 2 +- 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index 0126ec3..c6d199b 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -42,6 +42,7 @@ int cmd_check(int argc, const char **argv) { const char *objname, *s; struct objtool_file *file; + int ret; argc = parse_options(argc, argv, check_options, check_usage, 0); @@ -58,5 +59,12 @@ int cmd_check(int argc, const char **argv) if (!file) return 1; - return check(file, false); + ret = check(file); + if (ret) + return ret; + + if (file->elf->changed) + return elf_write(file->elf); + + return 0; } diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c index 3979f27..7b31121 100644 --- a/tools/objtool/builtin-orc.c +++ b/tools/objtool/builtin-orc.c @@ -32,6 +32,7 @@ int cmd_orc(int argc, const char **argv) if (!strncmp(argv[0], "gen", 3)) { struct objtool_file *file; + int ret; argc = parse_options(argc, argv, check_options, orc_usage, 0); if (argc != 1) @@ -43,7 +44,25 @@ int cmd_orc(int argc, const char **argv) if (!file) return 1; - return check(file, true); + ret = check(file); + if (ret) + return ret; + + if (list_empty(&file->insn_list)) + return 0; + + ret = create_orc(file); + if (ret) + return ret; + + ret = create_orc_sections(file); + if (ret) + return ret; + + if (!file->elf->changed) + return 0; + + return elf_write(file->elf); } if (!strcmp(argv[0], "dump")) { diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 9d4efa3..4afc2d5 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -2908,7 +2908,7 @@ static int validate_reachable_instructions(struct objtool_file *file) return 0; } -int check(struct objtool_file *file, bool orc) +int check(struct objtool_file *file) { int ret, warnings = 0; @@ -2960,22 +2960,6 @@ int check(struct objtool_file *file, bool orc) goto out; warnings += ret; - if (orc) { - ret = create_orc(file); - if (ret < 0) - goto out; - - ret = create_orc_sections(file); - if (ret < 0) - goto out; - } - - if (file->elf->changed) { - ret = elf_write(file->elf); - if (ret < 0) - goto out; - } - out: if (ret < 0) { /* diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h index 7efc43f..a635f68 100644 --- a/tools/objtool/objtool.h +++ b/tools/objtool/objtool.h @@ -22,7 +22,7 @@ struct objtool_file { struct objtool_file *objtool_open_read(const char *_objname); -int check(struct objtool_file *file, bool orc); +int check(struct objtool_file *file); int orc_dump(const char *objname); int create_orc(struct objtool_file *file); int create_orc_sections(struct objtool_file *file); diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c index 8269831..29180d5 100644 --- a/tools/objtool/weak.c +++ b/tools/objtool/weak.c @@ -17,7 +17,7 @@ return ENOSYS; \ }) -int __weak check(struct objtool_file *file, bool orc) +int __weak check(struct objtool_file *file) { UNSUPPORTED("check subcommand"); }
[tip: objtool/core] objtool: Move object file loading out of check()
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 6545eb030e6f14cef8793a86312483c788eaee46 Gitweb: https://git.kernel.org/tip/6545eb030e6f14cef8793a86312483c788eaee46 Author:Julien Thierry AuthorDate:Tue, 25 Aug 2020 13:47:39 +01:00 Committer: Josh Poimboeuf CommitterDate: Tue, 01 Sep 2020 17:19:07 -05:00 objtool: Move object file loading out of check() Structure objtool_file can be used by different subcommands. In fact it already is, by check and orc. Provide a function that allows to initialize objtool_file, that builtin can call, without relying on check to do the correct setup for them and explicitly hand the objtool_file to them. Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/builtin-check.c | 7 +- tools/objtool/builtin-orc.c | 8 +- tools/objtool/check.c | 42 ++ tools/objtool/objtool.c | 30 - tools/objtool/objtool.h | 4 ++- tools/objtool/weak.c | 4 +--- 6 files changed, 60 insertions(+), 35 deletions(-) diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index 7a44174..0126ec3 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -41,6 +41,7 @@ const struct option check_options[] = { int cmd_check(int argc, const char **argv) { const char *objname, *s; + struct objtool_file *file; argc = parse_options(argc, argv, check_options, check_usage, 0); @@ -53,5 +54,9 @@ int cmd_check(int argc, const char **argv) if (s && !s[9]) vmlinux = true; - return check(objname, false); + file = objtool_open_read(objname); + if (!file) + return 1; + + return check(file, false); } diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c index b1dfe20..3979f27 100644 --- a/tools/objtool/builtin-orc.c +++ b/tools/objtool/builtin-orc.c @@ -31,13 +31,19 @@ int cmd_orc(int argc, const char **argv) usage_with_options(orc_usage, check_options); if (!strncmp(argv[0], "gen", 3)) { + struct objtool_file *file; + argc = parse_options(argc, argv, check_options, orc_usage, 0); if (argc != 1) usage_with_options(orc_usage, check_options); objname = argv[0]; - return check(objname, true); + file = objtool_open_read(objname); + if (!file) + return 1; + + return check(file, true); } if (!strcmp(argv[0], "dump")) { diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 75d0cd2..9d4efa3 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -28,7 +28,6 @@ struct alternative { bool skip_orig; }; -const char *objname; struct cfi_init_state initial_func_cfi; struct instruction *find_insn(struct objtool_file *file, @@ -2909,37 +2908,22 @@ static int validate_reachable_instructions(struct objtool_file *file) return 0; } -static struct objtool_file file; - -int check(const char *_objname, bool orc) +int check(struct objtool_file *file, bool orc) { int ret, warnings = 0; - objname = _objname; - - file.elf = elf_open_read(objname, O_RDWR); - if (!file.elf) - return 1; - - INIT_LIST_HEAD(&file.insn_list); - hash_init(file.insn_hash); - INIT_LIST_HEAD(&file.static_call_list); - file.c_file = !vmlinux && find_section_by_name(file.elf, ".comment"); - file.ignore_unreachables = no_unreachable; - file.hints = false; - arch_initial_func_cfi_state(&initial_func_cfi); - ret = decode_sections(&file); + ret = decode_sections(file); if (ret < 0) goto out; warnings += ret; - if (list_empty(&file.insn_list)) + if (list_empty(&file->insn_list)) goto out; if (vmlinux && !validate_dup) { - ret = validate_vmlinux_functions(&file); + ret = validate_vmlinux_functions(file); if (ret < 0) goto out; @@ -2948,46 +2932,46 @@ int check(const char *_objname, bool orc) } if (retpoline) { - ret = validate_retpoline(&file); + ret = validate_retpoline(file); if (ret < 0) return ret; warnings += ret; } - ret = validate_functions(&file); + ret = validate_functions(file); if (ret < 0) goto out; warnings += ret; - ret = validate_unwind_hints(&file, NULL); + ret = validate_unwind_hints(file, NULL); i
[tip: objtool/core] objtool: Make sync-check consider the target architecture
The following commit has been merged into the objtool/core branch of tip: Commit-ID: bb090fdb70ecc51c91e1d86345adae064caa06c8 Gitweb: https://git.kernel.org/tip/bb090fdb70ecc51c91e1d86345adae064caa06c8 Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:20 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Make sync-check consider the target architecture Do not take into account outdated headers unrelated to the build of the current architecture. [ jpoimboe: use $SRCARCH directly ] Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/sync-check.sh | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh index b5f5266..cea1c12 100755 --- a/tools/objtool/sync-check.sh +++ b/tools/objtool/sync-check.sh @@ -1,6 +1,12 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 +if [ -z "$SRCARCH" ]; then + echo 'sync-check.sh: error: missing $SRCARCH environment variable' >&2 + exit 1 +fi + +if [ "$SRCARCH" = "x86" ]; then FILES=" arch/x86/include/asm/inat_types.h arch/x86/include/asm/orc_types.h @@ -13,6 +19,7 @@ arch/x86/include/asm/insn.h -I '^#include [\"<]\(asm/\)*inat.h[\">]' arch/x86/lib/inat.c -I '^#include [\"<]\(../include/\)*asm/insn.h[\">]' arch/x86/lib/insn.c -I '^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]' -I '^#include [\"<]\(../include/\)*asm/emulate_prefix.h[\">]' " +fi check_2 () { file1=$1
[tip: objtool/core] objtool: Skip ORC entry creation for non-text sections
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 3eaecac88a17f7fdf29561a197dc728f7f697c60 Gitweb: https://git.kernel.org/tip/3eaecac88a17f7fdf29561a197dc728f7f697c60 Author:Julien Thierry AuthorDate:Tue, 25 Aug 2020 13:47:41 +01:00 Committer: Josh Poimboeuf CommitterDate: Tue, 01 Sep 2020 17:19:11 -05:00 objtool: Skip ORC entry creation for non-text sections Orc generation is only done for text sections, but some instructions can be found in non-text sections (e.g. .discard.text sections). Skip setting their orc sections since their whole sections will be skipped for orc generation. Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/orc_gen.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c index e6b2363..22fe439 100644 --- a/tools/objtool/orc_gen.c +++ b/tools/objtool/orc_gen.c @@ -18,6 +18,9 @@ int create_orc(struct objtool_file *file) struct cfi_reg *cfa = &insn->cfi.cfa; struct cfi_reg *bp = &insn->cfi.regs[CFI_BP]; + if (!insn->sec->text) + continue; + orc->end = insn->cfi.end; if (cfa->base == CFI_UNDEFINED) {
[tip: objtool/core] objtool: Group headers to check in a single list
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 3890b8d92710af75baedf291832cf40193b33454 Gitweb: https://git.kernel.org/tip/3890b8d92710af75baedf291832cf40193b33454 Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:19 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Group headers to check in a single list In order to support multiple architectures and potentially different sets of headers to compare against their kernel equivalent, it is simpler to have all headers to check in a single list. Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/sync-check.sh | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh index aa099b2..b5f5266 100755 --- a/tools/objtool/sync-check.sh +++ b/tools/objtool/sync-check.sh @@ -1,14 +1,18 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 -FILES=' +FILES=" arch/x86/include/asm/inat_types.h arch/x86/include/asm/orc_types.h arch/x86/include/asm/emulate_prefix.h arch/x86/lib/x86-opcode-map.txt arch/x86/tools/gen-insn-attr-x86.awk include/linux/static_call_types.h -' +arch/x86/include/asm/inat.h -I '^#include [\"<]\(asm/\)*inat_types.h[\">]' +arch/x86/include/asm/insn.h -I '^#include [\"<]\(asm/\)*inat.h[\">]' +arch/x86/lib/inat.c -I '^#include [\"<]\(../include/\)*asm/insn.h[\">]' +arch/x86/lib/insn.c -I '^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]' -I '^#include [\"<]\(../include/\)*asm/emulate_prefix.h[\">]' +" check_2 () { file1=$1 @@ -41,11 +45,12 @@ fi cd ../.. -for i in $FILES; do - check $i -done +while read -r file_entry; do +if [ -z "$file_entry" ]; then + continue +fi -check arch/x86/include/asm/inat.h '-I "^#include [\"<]\(asm/\)*inat_types.h[\">]"' -check arch/x86/include/asm/insn.h '-I "^#include [\"<]\(asm/\)*inat.h[\">]"' -check arch/x86/lib/inat.c '-I "^#include [\"<]\(../include/\)*asm/insn.h[\">]"' -check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/emulate_prefix.h[\">]"' +check $file_entry +done <
[tip: objtool/core] objtool: Define 'struct orc_entry' only when needed
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 66734e32463bd1346466f92662feeaccef26e94f Gitweb: https://git.kernel.org/tip/66734e32463bd1346466f92662feeaccef26e94f Author:Julien Thierry AuthorDate:Tue, 25 Aug 2020 13:47:42 +01:00 Committer: Josh Poimboeuf CommitterDate: Tue, 01 Sep 2020 17:19:12 -05:00 objtool: Define 'struct orc_entry' only when needed Implementation of ORC requires some definitions that are currently provided by the target architecture headers. Do not depend on these definitions when the orc subcommand is not implemented. This avoid requiring arches with no orc implementation to provide dummy orc definitions. Signed-off-by: Julien Thierry Reviewed-by: Miroslav Benes Signed-off-by: Josh Poimboeuf --- tools/objtool/Makefile | 4 tools/objtool/arch.h | 2 ++ tools/objtool/check.h | 2 ++ 3 files changed, 8 insertions(+) diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index 7770edc..33d1e3c 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -55,6 +55,10 @@ ifeq ($(SRCARCH),x86) SUBCMD_ORC := y endif +ifeq ($(SUBCMD_ORC),y) + CFLAGS += -DINSN_USE_ORC +endif + export SUBCMD_CHECK SUBCMD_ORC export srctree OUTPUT CFLAGS SRCARCH AWK include $(srctree)/tools/build/Makefile.include diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h index 2e2ce08..b18c5f6 100644 --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -11,7 +11,9 @@ #include "objtool.h" #include "cfi.h" +#ifdef INSN_USE_ORC #include +#endif enum insn_type { INSN_JUMP_CONDITIONAL, diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 36d38b9..6cac345 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -43,7 +43,9 @@ struct instruction { struct symbol *func; struct list_head stack_ops; struct cfi_state cfi; +#ifdef INSN_USE_ORC struct orc_entry orc; +#endif }; struct instruction *find_insn(struct objtool_file *file,
[tip: objtool/core] objtool: Move macros describing structures to arch-dependent code
The following commit has been merged into the objtool/core branch of tip: Commit-ID: c8ea0d672521ef663f0f9a77faa94d0d47102d77 Gitweb: https://git.kernel.org/tip/c8ea0d672521ef663f0f9a77faa94d0d47102d77 Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:21 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Move macros describing structures to arch-dependent code Some macros are defined to describe the size and layout of structures exception_table_entry, jump_entry and alt_instr. These values can vary from one architecture to another. Have the values be defined by arch specific code. Suggested-by: Raphael Gault Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/arch/x86/include/arch_special.h | 20 ++- tools/objtool/special.c | 16 +-- 2 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 tools/objtool/arch/x86/include/arch_special.h diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h new file mode 100644 index 000..d818b2b --- /dev/null +++ b/tools/objtool/arch/x86/include/arch_special.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#ifndef _X86_ARCH_SPECIAL_H +#define _X86_ARCH_SPECIAL_H + +#define EX_ENTRY_SIZE 12 +#define EX_ORIG_OFFSET 0 +#define EX_NEW_OFFSET 4 + +#define JUMP_ENTRY_SIZE16 +#define JUMP_ORIG_OFFSET 0 +#define JUMP_NEW_OFFSET4 + +#define ALT_ENTRY_SIZE 13 +#define ALT_ORIG_OFFSET0 +#define ALT_NEW_OFFSET 4 +#define ALT_FEATURE_OFFSET 8 +#define ALT_ORIG_LEN_OFFSET10 +#define ALT_NEW_LEN_OFFSET 11 + +#endif /* _X86_ARCH_SPECIAL_H */ diff --git a/tools/objtool/special.c b/tools/objtool/special.c index e893f1e..b04f395 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -14,21 +14,7 @@ #include "builtin.h" #include "special.h" #include "warn.h" - -#define EX_ENTRY_SIZE 12 -#define EX_ORIG_OFFSET 0 -#define EX_NEW_OFFSET 4 - -#define JUMP_ENTRY_SIZE16 -#define JUMP_ORIG_OFFSET 0 -#define JUMP_NEW_OFFSET4 - -#define ALT_ENTRY_SIZE 13 -#define ALT_ORIG_OFFSET0 -#define ALT_NEW_OFFSET 4 -#define ALT_FEATURE_OFFSET 8 -#define ALT_ORIG_LEN_OFFSET10 -#define ALT_NEW_LEN_OFFSET 11 +#include "arch_special.h" #define X86_FEATURE_POPCNT (4*32+23) #define X86_FEATURE_SMAP (9*32+20)
[tip: objtool/core] objtool: Abstract alternative special case handling
The following commit has been merged into the objtool/core branch of tip: Commit-ID: eda3dc905834dc9c99132f987f77b68cf53a8682 Gitweb: https://git.kernel.org/tip/eda3dc905834dc9c99132f987f77b68cf53a8682 Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:22 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Abstract alternative special case handling Some alternatives associated with a specific feature need to be treated in a special way. Since the features and how to treat them vary from one architecture to another, move the special case handling to arch specific code. Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/arch/x86/Build | 1 +- tools/objtool/arch/x86/special.c | 37 +++- tools/objtool/objtool.h | 2 ++- tools/objtool/special.c | 32 --- tools/objtool/special.h | 2 ++- tools/objtool/weak.c | 2 +-- 6 files changed, 47 insertions(+), 29 deletions(-) create mode 100644 tools/objtool/arch/x86/special.c diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build index 7c50040..9f7869b 100644 --- a/tools/objtool/arch/x86/Build +++ b/tools/objtool/arch/x86/Build @@ -1,3 +1,4 @@ +objtool-y += special.o objtool-y += decode.o inat_tables_script = ../arch/x86/tools/gen-insn-attr-x86.awk diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c new file mode 100644 index 000..823561e --- /dev/null +++ b/tools/objtool/arch/x86/special.c @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include "../../special.h" +#include "../../builtin.h" + +#define X86_FEATURE_POPCNT (4 * 32 + 23) +#define X86_FEATURE_SMAP (9 * 32 + 20) + +void arch_handle_alternative(unsigned short feature, struct special_alt *alt) +{ + switch (feature) { + case X86_FEATURE_SMAP: + /* +* If UACCESS validation is enabled; force that alternative; +* otherwise force it the other way. +* +* What we want to avoid is having both the original and the +* alternative code flow at the same time, in that case we can +* find paths that see the STAC but take the NOP instead of +* CLAC and the other way around. +*/ + if (uaccess) + alt->skip_orig = true; + else + alt->skip_alt = true; + break; + case X86_FEATURE_POPCNT: + /* +* It has been requested that we don't validate the !POPCNT +* feature path which is a "very very small percentage of +* machines". +*/ + alt->skip_orig = true; + break; + default: + break; + } +} diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h index a635f68..4125d45 100644 --- a/tools/objtool/objtool.h +++ b/tools/objtool/objtool.h @@ -12,6 +12,8 @@ #include "elf.h" +#define __weak __attribute__((weak)) + struct objtool_file { struct elf *elf; struct list_head insn_list; diff --git a/tools/objtool/special.c b/tools/objtool/special.c index b04f395..1a2420f 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -16,9 +16,6 @@ #include "warn.h" #include "arch_special.h" -#define X86_FEATURE_POPCNT (4*32+23) -#define X86_FEATURE_SMAP (9*32+20) - struct special_entry { const char *sec; bool group, jump_or_nop; @@ -54,6 +51,10 @@ struct special_entry entries[] = { {}, }; +void __weak arch_handle_alternative(unsigned short feature, struct special_alt *alt) +{ +} + static int get_alt_entry(struct elf *elf, struct special_entry *entry, struct section *sec, int idx, struct special_alt *alt) @@ -78,30 +79,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry, feature = *(unsigned short *)(sec->data->d_buf + offset + entry->feature); - - /* -* It has been requested that we don't validate the !POPCNT -* feature path which is a "very very small percentage of -* machines". -*/ - if (feature == X86_FEATURE_POPCNT) - alt->skip_orig = true; - - /* -* If UACCESS validation is enabled; force that alternative; -* otherwise force it the other way. -* -* What we want to avoid is having both the original and the -* alternative code flow at the same
[tip: objtool/core] objtool: Rename frame.h -> objtool.h
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 00089c048eb4a8250325efb32a2724fd0da68cce Gitweb: https://git.kernel.org/tip/00089c048eb4a8250325efb32a2724fd0da68cce Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:25 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Rename frame.h -> objtool.h Header frame.h is getting more code annotations to help objtool analyze object files. Rename the file to objtool.h. [ jpoimboe: add objtool.h to MAINTAINERS ] Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- MAINTAINERS | 1 +- arch/x86/include/asm/nospec-branch.h | 2 +- arch/x86/kernel/kprobes/core.c | 2 +- arch/x86/kernel/kprobes/opt.c| 2 +- arch/x86/kernel/reboot.c | 2 +- arch/x86/kvm/svm/svm.c | 2 +- arch/x86/kvm/vmx/nested.c| 2 +- arch/x86/kvm/vmx/vmx.c | 2 +- arch/x86/xen/enlighten_pv.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 3 +-- include/linux/frame.h| 35 +--- include/linux/objtool.h | 35 +++- kernel/bpf/core.c| 2 +- kernel/kexec_core.c | 2 +- 14 files changed, 47 insertions(+), 47 deletions(-) delete mode 100644 include/linux/frame.h create mode 100644 include/linux/objtool.h diff --git a/MAINTAINERS b/MAINTAINERS index e4647c8..2605a08 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12481,6 +12481,7 @@ M: Josh Poimboeuf M: Peter Zijlstra S: Supported F: tools/objtool/ +F: include/linux/objtool.h OCELOT ETHERNET SWITCH DRIVER M: Microchip Linux Driver Support diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index e7752b4..86651e8 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -4,7 +4,7 @@ #define _ASM_X86_NOSPEC_BRANCH_H_ #include -#include +#include #include #include diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index fdadc37..ae24886 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -38,9 +38,9 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c index c068e21..9b1cb8f 100644 --- a/arch/x86/kernel/kprobes/opt.c +++ b/arch/x86/kernel/kprobes/opt.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c index a515e2d..db11594 100644 --- a/arch/x86/kernel/reboot.c +++ b/arch/x86/kernel/reboot.c @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 0194336..17ba613 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 23b58c2..ae4ff7c 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 -#include +#include #include #include diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 819c185..df29fb4 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -13,7 +13,6 @@ * Yaniv Kamay */ -#include #include #include #include @@ -22,6 +21,7 @@ #include #include #include +#include #include #include #include diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index 22e741e..58382d2 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -32,7 +32,7 @@ #include #include #include -#include +#include #include #include diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c index e9f448a..15b5bde 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c @@ -24,7 +24,7 @@ * */ -#include +#include #include #include #include @@ -599,4 +599,3 @@ out_open: return -EINVAL; } - diff --git a/include/linux/frame.h b/include/linux/frame.h deleted file mode 100644 index 303cda6..000 --- a/include/linux/frame.h +++ /dev/null @@ -1,35 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef _LINUX_FRAME_H -#define _LINUX_FRAME_H - -#ifdef CONFIG_STACK_VALIDATION -/* - * This macro marks the given function's stack frame as "non-standard", which - * tells objtool to ignore the function when doing stack metadata validation. - * It should only be used in special cases where you're 100% sure it won't - * affect the reliability
[tip: objtool/core] objtool: Make relocation in alternative handling arch dependent
The following commit has been merged into the objtool/core branch of tip: Commit-ID: 45245f51f9a4b9a883a8c94468473c1de9b88153 Gitweb: https://git.kernel.org/tip/45245f51f9a4b9a883a8c94468473c1de9b88153 Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:23 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Make relocation in alternative handling arch dependent As pointed out by the comment in handle_group_alt(), support of relocation for instructions in an alternative group depends on whether arch specific kernel code handles it. So, let objtool arch specific code decide whether a relocation for the alternative section should be accepted. Reviewed-by: Miroslav Benes Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- tools/objtool/arch/x86/special.c | 13 + tools/objtool/check.c| 19 ++- tools/objtool/check.h| 6 ++ tools/objtool/special.h | 4 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c index 823561e..34e0e16 100644 --- a/tools/objtool/arch/x86/special.c +++ b/tools/objtool/arch/x86/special.c @@ -35,3 +35,16 @@ void arch_handle_alternative(unsigned short feature, struct special_alt *alt) break; } } + +bool arch_support_alt_relocation(struct special_alt *special_alt, +struct instruction *insn, +struct reloc *reloc) +{ + /* +* The x86 alternatives code adjusts the offsets only when it +* encounters a branch instruction at the very beginning of the +* replacement group. +*/ + return insn->offset == special_alt->new_off && + (insn->type == INSN_CALL || is_static_jump(insn)); +} diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 4afc2d5..1796a7c 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -110,12 +110,6 @@ static struct instruction *prev_insn_same_sym(struct objtool_file *file, for (insn = next_insn_same_sec(file, insn); insn; \ insn = next_insn_same_sec(file, insn)) -static bool is_static_jump(struct instruction *insn) -{ - return insn->type == INSN_JUMP_CONDITIONAL || - insn->type == INSN_JUMP_UNCONDITIONAL; -} - static bool is_sibling_call(struct instruction *insn) { /* An indirect jump is either a sibling call or a jump to a table. */ @@ -972,6 +966,8 @@ static int handle_group_alt(struct objtool_file *file, alt_group = alt_group_next_index++; insn = *new_insn; sec_for_each_insn_from(file, insn) { + struct reloc *alt_reloc; + if (insn->offset >= special_alt->new_off + special_alt->new_len) break; @@ -988,14 +984,11 @@ static int handle_group_alt(struct objtool_file *file, * .altinstr_replacement section, unless the arch's * alternatives code can adjust the relative offsets * accordingly. -* -* The x86 alternatives code adjusts the offsets only when it -* encounters a branch instruction at the very beginning of the -* replacement group. */ - if ((insn->offset != special_alt->new_off || - (insn->type != INSN_CALL && !is_static_jump(insn))) && - find_reloc_by_dest_range(file->elf, insn->sec, insn->offset, insn->len)) { + alt_reloc = find_reloc_by_dest_range(file->elf, insn->sec, + insn->offset, insn->len); + if (alt_reloc && + !arch_support_alt_relocation(special_alt, insn, alt_reloc)) { WARN_FUNC("unsupported relocation in alternatives section", insn->sec, insn->offset); diff --git a/tools/objtool/check.h b/tools/objtool/check.h index 6cac345..1de1188 100644 --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -48,6 +48,12 @@ struct instruction { #endif }; +static inline bool is_static_jump(struct instruction *insn) +{ + return insn->type == INSN_JUMP_CONDITIONAL || + insn->type == INSN_JUMP_UNCONDITIONAL; +} + struct instruction *find_insn(struct objtool_file *file, struct section *sec, unsigned long offset); diff --git a/tools/objtool/special.h b/tools/objtool/special.h index 44da89a..1dc1bb3 100644 --- a/tools/objtool/special.h +++ b/tools/objtool/special.h @@ -7,6 +7,7 @@ #define _SPECIAL_H #include +#include "check.h" #include "elf.h" struct special_alt { @@ -30,4 +31,7 @@ int special_g
[tip: objtool/core] objtool: Make unwind hint definitions available to other architectures
The following commit has been merged into the objtool/core branch of tip: Commit-ID: ee819aedf34a8f35cd54ee3967c7beb4d1d4a635 Gitweb: https://git.kernel.org/tip/ee819aedf34a8f35cd54ee3967c7beb4d1d4a635 Author:Julien Thierry AuthorDate:Fri, 04 Sep 2020 16:30:27 +01:00 Committer: Josh Poimboeuf CommitterDate: Thu, 10 Sep 2020 10:43:13 -05:00 objtool: Make unwind hint definitions available to other architectures Unwind hints are useful to provide objtool with information about stack states in non-standard functions/code. While the type of information being provided might be very arch specific, the mechanism to provide the information can be useful for other architectures. Move the relevant unwint hint definitions for all architectures to see. [ jpoimboe: REGS_IRET -> REGS_PARTIAL ] Signed-off-by: Julien Thierry Signed-off-by: Josh Poimboeuf --- arch/x86/include/asm/orc_types.h | 34 +-- arch/x86/include/asm/unwind_hints.h| 56 +- arch/x86/kernel/unwind_orc.c | 11 +- include/linux/objtool.h| 88 - tools/arch/x86/include/asm/orc_types.h | 34 +-- tools/include/linux/objtool.h | 129 - tools/objtool/check.c | 4 +- tools/objtool/orc_dump.c | 9 +- tools/objtool/orc_gen.c| 5 +- tools/objtool/sync-check.sh| 4 +- 10 files changed, 249 insertions(+), 125 deletions(-) create mode 100644 tools/include/linux/objtool.h diff --git a/arch/x86/include/asm/orc_types.h b/arch/x86/include/asm/orc_types.h index d255349..fdbffec 100644 --- a/arch/x86/include/asm/orc_types.h +++ b/arch/x86/include/asm/orc_types.h @@ -39,27 +39,6 @@ #define ORC_REG_SP_INDIRECT9 #define ORC_REG_MAX15 -/* - * ORC_TYPE_CALL: Indicates that sp_reg+sp_offset resolves to PREV_SP (the - * caller's SP right before it made the call). Used for all callable - * functions, i.e. all C code and all callable asm functions. - * - * ORC_TYPE_REGS: Used in entry code to indicate that sp_reg+sp_offset points - * to a fully populated pt_regs from a syscall, interrupt, or exception. - * - * ORC_TYPE_REGS_IRET: Used in entry code to indicate that sp_reg+sp_offset - * points to the iret return frame. - * - * The UNWIND_HINT macros are used only for the unwind_hint struct. They - * aren't used in struct orc_entry due to size and complexity constraints. - * Objtool converts them to real types when it converts the hints to orc - * entries. - */ -#define ORC_TYPE_CALL 0 -#define ORC_TYPE_REGS 1 -#define ORC_TYPE_REGS_IRET 2 -#define UNWIND_HINT_TYPE_RET_OFFSET3 - #ifndef __ASSEMBLY__ /* * This struct is more or less a vastly simplified version of the DWARF Call @@ -78,19 +57,6 @@ struct orc_entry { unsignedend:1; } __packed; -/* - * This struct is used by asm and inline asm code to manually annotate the - * location of registers on the stack for the ORC unwinder. - * - * Type can be either ORC_TYPE_* or UNWIND_HINT_TYPE_*. - */ -struct unwind_hint { - u32 ip; - s16 sp_offset; - u8 sp_reg; - u8 type; - u8 end; -}; #endif /* __ASSEMBLY__ */ #endif /* _ORC_TYPES_H */ diff --git a/arch/x86/include/asm/unwind_hints.h b/arch/x86/include/asm/unwind_hints.h index 7d903fd..664d461 100644 --- a/arch/x86/include/asm/unwind_hints.h +++ b/arch/x86/include/asm/unwind_hints.h @@ -1,51 +1,17 @@ #ifndef _ASM_X86_UNWIND_HINTS_H #define _ASM_X86_UNWIND_HINTS_H +#include + #include "orc_types.h" #ifdef __ASSEMBLY__ -/* - * In asm, there are two kinds of code: normal C-type callable functions and - * the rest. The normal callable functions can be called by other code, and - * don't do anything unusual with the stack. Such normal callable functions - * are annotated with the ENTRY/ENDPROC macros. Most asm code falls in this - * category. In this case, no special debugging annotations are needed because - * objtool can automatically generate the ORC data for the ORC unwinder to read - * at runtime. - * - * Anything which doesn't fall into the above category, such as syscall and - * interrupt handlers, tends to not be called directly by other functions, and - * often does unusual non-C-function-type things with the stack pointer. Such - * code needs to be annotated such that objtool can understand it. The - * following CFI hint macros are for this type of code. - * - * These macros provide hints to objtool about the state of the stack at each - * instruction. Objtool starts from the hints and follows the code flow, - * making automatic CFI adjustments when it sees pushes and pops, filling out - * the debuginfo as necessary. It will also warn if it sees any - * inconsistencies. - */ -.macro UNWIND_HINT sp_reg=ORC_REG_SP sp_
[PATCH] objtool: Fix sync-check.sh bashisms
Previous patches introduced some non SUS compliant changes to sync-check.sh. Replace used bash features for standard shell. Signed-off-by: Julien Thierry --- tools/objtool/sync-check.sh | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Note: This patch applies on Josh P.'s objtool/core.WIP.julien branch diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh index b81cda59d878..606a4b5e929f 100755 --- a/tools/objtool/sync-check.sh +++ b/tools/objtool/sync-check.sh @@ -8,7 +8,7 @@ fi FILES="include/linux/objtool.h" -if [ "$SRCARCH" == "x86" ]; then +if [ "$SRCARCH" = "x86" ]; then FILES="$FILES arch/x86/include/asm/inat_types.h arch/x86/include/asm/orc_types.h @@ -60,4 +60,6 @@ while read -r file_entry; do fi check $file_entry -done <<< "$FILES" +done <