Re: [RFC PATCH v2 13/13] objtool: arm64: Enable stack validation for arm64

2021-03-09 Thread Julien Thierry




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

2021-03-09 Thread Julien Thierry




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

2021-03-04 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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/

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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

2021-03-03 Thread Julien Thierry
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/

2021-02-03 Thread Julien Thierry




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

2021-02-03 Thread Julien Thierry




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/

2021-02-03 Thread Julien Thierry

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

2021-02-03 Thread Julien Thierry




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

2021-02-02 Thread Julien Thierry




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

2021-01-21 Thread Julien Thierry




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

2021-01-21 Thread Julien Thierry

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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry

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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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/

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-20 Thread Julien Thierry
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

2021-01-19 Thread Julien Thierry

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

2021-01-18 Thread tip-bot2 for Julien Thierry
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

2021-01-18 Thread tip-bot2 for Julien Thierry
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

2021-01-18 Thread tip-bot2 for Julien Thierry
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

2020-10-14 Thread Julien Thierry
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

2020-10-14 Thread Julien Thierry
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

2020-10-14 Thread Julien Thierry
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

2020-10-14 Thread Julien Thierry
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

2020-10-13 Thread Julien Thierry




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

2020-10-12 Thread Julien Thierry




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

2020-09-28 Thread Julien Thierry
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

2020-09-28 Thread Julien Thierry
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

2020-09-28 Thread Julien Thierry
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

2020-09-28 Thread Julien Thierry
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

2020-09-21 Thread tip-bot2 for Julien Thierry
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()

2020-09-21 Thread tip-bot2 for Julien Thierry
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

2020-09-21 Thread tip-bot2 for Julien Thierry
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

2020-09-21 Thread Julien Thierry




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

2020-09-21 Thread Julien Thierry




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

2020-09-21 Thread Julien Thierry




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

2020-09-15 Thread Julien Thierry
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

2020-09-15 Thread Julien Thierry
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

2020-09-15 Thread Julien Thierry
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

2020-09-15 Thread Julien Thierry
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

2020-09-15 Thread Julien Thierry
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

2020-09-15 Thread Julien Thierry
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

2020-09-15 Thread Julien Thierry
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

2020-09-15 Thread Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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()

2020-09-10 Thread tip-bot2 for Julien Thierry
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()

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-10 Thread tip-bot2 for Julien Thierry
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

2020-09-08 Thread Julien Thierry
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 <

  1   2   3   4   5   6   7   >