2020-09-04 15:33:17

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 00/10] Make check implementation arch agnostic

Hi,

The current implementation of the check subcommand has various x86 bits
here and there. In order to prepare objtool to provide check for other
architectures, add some abstraction over the x86 specific bits, relying
on objtool arch specific code to provide some necessary operations.

This is part of the effort to implement check for arm64, initiated [1]
by Raphael. The series is based on top of the separation of check & orc
subcommands series[2].

I've push both series base on top of tip/objtool/core at [3].

- The first two patches make it simpler for new arches to provide their
list of kernel headers, without worrying about modifications in the x86
headers.
- Patch 3 Moves arch specific macros to more suitable location
- Patches 4 and 5 add abstraction to handle alternatives
- Patch 6 adds abstraction to handle jump table
- Patches 7-10 makes unwind hint definitions shared across architectures

Changes since v2 [4]:
- Rebased on v5.9-rc1
- Under tools/objtool/arch/x86/, rename arch_special.c to special.c
- Rename include/linux/frame.h to inclide/linux/objtool.h
- Share unwind hint types across architectures

[1] https://lkml.org/lkml/2019/8/16/400
[2] https://lkml.org/lkml/2020/6/4/675
[3] https://github.com/julien-thierry/linux/tree/arch-independent-check
[4] https://lkml.org/lkml/2020/7/30/424

Cheers,

Julien

-->

Julien Thierry (9):
objtool: Group headers to check in a single list
objtool: Make sync-check consider the target architecture
objtool: Move macros describing structures to arch-dependent code
objtool: Abstract alternative special case handling
objtool: Make relocation in alternative handling arch dependent
headers: Rename frame.h
objtool: Only include valid definitions depending on source file type
objtool: Make unwind hints definitions available to other
architectures
objtool: Decode unwind hint register depending on architecture

Raphael Gault (1):
objtool: Refactor switch-tables code to support other architectures

arch/x86/include/asm/nospec-branch.h | 2 +-
arch/x86/include/asm/orc_types.h | 34 ----
arch/x86/include/asm/unwind_hints.h | 50 +-----
arch/x86/kernel/kprobes/core.c | 2 +-
arch/x86/kernel/kprobes/opt.c | 2 +-
arch/x86/kernel/reboot.c | 2 +-
arch/x86/kernel/unwind_orc.c | 11 +-
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 | 134 ++++++++++++++++
kernel/bpf/core.c | 2 +-
kernel/kexec_core.c | 2 +-
tools/arch/x86/include/asm/orc_types.h | 34 ----
tools/include/linux/objtool.h | 134 ++++++++++++++++
tools/objtool/Makefile | 2 +-
tools/objtool/arch.h | 2 +
tools/objtool/arch/x86/Build | 1 +
tools/objtool/arch/x86/decode.c | 37 +++++
tools/objtool/arch/x86/include/arch_special.h | 20 +++
tools/objtool/arch/x86/special.c | 145 ++++++++++++++++++
tools/objtool/check.c | 137 ++---------------
tools/objtool/check.h | 7 +-
tools/objtool/objtool.h | 2 +
tools/objtool/orc_dump.c | 7 +-
tools/objtool/orc_gen.c | 5 +-
tools/objtool/special.c | 48 +-----
tools/objtool/special.h | 10 ++
tools/objtool/sync-check.sh | 27 ++--
tools/objtool/weak.c | 2 -
33 files changed, 561 insertions(+), 346 deletions(-)
delete mode 100644 include/linux/frame.h
create mode 100644 include/linux/objtool.h
create mode 100644 tools/include/linux/objtool.h
create mode 100644 tools/objtool/arch/x86/include/arch_special.h
create mode 100644 tools/objtool/arch/x86/special.c

--
2.21.3


2020-09-04 15:34:52

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 04/10] 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 <[email protected]>
Signed-off-by: Julien Thierry <[email protected]>
---
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 7c5004008e97..9f7869b5c5e0 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 000000000000..823561e4015c
--- /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 46240098f08d..79716f4158e8 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 b04f395de5de..1a2420febd08 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 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 (feature == X86_FEATURE_SMAP) {
- if (uaccess)
- alt->skip_orig = true;
- else
- alt->skip_alt = true;
- }
+ arch_handle_alternative(feature, alt);
}

orig_reloc = find_reloc_by_dest(elf, sec, offset + entry->orig);
diff --git a/tools/objtool/special.h b/tools/objtool/special.h
index 35061530e46e..44da89afeda2 100644
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -28,4 +28,6 @@ struct special_alt {

int special_get_alts(struct elf *elf, struct list_head *alts);

+void arch_handle_alternative(unsigned short feature, struct special_alt *alt);
+
#endif /* _SPECIAL_H */
diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c
index 29180d599b08..7843e9a7a72f 100644
--- a/tools/objtool/weak.c
+++ b/tools/objtool/weak.c
@@ -9,8 +9,6 @@
#include <errno.h>
#include "objtool.h"

-#define __weak __attribute__((weak))
-
#define UNSUPPORTED(name) \
({ \
fprintf(stderr, "error: objtool: " name " not implemented\n"); \
--
2.21.3

2020-09-04 15:35:48

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 07/10] headers: Rename frame.h

Header frame.h is getting more code annotations to help objtool analyze
object files.

Rename the file to objtool.h.

Signed-off-by: Julien Thierry <[email protected]>
---
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 => objtool.h} | 6 +++---
kernel/bpf/core.c | 2 +-
kernel/kexec_core.c | 2 +-
12 files changed, 14 insertions(+), 15 deletions(-)
rename include/linux/{frame.h => objtool.h} (93%)

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index e7752b4038ff..86651e86289d 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 <linux/static_key.h>
-#include <linux/frame.h>
+#include <linux/objtool.h>

#include <asm/alternative.h>
#include <asm/alternative-asm.h>
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index fdadc37d72af..ae2488643029 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -38,9 +38,9 @@
#include <linux/kdebug.h>
#include <linux/kallsyms.h>
#include <linux/ftrace.h>
-#include <linux/frame.h>
#include <linux/kasan.h>
#include <linux/moduleloader.h>
+#include <linux/objtool.h>
#include <linux/vmalloc.h>
#include <linux/pgtable.h>

diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 40f380461e6d..f39ce3f67863 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -16,7 +16,7 @@
#include <linux/kdebug.h>
#include <linux/kallsyms.h>
#include <linux/ftrace.h>
-#include <linux/frame.h>
+#include <linux/objtool.h>
#include <linux/pgtable.h>

#include <asm/text-patching.h>
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 0ec7ced727fe..603f33645e75 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -10,7 +10,7 @@
#include <linux/sched.h>
#include <linux/tboot.h>
#include <linux/delay.h>
-#include <linux/frame.h>
+#include <linux/objtool.h>
#include <linux/pgtable.h>
#include <acpi/reboot.h>
#include <asm/io.h>
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 03dd7bac8034..2ff0152ae6c2 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -19,7 +19,7 @@
#include <linux/trace_events.h>
#include <linux/slab.h>
#include <linux/hashtable.h>
-#include <linux/frame.h>
+#include <linux/objtool.h>
#include <linux/psp-sev.h>
#include <linux/file.h>
#include <linux/pagemap.h>
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 23b58c28a1c9..ae4ff7c624a4 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 <linux/frame.h>
+#include <linux/objtool.h>
#include <linux/percpu.h>

#include <asm/debugreg.h>
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 46ba2e03a892..e1f24749fa67 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -13,7 +13,6 @@
* Yaniv Kamay <[email protected]>
*/

-#include <linux/frame.h>
#include <linux/highmem.h>
#include <linux/hrtimer.h>
#include <linux/kernel.h>
@@ -22,6 +21,7 @@
#include <linux/moduleparam.h>
#include <linux/mod_devicetable.h>
#include <linux/mm.h>
+#include <linux/objtool.h>
#include <linux/sched.h>
#include <linux/sched/smt.h>
#include <linux/slab.h>
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 22e741e0b10c..58382d26f153 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -32,7 +32,7 @@
#include <linux/pci.h>
#include <linux/gfp.h>
#include <linux/edd.h>
-#include <linux/frame.h>
+#include <linux/objtool.h>

#include <xen/xen.h>
#include <xen/events.h>
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index e9f448a5ebb3..15b5bde69324 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -24,7 +24,7 @@
*
*/

-#include <linux/frame.h>
+#include <linux/objtool.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/slab.h>
@@ -599,4 +599,3 @@ int vmw_msg_ioctl(struct drm_device *dev, void *data,

return -EINVAL;
}
-
diff --git a/include/linux/frame.h b/include/linux/objtool.h
similarity index 93%
rename from include/linux/frame.h
rename to include/linux/objtool.h
index 303cda600e56..358175c9c2b5 100644
--- a/include/linux/frame.h
+++ b/include/linux/objtool.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _LINUX_FRAME_H
-#define _LINUX_FRAME_H
+#ifndef _LINUX_OBJTOOL_H
+#define _LINUX_OBJTOOL_H

#ifdef CONFIG_STACK_VALIDATION
/*
@@ -32,4 +32,4 @@

#endif /* CONFIG_STACK_VALIDATION */

-#endif /* _LINUX_FRAME_H */
+#endif /* _LINUX_OBJTOOL_H */
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ed0b3578867c..03e284873644 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -25,7 +25,7 @@
#include <linux/moduleloader.h>
#include <linux/bpf.h>
#include <linux/btf.h>
-#include <linux/frame.h>
+#include <linux/objtool.h>
#include <linux/rbtree_latch.h>
#include <linux/kallsyms.h>
#include <linux/rcupdate.h>
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index c19c0dad1ebe..c5e5e5a11535 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -36,7 +36,7 @@
#include <linux/syscore_ops.h>
#include <linux/compiler.h>
#include <linux/hugetlb.h>
-#include <linux/frame.h>
+#include <linux/objtool.h>

#include <asm/page.h>
#include <asm/sections.h>
--
2.21.3

2020-09-04 15:36:16

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 08/10] 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 <[email protected]>
Signed-off-by: Julien Thierry <[email protected]>
---
include/linux/objtool.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 358175c9c2b5..15e9997a9fb4 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)
--
2.21.3

2020-09-04 15:37:00

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 10/10] 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 <[email protected]>
---
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 b18c5f61d42d..4a84c3081b8e 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 1967370440b3..cde9c36e40ae 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 <asm/orc_types.h>

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 60e23c8f93e0..8630a2d5e68c 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1261,32 +1261,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;
--
2.21.3

2020-09-04 15:37:14

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 03/10] 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 <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Signed-off-by: Julien Thierry <[email protected]>
---
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 000000000000..d818b2bffa02
--- /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_SIZE 16
+#define JUMP_ORIG_OFFSET 0
+#define JUMP_NEW_OFFSET 4
+
+#define ALT_ENTRY_SIZE 13
+#define ALT_ORIG_OFFSET 0
+#define ALT_NEW_OFFSET 4
+#define ALT_FEATURE_OFFSET 8
+#define ALT_ORIG_LEN_OFFSET 10
+#define ALT_NEW_LEN_OFFSET 11
+
+#endif /* _X86_ARCH_SPECIAL_H */
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index e893f1e48e44..b04f395de5de 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_SIZE 16
-#define JUMP_ORIG_OFFSET 0
-#define JUMP_NEW_OFFSET 4
-
-#define ALT_ENTRY_SIZE 13
-#define ALT_ORIG_OFFSET 0
-#define ALT_NEW_OFFSET 4
-#define ALT_FEATURE_OFFSET 8
-#define ALT_ORIG_LEN_OFFSET 10
-#define ALT_NEW_LEN_OFFSET 11
+#include "arch_special.h"

#define X86_FEATURE_POPCNT (4*32+23)
#define X86_FEATURE_SMAP (9*32+20)
--
2.21.3

2020-09-04 15:37:44

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 05/10] 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 <[email protected]>
Signed-off-by: Julien Thierry <[email protected]>
---
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 823561e4015c..34e0e162e6fd 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 6156bd9a687c..8217a9a9a838 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -109,12 +109,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. */
@@ -866,6 +860,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;

@@ -882,14 +878,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 0abdf8efdbc0..58374255934b 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -47,6 +47,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 44da89afeda2..1dc1bb3e74c6 100644
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -7,6 +7,7 @@
#define _SPECIAL_H

#include <stdbool.h>
+#include "check.h"
#include "elf.h"

struct special_alt {
@@ -30,4 +31,7 @@ int special_get_alts(struct elf *elf, struct list_head *alts);

void arch_handle_alternative(unsigned short feature, struct special_alt *alt);

+bool arch_support_alt_relocation(struct special_alt *special_alt,
+ struct instruction *insn,
+ struct reloc *reloc);
#endif /* _SPECIAL_H */
--
2.21.3

2020-09-04 15:37:51

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 06/10] objtool: Refactor switch-tables code to support other architectures

From: Raphael Gault <[email protected]>

The way to identify switch-tables and retrieves all the data necessary
to handle the different execution branches is not the same on all
architecture. In order to be able to add other architecture support,
define an arch-dependent function to process jump-tables.

Reviewed-by: Miroslav Benes <[email protected]>
Signed-off-by: Raphael Gault <[email protected]>
[J.T.: Move arm64 bits out of this patch,
Have only one function to find the start of the jump table,
for now assume that the jump table format will be the same as
x86]
Signed-off-by: Julien Thierry <[email protected]>
---
tools/objtool/arch/x86/special.c | 95 ++++++++++++++++++++++++++++++++
tools/objtool/check.c | 88 +----------------------------
tools/objtool/check.h | 1 -
tools/objtool/special.h | 4 ++
4 files changed, 102 insertions(+), 86 deletions(-)

diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index 34e0e162e6fd..fd4af88c0ea5 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -1,4 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#include <string.h>
+
#include "../../special.h"
#include "../../builtin.h"

@@ -48,3 +50,96 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
return insn->offset == special_alt->new_off &&
(insn->type == INSN_CALL || is_static_jump(insn));
}
+
+/*
+ * There are 3 basic jump table patterns:
+ *
+ * 1. jmpq *[rodata addr](,%reg,8)
+ *
+ * This is the most common case by far. It jumps to an address in a simple
+ * jump table which is stored in .rodata.
+ *
+ * 2. jmpq *[rodata addr](%rip)
+ *
+ * This is caused by a rare GCC quirk, currently only seen in three driver
+ * functions in the kernel, only with certain obscure non-distro configs.
+ *
+ * As part of an optimization, GCC makes a copy of an existing switch jump
+ * table, modifies it, and then hard-codes the jump (albeit with an indirect
+ * jump) to use a single entry in the table. The rest of the jump table and
+ * some of its jump targets remain as dead code.
+ *
+ * In such a case we can just crudely ignore all unreachable instruction
+ * warnings for the entire object file. Ideally we would just ignore them
+ * for the function, but that would require redesigning the code quite a
+ * bit. And honestly that's just not worth doing: unreachable instruction
+ * warnings are of questionable value anyway, and this is such a rare issue.
+ *
+ * 3. mov [rodata addr],%reg1
+ * ... some instructions ...
+ * jmpq *(%reg1,%reg2,8)
+ *
+ * This is a fairly uncommon pattern which is new for GCC 6. As of this
+ * writing, there are 11 occurrences of it in the allmodconfig kernel.
+ *
+ * As of GCC 7 there are quite a few more of these and the 'in between' code
+ * is significant. Esp. with KASAN enabled some of the code between the mov
+ * and jmpq uses .rodata itself, which can confuse things.
+ *
+ * TODO: Once we have DWARF CFI and smarter instruction decoding logic,
+ * ensure the same register is used in the mov and jump instructions.
+ *
+ * NOTE: RETPOLINE made it harder still to decode dynamic jumps.
+ */
+struct reloc *arch_find_switch_table(struct objtool_file *file,
+ struct instruction *insn)
+{
+ struct reloc *text_reloc, *rodata_reloc;
+ struct section *table_sec;
+ unsigned long table_offset;
+
+ /* look for a relocation which references .rodata */
+ text_reloc = find_reloc_by_dest_range(file->elf, insn->sec,
+ insn->offset, insn->len);
+ if (!text_reloc || text_reloc->sym->type != STT_SECTION ||
+ !text_reloc->sym->sec->rodata)
+ return NULL;
+
+ table_offset = text_reloc->addend;
+ table_sec = text_reloc->sym->sec;
+
+ if (text_reloc->type == R_X86_64_PC32)
+ table_offset += 4;
+
+ /*
+ * Make sure the .rodata address isn't associated with a
+ * symbol. GCC jump tables are anonymous data.
+ *
+ * Also support C jump tables which are in the same format as
+ * switch jump tables. For objtool to recognize them, they
+ * need to be placed in the C_JUMP_TABLE_SECTION section. They
+ * have symbols associated with them.
+ */
+ if (find_symbol_containing(table_sec, table_offset) &&
+ strcmp(table_sec->name, C_JUMP_TABLE_SECTION))
+ return NULL;
+
+ /*
+ * Each table entry has a rela associated with it. The rela
+ * should reference text in the same function as the original
+ * instruction.
+ */
+ rodata_reloc = find_reloc_by_dest(file->elf, table_sec, table_offset);
+ if (!rodata_reloc)
+ return NULL;
+
+ /*
+ * Use of RIP-relative switch jumps is quite rare, and
+ * indicates a rare GCC quirk/bug which can leave dead
+ * code behind.
+ */
+ if (text_reloc->type == R_X86_64_PC32)
+ file->ignore_unreachables = true;
+
+ return rodata_reloc;
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8217a9a9a838..921b4ba2d0f9 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -19,8 +19,6 @@

#define FAKE_JUMP_OFFSET -1

-#define C_JUMP_TABLE_SECTION ".rodata..c_jump_table"
-
struct alternative {
struct list_head list;
struct instruction *insn;
@@ -1085,55 +1083,14 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,

/*
* find_jump_table() - Given a dynamic jump, find the switch jump table in
- * .rodata associated with it.
- *
- * There are 3 basic patterns:
- *
- * 1. jmpq *[rodata addr](,%reg,8)
- *
- * This is the most common case by far. It jumps to an address in a simple
- * jump table which is stored in .rodata.
- *
- * 2. jmpq *[rodata addr](%rip)
- *
- * This is caused by a rare GCC quirk, currently only seen in three driver
- * functions in the kernel, only with certain obscure non-distro configs.
- *
- * As part of an optimization, GCC makes a copy of an existing switch jump
- * table, modifies it, and then hard-codes the jump (albeit with an indirect
- * jump) to use a single entry in the table. The rest of the jump table and
- * some of its jump targets remain as dead code.
- *
- * In such a case we can just crudely ignore all unreachable instruction
- * warnings for the entire object file. Ideally we would just ignore them
- * for the function, but that would require redesigning the code quite a
- * bit. And honestly that's just not worth doing: unreachable instruction
- * warnings are of questionable value anyway, and this is such a rare issue.
- *
- * 3. mov [rodata addr],%reg1
- * ... some instructions ...
- * jmpq *(%reg1,%reg2,8)
- *
- * This is a fairly uncommon pattern which is new for GCC 6. As of this
- * writing, there are 11 occurrences of it in the allmodconfig kernel.
- *
- * As of GCC 7 there are quite a few more of these and the 'in between' code
- * is significant. Esp. with KASAN enabled some of the code between the mov
- * and jmpq uses .rodata itself, which can confuse things.
- *
- * TODO: Once we have DWARF CFI and smarter instruction decoding logic,
- * ensure the same register is used in the mov and jump instructions.
- *
- * NOTE: RETPOLINE made it harder still to decode dynamic jumps.
+ * associated with it.
*/
static struct reloc *find_jump_table(struct objtool_file *file,
struct symbol *func,
struct instruction *insn)
{
- struct reloc *text_reloc, *table_reloc;
+ struct reloc *table_reloc;
struct instruction *dest_insn, *orig_insn = insn;
- struct section *table_sec;
- unsigned long table_offset;

/*
* Backward search using the @first_jump_src links, these help avoid
@@ -1154,52 +1111,13 @@ static struct reloc *find_jump_table(struct objtool_file *file,
insn->jump_dest->offset > orig_insn->offset))
break;

- /* look for a relocation which references .rodata */
- text_reloc = find_reloc_by_dest_range(file->elf, insn->sec,
- insn->offset, insn->len);
- if (!text_reloc || text_reloc->sym->type != STT_SECTION ||
- !text_reloc->sym->sec->rodata)
- continue;
-
- table_offset = text_reloc->addend;
- table_sec = text_reloc->sym->sec;
-
- if (text_reloc->type == R_X86_64_PC32)
- table_offset += 4;
-
- /*
- * Make sure the .rodata address isn't associated with a
- * symbol. GCC jump tables are anonymous data.
- *
- * Also support C jump tables which are in the same format as
- * switch jump tables. For objtool to recognize them, they
- * need to be placed in the C_JUMP_TABLE_SECTION section. They
- * have symbols associated with them.
- */
- if (find_symbol_containing(table_sec, table_offset) &&
- strcmp(table_sec->name, C_JUMP_TABLE_SECTION))
- continue;
-
- /*
- * Each table entry has a reloc associated with it. The reloc
- * should reference text in the same function as the original
- * instruction.
- */
- table_reloc = find_reloc_by_dest(file->elf, table_sec, table_offset);
+ table_reloc = arch_find_switch_table(file, insn);
if (!table_reloc)
continue;
dest_insn = find_insn(file, table_reloc->sym->sec, table_reloc->addend);
if (!dest_insn || !dest_insn->func || dest_insn->func->pfunc != func)
continue;

- /*
- * Use of RIP-relative switch jumps is quite rare, and
- * indicates a rare GCC quirk/bug which can leave dead code
- * behind.
- */
- if (text_reloc->type == R_X86_64_PC32)
- file->ignore_unreachables = true;
-
return table_reloc;
}

diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 58374255934b..714ab8df51df 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -65,5 +65,4 @@ struct instruction *find_insn(struct objtool_file *file,
insn->sec == sec; \
insn = list_next_entry(insn, list))

-
#endif /* _CHECK_H */
diff --git a/tools/objtool/special.h b/tools/objtool/special.h
index 1dc1bb3e74c6..abddf38ef334 100644
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -10,6 +10,8 @@
#include "check.h"
#include "elf.h"

+#define C_JUMP_TABLE_SECTION ".rodata..c_jump_table"
+
struct special_alt {
struct list_head list;

@@ -34,4 +36,6 @@ void arch_handle_alternative(unsigned short feature, struct special_alt *alt);
bool arch_support_alt_relocation(struct special_alt *special_alt,
struct instruction *insn,
struct reloc *reloc);
+struct reloc *arch_find_switch_table(struct objtool_file *file,
+ struct instruction *insn);
#endif /* _SPECIAL_H */
--
2.21.3

2020-09-04 15:38:19

by Julien Thierry

[permalink] [raw]
Subject: [PATCH v3 09/10] objtool: Make unwind hints 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.

Signed-off-by: Julien Thierry <[email protected]>
---
arch/x86/include/asm/orc_types.h | 34 -------
arch/x86/include/asm/unwind_hints.h | 50 ++-------
arch/x86/kernel/unwind_orc.c | 11 +-
include/linux/objtool.h | 93 +++++++++++++++++
tools/arch/x86/include/asm/orc_types.h | 34 -------
tools/include/linux/objtool.h | 134 +++++++++++++++++++++++++
tools/objtool/check.c | 3 +-
tools/objtool/orc_dump.c | 7 +-
tools/objtool/orc_gen.c | 5 +-
tools/objtool/sync-check.sh | 4 +-
10 files changed, 254 insertions(+), 121 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 d25534940bde..fdbffec4cfde 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_INDIRECT 9
#define ORC_REG_MAX 15

-/*
- * 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_OFFSET 3
-
#ifndef __ASSEMBLY__
/*
* This struct is more or less a vastly simplified version of the DWARF Call
@@ -78,19 +57,6 @@ struct orc_entry {
unsigned end: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 7d903fdb3f43..da38f7f3ae2d 100644
--- a/arch/x86/include/asm/unwind_hints.h
+++ b/arch/x86/include/asm/unwind_hints.h
@@ -1,48 +1,14 @@
#ifndef _ASM_X86_UNWIND_HINTS_H
#define _ASM_X86_UNWIND_HINTS_H

+#include <linux/objtool.h>
+
#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_offset=0 type=ORC_TYPE_CALL end=0
-#ifdef CONFIG_STACK_VALIDATION
-.Lunwind_hint_ip_\@:
- .pushsection .discard.unwind_hints
- /* struct unwind_hint */
- .long .Lunwind_hint_ip_\@ - .
- .short \sp_offset
- .byte \sp_reg
- .byte \type
- .byte \end
- .balign 4
- .popsection
-#endif
-.endm
-
.macro UNWIND_HINT_EMPTY
- UNWIND_HINT sp_reg=ORC_REG_UNDEFINED end=1
+ UNWIND_HINT sp_reg=ORC_REG_UNDEFINED type=UNWIND_HINT_TYPE_CALL end=1
.endm

.macro UNWIND_HINT_REGS base=%rsp offset=0 indirect=0 extra=1 iret=0
@@ -67,12 +33,12 @@
.set sp_offset, \offset

.if \iret
- .set type, ORC_TYPE_REGS_IRET
+ .set type, UNWIND_HINT_TYPE_REGS_IRET
.elseif \extra == 0
- .set type, ORC_TYPE_REGS_IRET
+ .set type, UNWIND_HINT_TYPE_REGS_IRET
.set sp_offset, \offset + (16*8)
.else
- .set type, ORC_TYPE_REGS
+ .set type, UNWIND_HINT_TYPE_REGS
.endif

UNWIND_HINT sp_reg=sp_reg sp_offset=sp_offset type=type
@@ -83,7 +49,7 @@
.endm

.macro UNWIND_HINT_FUNC sp_offset=8
- UNWIND_HINT sp_offset=\sp_offset
+ UNWIND_HINT sp_reg=ORC_REG_SP sp_offset=\sp_offset type=UNWIND_HINT_TYPE_CALL
.endm

/*
@@ -92,7 +58,7 @@
* initial_func_cfi.
*/
.macro UNWIND_HINT_RET_OFFSET sp_offset=8
- UNWIND_HINT type=UNWIND_HINT_TYPE_RET_OFFSET sp_offset=\sp_offset
+ UNWIND_HINT sp_reg=ORC_REG_SP type=UNWIND_HINT_TYPE_RET_OFFSET sp_offset=\sp_offset
.endm

#endif /* __ASSEMBLY__ */
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index ec88bbe08a32..4d8b5db57c9a 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/objtool.h>
#include <linux/module.h>
#include <linux/sort.h>
#include <asm/ptrace.h>
@@ -127,12 +128,12 @@ static struct orc_entry null_orc_entry = {
.sp_offset = sizeof(long),
.sp_reg = ORC_REG_SP,
.bp_reg = ORC_REG_UNDEFINED,
- .type = ORC_TYPE_CALL
+ .type = UNWIND_HINT_TYPE_CALL
};

/* Fake frame pointer entry -- used as a fallback for generated code */
static struct orc_entry orc_fp_entry = {
- .type = ORC_TYPE_CALL,
+ .type = UNWIND_HINT_TYPE_CALL,
.sp_reg = ORC_REG_BP,
.sp_offset = 16,
.bp_reg = ORC_REG_PREV_SP,
@@ -531,7 +532,7 @@ bool unwind_next_frame(struct unwind_state *state)

/* Find IP, SP and possibly regs: */
switch (orc->type) {
- case ORC_TYPE_CALL:
+ case UNWIND_HINT_TYPE_CALL:
ip_p = sp - sizeof(long);

if (!deref_stack_reg(state, ip_p, &state->ip))
@@ -546,7 +547,7 @@ bool unwind_next_frame(struct unwind_state *state)
state->signal = false;
break;

- case ORC_TYPE_REGS:
+ case UNWIND_HINT_TYPE_REGS:
if (!deref_stack_regs(state, sp, &state->ip, &state->sp)) {
orc_warn_current("can't access registers at %pB\n",
(void *)orig_ip);
@@ -559,7 +560,7 @@ bool unwind_next_frame(struct unwind_state *state)
state->signal = true;
break;

- case ORC_TYPE_REGS_IRET:
+ case UNWIND_HINT_TYPE_REGS_IRET:
if (!deref_stack_iret_regs(state, sp, &state->ip, &state->sp)) {
orc_warn_current("can't access iret registers at %pB\n",
(void *)orig_ip);
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 15e9997a9fb4..6df371510b96 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -2,9 +2,60 @@
#ifndef _LINUX_OBJTOOL_H
#define _LINUX_OBJTOOL_H

+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/*
+ * This struct is used by asm and inline asm code to manually annotate the
+ * location of registers on the stack.
+ */
+struct unwind_hint {
+ u32 ip;
+ s16 sp_offset;
+ u8 sp_reg;
+ u8 type;
+ u8 end;
+};
+#endif
+
+/*
+ * UNWIND_HINT_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.
+ *
+ * UNWIND_HINT_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.
+ *
+ * UNWIND_HINT_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 UNWIND_HINT_TYPE_CALL 0
+#define UNWIND_HINT_TYPE_REGS 1
+#define UNWIND_HINT_TYPE_REGS_IRET 2
+#define UNWIND_HINT_TYPE_RET_OFFSET 3
+
#ifdef CONFIG_STACK_VALIDATION

#ifndef __ASSEMBLY__
+
+#define UNWIND_HINT(sp_reg, sp_offset, type, end) \
+ "987: \n\t" \
+ ".pushsection .discard.unwind_hints\n\t" \
+ /* struct unwind_hint */ \
+ ".long 987b - .\n\t" \
+ ".short " __stringify(sp_offset) "\n\t" \
+ ".byte " __stringify(sp_reg) "\n\t" \
+ ".byte " __stringify(type) "\n\t" \
+ ".byte " __stringify(end) "\n\t" \
+ ".balign 4 \n\t" \
+ ".popsection\n\t"
+
/*
* This macro marks the given function's stack frame as "non-standard", which
* tells objtool to ignore the function when doing stack metadata validation.
@@ -29,12 +80,54 @@
.long 999b; \
.popsection;

+/*
+ * 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:req sp_offset=0 type:req end=0
+.Lunwind_hint_ip_\@:
+ .pushsection .discard.unwind_hints
+ /* struct unwind_hint */
+ .long .Lunwind_hint_ip_\@ - .
+ .short \sp_offset
+ .byte \sp_reg
+ .byte \type
+ .byte \end
+ .balign 4
+ .popsection
+.endm
+
#endif /* __ASSEMBLY__ */

#else /* !CONFIG_STACK_VALIDATION */

+#ifndef __ASSEMBLY__
+
+#define UNWIND_HINT(sp_reg, sp_offset, type, end) \
+ "\n\t"
#define STACK_FRAME_NON_STANDARD(func)
+#else
#define ANNOTATE_INTRA_FUNCTION_CALL
+.macro UNWIND_HINT sp_reg:req sp_offset=0 type:req end=0
+.endm
+#endif

#endif /* CONFIG_STACK_VALIDATION */

diff --git a/tools/arch/x86/include/asm/orc_types.h b/tools/arch/x86/include/asm/orc_types.h
index d25534940bde..fdbffec4cfde 100644
--- a/tools/arch/x86/include/asm/orc_types.h
+++ b/tools/arch/x86/include/asm/orc_types.h
@@ -39,27 +39,6 @@
#define ORC_REG_SP_INDIRECT 9
#define ORC_REG_MAX 15

-/*
- * 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_OFFSET 3
-
#ifndef __ASSEMBLY__
/*
* This struct is more or less a vastly simplified version of the DWARF Call
@@ -78,19 +57,6 @@ struct orc_entry {
unsigned end: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/tools/include/linux/objtool.h b/tools/include/linux/objtool.h
new file mode 100644
index 000000000000..6df371510b96
--- /dev/null
+++ b/tools/include/linux/objtool.h
@@ -0,0 +1,134 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_OBJTOOL_H
+#define _LINUX_OBJTOOL_H
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/*
+ * This struct is used by asm and inline asm code to manually annotate the
+ * location of registers on the stack.
+ */
+struct unwind_hint {
+ u32 ip;
+ s16 sp_offset;
+ u8 sp_reg;
+ u8 type;
+ u8 end;
+};
+#endif
+
+/*
+ * UNWIND_HINT_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.
+ *
+ * UNWIND_HINT_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.
+ *
+ * UNWIND_HINT_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 UNWIND_HINT_TYPE_CALL 0
+#define UNWIND_HINT_TYPE_REGS 1
+#define UNWIND_HINT_TYPE_REGS_IRET 2
+#define UNWIND_HINT_TYPE_RET_OFFSET 3
+
+#ifdef CONFIG_STACK_VALIDATION
+
+#ifndef __ASSEMBLY__
+
+#define UNWIND_HINT(sp_reg, sp_offset, type, end) \
+ "987: \n\t" \
+ ".pushsection .discard.unwind_hints\n\t" \
+ /* struct unwind_hint */ \
+ ".long 987b - .\n\t" \
+ ".short " __stringify(sp_offset) "\n\t" \
+ ".byte " __stringify(sp_reg) "\n\t" \
+ ".byte " __stringify(type) "\n\t" \
+ ".byte " __stringify(end) "\n\t" \
+ ".balign 4 \n\t" \
+ ".popsection\n\t"
+
+/*
+ * 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 of frame pointers and kernel stack traces.
+ *
+ * For more information, see tools/objtool/Documentation/stack-validation.txt.
+ */
+#define STACK_FRAME_NON_STANDARD(func) \
+ 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.
+ */
+#define ANNOTATE_INTRA_FUNCTION_CALL \
+ 999: \
+ .pushsection .discard.intra_function_calls; \
+ .long 999b; \
+ .popsection;
+
+/*
+ * 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:req sp_offset=0 type:req end=0
+.Lunwind_hint_ip_\@:
+ .pushsection .discard.unwind_hints
+ /* struct unwind_hint */
+ .long .Lunwind_hint_ip_\@ - .
+ .short \sp_offset
+ .byte \sp_reg
+ .byte \type
+ .byte \end
+ .balign 4
+ .popsection
+.endm
+
+#endif /* __ASSEMBLY__ */
+
+#else /* !CONFIG_STACK_VALIDATION */
+
+#ifndef __ASSEMBLY__
+
+#define UNWIND_HINT(sp_reg, sp_offset, type, end) \
+ "\n\t"
+#define STACK_FRAME_NON_STANDARD(func)
+#else
+#define ANNOTATE_INTRA_FUNCTION_CALL
+.macro UNWIND_HINT sp_reg:req sp_offset=0 type:req end=0
+.endm
+#endif
+
+#endif /* CONFIG_STACK_VALIDATION */
+
+#endif /* _LINUX_OBJTOOL_H */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 921b4ba2d0f9..60e23c8f93e0 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -14,6 +14,7 @@
#include "warn.h"
#include "arch_elf.h"

+#include <linux/objtool.h>
#include <linux/hashtable.h>
#include <linux/kernel.h>

@@ -1678,7 +1679,7 @@ static int update_cfi_state(struct instruction *insn, struct cfi_state *cfi,
return 0;
}

- if (cfi->type == ORC_TYPE_REGS || cfi->type == ORC_TYPE_REGS_IRET)
+ if (cfi->type == UNWIND_HINT_TYPE_REGS || cfi->type == UNWIND_HINT_TYPE_REGS_IRET)
return update_cfi_state_regs(insn, cfi, op);

switch (op->dest.type) {
diff --git a/tools/objtool/orc_dump.c b/tools/objtool/orc_dump.c
index fca46e006fc2..911ca17e4865 100644
--- a/tools/objtool/orc_dump.c
+++ b/tools/objtool/orc_dump.c
@@ -4,6 +4,7 @@
*/

#include <unistd.h>
+#include <linux/objtool.h>
#include <asm/orc_types.h>
#include "objtool.h"
#include "warn.h"
@@ -37,11 +38,11 @@ static const char *reg_name(unsigned int reg)
static const char *orc_type_name(unsigned int type)
{
switch (type) {
- case ORC_TYPE_CALL:
+ case UNWIND_HINT_TYPE_CALL:
return "call";
- case ORC_TYPE_REGS:
+ case UNWIND_HINT_TYPE_REGS:
return "regs";
- case ORC_TYPE_REGS_IRET:
+ case UNWIND_HINT_TYPE_REGS_IRET:
return "iret";
default:
return "?";
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index 66fd56c33303..597ecffead92 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -6,6 +6,9 @@
#include <stdlib.h>
#include <string.h>

+#include <linux/objtool.h>
+#include <asm/orc_types.h>
+
#include "check.h"
#include "warn.h"

@@ -146,7 +149,7 @@ int create_orc_sections(struct objtool_file *file)
struct orc_entry empty = {
.sp_reg = ORC_REG_UNDEFINED,
.bp_reg = ORC_REG_UNDEFINED,
- .type = ORC_TYPE_CALL,
+ .type = UNWIND_HINT_TYPE_CALL,
};

sec = find_section_by_name(file->elf, ".orc_unwind");
diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh
index 07249900db1c..4c935bde8960 100755
--- a/tools/objtool/sync-check.sh
+++ b/tools/objtool/sync-check.sh
@@ -3,8 +3,10 @@

TARGET_ARCH=$1

+FILES="include/linux/objtool.h"
+
if [ "$TARGET_ARCH" == "x86" ]; then
-FILES="
+FILES="$FILES
arch/x86/include/asm/inat_types.h
arch/x86/include/asm/orc_types.h
arch/x86/include/asm/emulate_prefix.h
--
2.21.3

2020-09-04 18:50:04

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v3 09/10] objtool: Make unwind hints definitions available to other architectures

On Fri, Sep 04, 2020 at 04:30:27PM +0100, Julien Thierry wrote:
> +/*
> + * UNWIND_HINT_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.
> + *
> + * UNWIND_HINT_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.
> + *
> + * UNWIND_HINT_TYPE_REGS_IRET: Used in entry code to indicate that sp_reg+sp_offset
> + * points to the iret return frame.

Now that this is generic, I think REGS_PARTIAL would be better.

> + *
> + * 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.

Now that ORC_TYPE_* have been replaced by UNWIND_HINT_TYPE_*, I think
this last paragraph should be removed.

--
Josh

2020-09-04 18:58:57

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v3 00/10] Make check implementation arch agnostic

On Fri, Sep 04, 2020 at 04:30:18PM +0100, Julien Thierry wrote:
> Hi,
>
> The current implementation of the check subcommand has various x86 bits
> here and there. In order to prepare objtool to provide check for other
> architectures, add some abstraction over the x86 specific bits, relying
> on objtool arch specific code to provide some necessary operations.
>
> This is part of the effort to implement check for arm64, initiated [1]
> by Raphael. The series is based on top of the separation of check & orc
> subcommands series[2].
>
> I've push both series base on top of tip/objtool/core at [3].
>
> - The first two patches make it simpler for new arches to provide their
> list of kernel headers, without worrying about modifications in the x86
> headers.
> - Patch 3 Moves arch specific macros to more suitable location
> - Patches 4 and 5 add abstraction to handle alternatives
> - Patch 6 adds abstraction to handle jump table
> - Patches 7-10 makes unwind hint definitions shared across architectures
>
> Changes since v2 [4]:
> - Rebased on v5.9-rc1
> - Under tools/objtool/arch/x86/, rename arch_special.c to special.c
> - Rename include/linux/frame.h to inclide/linux/objtool.h
> - Share unwind hint types across architectures

Thanks. These look good. We're still trying to get our merge process
worked out, and tip/objtool/core is now pretty old, but these apply well
enough.

If there are no more comments I can fix up the few minor comments I had
and then try to get them merged after your other set (once Peter and I
figure out how to do that :-)

--
Josh

2020-09-08 19:52:41

by Julien Thierry

[permalink] [raw]
Subject: [PATCH] objtool: Fix sync-check.sh bashisms

Previous patches introduced some non SUS compliant changes
to sync-check.sh.

Replace used bash features for standard shell.

Signed-off-by: Julien Thierry <[email protected]>
---
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 <<EOF
+$FILES
+EOF
--
2.21.3

2020-09-11 10:46:47

by Miroslav Benes

[permalink] [raw]
Subject: Re: [PATCH v3 00/10] Make check implementation arch agnostic

On Fri, 4 Sep 2020, Julien Thierry wrote:

> Hi,
>
> The current implementation of the check subcommand has various x86 bits
> here and there. In order to prepare objtool to provide check for other
> architectures, add some abstraction over the x86 specific bits, relying
> on objtool arch specific code to provide some necessary operations.
>
> This is part of the effort to implement check for arm64, initiated [1]
> by Raphael. The series is based on top of the separation of check & orc
> subcommands series[2].
>
> I've push both series base on top of tip/objtool/core at [3].
>
> - The first two patches make it simpler for new arches to provide their
> list of kernel headers, without worrying about modifications in the x86
> headers.
> - Patch 3 Moves arch specific macros to more suitable location
> - Patches 4 and 5 add abstraction to handle alternatives
> - Patch 6 adds abstraction to handle jump table
> - Patches 7-10 makes unwind hint definitions shared across architectures
>
> Changes since v2 [4]:
> - Rebased on v5.9-rc1
> - Under tools/objtool/arch/x86/, rename arch_special.c to special.c
> - Rename include/linux/frame.h to inclide/linux/objtool.h
> - Share unwind hint types across architectures
>
> [1] https://lkml.org/lkml/2019/8/16/400
> [2] https://lkml.org/lkml/2020/6/4/675
> [3] https://github.com/julien-thierry/linux/tree/arch-independent-check
> [4] https://lkml.org/lkml/2020/7/30/424

Hi,

Josh merged the patch set already, but FWIW

Reviewed-by: Miroslav Benes <[email protected]>

for the new changes (patches 7, 9 and 10).

Miroslav