* [PATCH v4 01/15] Revert "powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto"
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 02/15] objtool: Move back misplaced comment Christophe Leroy
` (15 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
This reverts commit 1e688dd2a3d6759d416616ff07afc4bb836c4213.
That commit aimed at optimising the code around generation of
WARN_ON/BUG_ON but this leads to a lot of dead code erroneously
generated by GCC.
That dead code becomes a problem when we start using objtool validation
because objtool will abort validation with a warning as soon as it
detects unreachable code. This is because unreachable code might
be the indication that objtool doesn't properly decode object text.
text data bss dec hex filename
9551585 3627834 224376 13403795 cc8693 vmlinux.before
9535281 3628358 224376 13388015 cc48ef vmlinux.after
Once this change is reverted, in a standard configuration (pmac32 +
function tracer) the text is reduced by 16k which is around 1.7%
We already had problem with it when starting to use objtool on powerpc
as a replacement for recordmcount, see commit 93e3f45a2631 ("powerpc:
Fix __WARN_FLAGS() for use with Objtool")
There is also a problem with at least GCC 12, on ppc64_defconfig +
CONFIG_CC_OPTIMIZE_FOR_SIZE=y + CONFIG_DEBUG_SECTION_MISMATCH=y :
LD .tmp_vmlinux.kallsyms1
powerpc64-linux-ld: net/ipv4/tcp_input.o:(__ex_table+0xc4): undefined reference to `.L2136'
make[2]: *** [scripts/Makefile.vmlinux:36: vmlinux] Error 1
make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1238: vmlinux] Error 2
Taking into account that other problems are encountered with that
'asm goto' in WARN_ON(), including build failures, keeping that
change is not worth it allthough it is primarily a compiler bug.
Revert it for now.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Naveen N Rao <naveen@kernel.org>
---
arch/powerpc/include/asm/book3s/64/kup.h | 2 +-
arch/powerpc/include/asm/bug.h | 67 ++++--------------------
arch/powerpc/kernel/misc_32.S | 2 +-
arch/powerpc/kernel/traps.c | 9 +---
4 files changed, 15 insertions(+), 65 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 497a7bd31ecc..e875cb7e68dc 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -90,7 +90,7 @@
/* Prevent access to userspace using any key values */
LOAD_REG_IMMEDIATE(\gpr2, AMR_KUAP_BLOCKED)
999: tdne \gpr1, \gpr2
- EMIT_WARN_ENTRY 999b, __FILE__, __LINE__, (BUGFLAG_WARNING | BUGFLAG_ONCE)
+ EMIT_BUG_ENTRY 999b, __FILE__, __LINE__, (BUGFLAG_WARNING | BUGFLAG_ONCE)
END_MMU_FTR_SECTION_NESTED_IFSET(MMU_FTR_KUAP, 67)
#endif
.endm
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 492530adecc2..abb608dff15a 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -4,14 +4,13 @@
#ifdef __KERNEL__
#include <asm/asm-compat.h>
-#include <asm/extable.h>
#ifdef CONFIG_BUG
#ifdef __ASSEMBLY__
#include <asm/asm-offsets.h>
#ifdef CONFIG_DEBUG_BUGVERBOSE
-.macro __EMIT_BUG_ENTRY addr,file,line,flags
+.macro EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
.4byte 5002f - .
@@ -23,7 +22,7 @@
.previous
.endm
#else
-.macro __EMIT_BUG_ENTRY addr,file,line,flags
+.macro EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
.short \flags
@@ -32,18 +31,6 @@
.endm
#endif /* verbose */
-.macro EMIT_WARN_ENTRY addr,file,line,flags
- EX_TABLE(\addr,\addr+4)
- __EMIT_BUG_ENTRY \addr,\file,\line,\flags
-.endm
-
-.macro EMIT_BUG_ENTRY addr,file,line,flags
- .if \flags & 1 /* BUGFLAG_WARNING */
- .err /* Use EMIT_WARN_ENTRY for warnings */
- .endif
- __EMIT_BUG_ENTRY \addr,\file,\line,\flags
-.endm
-
#else /* !__ASSEMBLY__ */
/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
sizeof(struct bug_entry), respectively */
@@ -73,16 +60,6 @@
"i" (sizeof(struct bug_entry)), \
##__VA_ARGS__)
-#define WARN_ENTRY(insn, flags, label, ...) \
- asm_volatile_goto( \
- "1: " insn "\n" \
- EX_TABLE(1b, %l[label]) \
- _EMIT_BUG_ENTRY \
- : : "i" (__FILE__), "i" (__LINE__), \
- "i" (flags), \
- "i" (sizeof(struct bug_entry)), \
- ##__VA_ARGS__ : : label)
-
/*
* BUG_ON() and WARN_ON() do their best to cooperate with compile-time
* optimisations. However depending on the complexity of the condition
@@ -95,16 +72,7 @@
} while (0)
#define HAVE_ARCH_BUG
-#define __WARN_FLAGS(flags) do { \
- __label__ __label_warn_on; \
- \
- WARN_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags), __label_warn_on); \
- barrier_before_unreachable(); \
- __builtin_unreachable(); \
- \
-__label_warn_on: \
- break; \
-} while (0)
+#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags))
#ifdef CONFIG_PPC64
#define BUG_ON(x) do { \
@@ -117,25 +85,15 @@ __label_warn_on: \
} while (0)
#define WARN_ON(x) ({ \
- bool __ret_warn_on = false; \
- do { \
- if (__builtin_constant_p((x))) { \
- if (!(x)) \
- break; \
+ int __ret_warn_on = !!(x); \
+ if (__builtin_constant_p(__ret_warn_on)) { \
+ if (__ret_warn_on) \
__WARN(); \
- __ret_warn_on = true; \
- } else { \
- __label__ __label_warn_on; \
- \
- WARN_ENTRY(PPC_TLNEI " %4, 0", \
- BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
- __label_warn_on, \
- "r" ((__force long)(x))); \
- break; \
-__label_warn_on: \
- __ret_warn_on = true; \
- } \
- } while (0); \
+ } else { \
+ BUG_ENTRY(PPC_TLNEI " %4, 0", \
+ BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
+ "r" (__ret_warn_on)); \
+ } \
unlikely(__ret_warn_on); \
})
@@ -148,11 +106,8 @@ __label_warn_on: \
#ifdef __ASSEMBLY__
.macro EMIT_BUG_ENTRY addr,file,line,flags
.endm
-.macro EMIT_WARN_ENTRY addr,file,line,flags
-.endm
#else /* !__ASSEMBLY__ */
#define _EMIT_BUG_ENTRY
-#define _EMIT_WARN_ENTRY
#endif
#endif /* CONFIG_BUG */
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index daf8f87d2372..fd11ec42df89 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -237,7 +237,7 @@ _GLOBAL(copy_page)
addi r3,r3,-4
0: twnei r5, 0 /* WARN if r3 is not cache aligned */
- EMIT_WARN_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
+ EMIT_BUG_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
addi r4,r4,-4
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index ab95105c69ca..f5ce282dc4b8 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1508,13 +1508,8 @@ static void do_program_check(struct pt_regs *regs)
if (!(regs->msr & MSR_PR) && /* not user-mode */
report_bug(bugaddr, regs) == BUG_TRAP_TYPE_WARN) {
- const struct exception_table_entry *entry;
-
- entry = search_exception_tables(bugaddr);
- if (entry) {
- regs_set_return_ip(regs, extable_fixup(entry) + regs->nip - bugaddr);
- return;
- }
+ regs_add_return_ip(regs, 4);
+ return;
}
if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE) && user_mode(regs)) {
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 02/15] objtool: Move back misplaced comment
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 01/15] Revert "powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto" Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 03/15] objtool: Allow an architecture to disable objtool on ASM files Christophe Leroy
` (14 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
A comment was introduced by commit 113d4bc90483 ("objtool: Fix
clang switch table edge case") and wrongly moved by
commit d871f7b5a6a2 ("objtool: Refactor jump table code to support
other architectures") without the piece of code added with the
comment in the original commit.
Fixes: d871f7b5a6a2 ("objtool: Refactor jump table code to support other architectures")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/objtool/arch/x86/special.c | 5 -----
tools/objtool/check.c | 6 ++++++
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index 29e949579ede..8e8302fe909f 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -118,11 +118,6 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
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;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8936a05f0e5a..25f6df4713ed 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2072,6 +2072,12 @@ static struct reloc *find_jump_table(struct objtool_file *file,
table_reloc = arch_find_switch_table(file, insn);
if (!table_reloc)
continue;
+
+ /*
+ * Each table entry has a rela associated with it. The rela
+ * should reference text in the same function as the original
+ * instruction.
+ */
dest_insn = find_insn(file, table_reloc->sym->sec, reloc_addend(table_reloc));
if (!dest_insn || !insn_func(dest_insn) || insn_func(dest_insn)->pfunc != func)
continue;
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 03/15] objtool: Allow an architecture to disable objtool on ASM files
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 01/15] Revert "powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto" Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 02/15] objtool: Move back misplaced comment Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 04/15] objtool: Fix JUMP_ENTRY_SIZE for bi-arch like powerpc Christophe Leroy
` (13 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
Supporting objtool on ASM files requires quite an effort.
Features like UACCESS validation don't require ASM files validation.
In order to allow architectures to enable objtool validation
without spending unnecessary effort on cleaning up ASM files,
provide an option to disable objtool validation on ASM files.
Suggested-by: Naveen N Rao <naveen@kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/Kconfig | 5 +++++
scripts/Makefile.build | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index aff2746c8af2..3330ed761260 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1111,6 +1111,11 @@ config ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
config HAVE_OBJTOOL
bool
+config ARCH_OBJTOOL_SKIP_ASM
+ bool
+ help
+ Architecture doesn't support objtool on ASM files
+
config HAVE_JUMP_LABEL_HACK
bool
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 6413342a03f4..5818baddfb27 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -342,7 +342,11 @@ $(obj)/%.s: $(src)/%.S FORCE
$(call if_changed_dep,cpp_s_S)
quiet_cmd_as_o_S = AS $(quiet_modtag) $@
+ifndef CONFIG_ARCH_OBJTOOL_SKIP_ASM
cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< $(cmd_objtool)
+else
+ cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
+endif
ifdef CONFIG_ASM_MODVERSIONS
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 04/15] objtool: Fix JUMP_ENTRY_SIZE for bi-arch like powerpc
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (2 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 03/15] objtool: Allow an architecture to disable objtool on ASM files Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 05/15] objtool: Add INSN_RETURN_CONDITIONAL Christophe Leroy
` (12 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
struct jump_entry {
s32 code;
s32 target;
long key;
};
It means that the size of the third argument depends on
whether we are building a 32 bits or 64 bits kernel.
Therefore JUMP_ENTRY_SIZE must depend on elf_class_addrsize(elf).
To allow that, entries[] table must be initialised at runtime. This is
easily done by moving it into its only user which is special_get_alts().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
.../arch/powerpc/include/arch/special.h | 2 +-
tools/objtool/special.c | 55 +++++++++----------
2 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/tools/objtool/arch/powerpc/include/arch/special.h b/tools/objtool/arch/powerpc/include/arch/special.h
index ffef9ada7133..b17802dcf436 100644
--- a/tools/objtool/arch/powerpc/include/arch/special.h
+++ b/tools/objtool/arch/powerpc/include/arch/special.h
@@ -6,7 +6,7 @@
#define EX_ORIG_OFFSET 0
#define EX_NEW_OFFSET 4
-#define JUMP_ENTRY_SIZE 16
+#define JUMP_ENTRY_SIZE (8 + elf_addr_size(elf)) /* 12 on PPC32, 16 on PPC64 */
#define JUMP_ORIG_OFFSET 0
#define JUMP_NEW_OFFSET 4
#define JUMP_KEY_OFFSET 8
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index 91b1950f5bd8..b3f07e8beb85 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -26,34 +26,6 @@ struct special_entry {
unsigned char key; /* jump_label key */
};
-static const struct special_entry entries[] = {
- {
- .sec = ".altinstructions",
- .group = true,
- .size = ALT_ENTRY_SIZE,
- .orig = ALT_ORIG_OFFSET,
- .orig_len = ALT_ORIG_LEN_OFFSET,
- .new = ALT_NEW_OFFSET,
- .new_len = ALT_NEW_LEN_OFFSET,
- .feature = ALT_FEATURE_OFFSET,
- },
- {
- .sec = "__jump_table",
- .jump_or_nop = true,
- .size = JUMP_ENTRY_SIZE,
- .orig = JUMP_ORIG_OFFSET,
- .new = JUMP_NEW_OFFSET,
- .key = JUMP_KEY_OFFSET,
- },
- {
- .sec = "__ex_table",
- .size = EX_ENTRY_SIZE,
- .orig = EX_ORIG_OFFSET,
- .new = EX_NEW_OFFSET,
- },
- {},
-};
-
void __weak arch_handle_alternative(unsigned short feature, struct special_alt *alt)
{
}
@@ -144,6 +116,33 @@ int special_get_alts(struct elf *elf, struct list_head *alts)
unsigned int nr_entries;
struct special_alt *alt;
int idx, ret;
+ const struct special_entry entries[] = {
+ {
+ .sec = ".altinstructions",
+ .group = true,
+ .size = ALT_ENTRY_SIZE,
+ .orig = ALT_ORIG_OFFSET,
+ .orig_len = ALT_ORIG_LEN_OFFSET,
+ .new = ALT_NEW_OFFSET,
+ .new_len = ALT_NEW_LEN_OFFSET,
+ .feature = ALT_FEATURE_OFFSET,
+ },
+ {
+ .sec = "__jump_table",
+ .jump_or_nop = true,
+ .size = JUMP_ENTRY_SIZE,
+ .orig = JUMP_ORIG_OFFSET,
+ .new = JUMP_NEW_OFFSET,
+ .key = JUMP_KEY_OFFSET,
+ },
+ {
+ .sec = "__ex_table",
+ .size = EX_ENTRY_SIZE,
+ .orig = EX_ORIG_OFFSET,
+ .new = EX_NEW_OFFSET,
+ },
+ {},
+ };
INIT_LIST_HEAD(alts);
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 05/15] objtool: Add INSN_RETURN_CONDITIONAL
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (3 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 04/15] objtool: Fix JUMP_ENTRY_SIZE for bi-arch like powerpc Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 06/15] objtool: Add support for relative switch tables Christophe Leroy
` (11 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
Most functions have an unconditional return at the end, like
this one:
00000000 <is_exec_fault>:
0: 81 22 04 d0 lwz r9,1232(r2)
4: 38 60 00 00 li r3,0
8: 2c 09 00 00 cmpwi r9,0
c: 4d 82 00 20 beqlr <== Conditional return
10: 80 69 00 a0 lwz r3,160(r9)
14: 54 63 00 36 clrrwi r3,r3,4
18: 68 63 04 00 xori r3,r3,1024
1c: 7c 63 00 34 cntlzw r3,r3
20: 54 63 d9 7e srwi r3,r3,5
24: 4e 80 00 20 blr <== Unconditional return
But other functions like this other one below only have
conditional returns:
00000028 <pte_update.isra.0>:
28: 81 25 00 00 lwz r9,0(r5)
2c: 2c 08 00 00 cmpwi r8,0
30: 7d 29 30 78 andc r9,r9,r6
34: 7d 27 3b 78 or r7,r9,r7
38: 54 84 65 3a rlwinm r4,r4,12,20,29
3c: 81 23 00 18 lwz r9,24(r3)
40: 41 82 00 58 beq 98 <pte_update.isra.0+0x70>
44: 7d 29 20 2e lwzx r9,r9,r4
48: 55 29 07 3a rlwinm r9,r9,0,28,29
4c: 2c 09 00 0c cmpwi r9,12
50: 41 82 00 08 beq 58 <pte_update.isra.0+0x30>
54: 39 00 00 80 li r8,128
58: 2c 08 00 01 cmpwi r8,1
5c: 90 e5 00 00 stw r7,0(r5)
60: 4d a2 00 20 beqlr+ <== Conditional return
64: 7c e9 3b 78 mr r9,r7
68: 39 40 00 00 li r10,0
6c: 39 4a 00 04 addi r10,r10,4
70: 7c 0a 40 00 cmpw r10,r8
74: 91 25 00 04 stw r9,4(r5)
78: 91 25 00 08 stw r9,8(r5)
7c: 38 a5 00 10 addi r5,r5,16
80: 91 25 ff fc stw r9,-4(r5)
84: 4c 80 00 20 bgelr <== Conditional return
88: 55 49 60 26 slwi r9,r10,12
8c: 7d 29 3a 14 add r9,r9,r7
90: 91 25 00 00 stw r9,0(r5)
94: 4b ff ff d8 b 6c <pte_update.isra.0+0x44>
98: 39 00 00 04 li r8,4
9c: 4b ff ff bc b 58 <pte_update.isra.0+0x30>
If conditional returns are decoded as INSN_OTHER, objtool considers
that the second function never returns.
If conditional returns are decoded as INSN_RETURN, objtool considers
that code after that conditional return is dead.
To overcome this situation, introduce INSN_RETURN_CONDITIONAL which
is taken as a confirmation that a function is not noreturn but still
sees following code as reachable.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/objtool/check.c | 2 +-
tools/objtool/include/objtool/arch.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 25f6df4713ed..ae0019412123 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -219,7 +219,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
func_for_each_insn(file, func, insn) {
empty = false;
- if (insn->type == INSN_RETURN)
+ if (insn->type == INSN_RETURN || insn->type == INSN_RETURN_CONDITIONAL)
return false;
}
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 2b6d2ce4f9a5..84ba75112934 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -19,6 +19,7 @@ enum insn_type {
INSN_CALL,
INSN_CALL_DYNAMIC,
INSN_RETURN,
+ INSN_RETURN_CONDITIONAL,
INSN_CONTEXT_SWITCH,
INSN_BUG,
INSN_NOP,
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 06/15] objtool: Add support for relative switch tables
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (4 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 05/15] objtool: Add INSN_RETURN_CONDITIONAL Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 07/15] objtool: Merge mark_func_jump_tables() and add_func_jump_tables() Christophe Leroy
` (10 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
On powerpc, switch tables are relative, than means the address of the
table is added to the value of the entry in order to get the pointed
address: (r10 is the table address, r4 the index in the table)
lis r10,0 <== Load r10 with upper part of .rodata address
R_PPC_ADDR16_HA .rodata
addi r10,r10,0 <== Add lower part of .rodata address
R_PPC_ADDR16_LO .rodata
lwzx r8,r10,r4 <== Read table entry at r10 + r4 into r8
add r10,r8,r10 <== Add table address to read value
mtctr r10 <== Save calculated address in CTR
bctr <== Branch to address in CTR
RELOCATION RECORDS FOR [.rodata]:
OFFSET TYPE VALUE
00000000 R_PPC_REL32 .text+0x0000054c
00000004 R_PPC_REL32 .text+0x000003d0
...
But for c_jump_tables it is not the case, they contain the
pointed address directly:
lis r28,0 <== Load r28 with upper .rodata..c_jump_table
R_PPC_ADDR16_HA .rodata..c_jump_table
addi r28,r28,0 <== Add lower part of .rodata..c_jump_table
R_PPC_ADDR16_LO .rodata..c_jump_table
lwzx r10,r28,r10 <== Read table entry at r10 + r28 into r10
mtctr r10 <== Save read value in CTR
bctr <== Branch to address in CTR
RELOCATION RECORDS FOR [.rodata..c_jump_table]:
OFFSET TYPE VALUE
00000000 R_PPC_ADDR32 .text+0x00000dc8
00000004 R_PPC_ADDR32 .text+0x00000dc8
...
Add support to objtool for relative tables, based on the relocation
type which is R_PPC_REL32 for switch tables and R_PPC_ADDR32 for
C jump tables. Do the comparison using R_ABS32 and R_ABS64 which are
architecture agnostic.
And use correct size for 'long' instead of hard coding a size of '8'.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/objtool/check.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index ae0019412123..5a6a87ddbf27 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1988,7 +1988,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
struct symbol *pfunc = insn_func(insn)->pfunc;
struct reloc *table = insn_jump_table(insn);
struct instruction *dest_insn;
- unsigned int prev_offset = 0;
+ unsigned int offset, prev_offset = 0;
struct reloc *reloc = table;
struct alternative *alt;
@@ -2003,7 +2003,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
break;
/* Make sure the table entries are consecutive: */
- if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
+ if (prev_offset && reloc_offset(reloc) != prev_offset + elf_addr_size(file->elf))
break;
/* Detect function pointers from contiguous objects: */
@@ -2011,7 +2011,12 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
reloc_addend(reloc) == pfunc->offset)
break;
- dest_insn = find_insn(file, reloc->sym->sec, reloc_addend(reloc));
+ if (reloc_type(reloc) == R_ABS32 || reloc_type(reloc) == R_ABS64)
+ offset = reloc_addend(reloc);
+ else
+ offset = reloc_addend(reloc) + reloc_offset(table) - reloc_offset(reloc);
+
+ dest_insn = find_insn(file, reloc->sym->sec, offset);
if (!dest_insn)
break;
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 07/15] objtool: Merge mark_func_jump_tables() and add_func_jump_tables()
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (5 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 06/15] objtool: Add support for relative switch tables Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 08/15] objtool: Track general purpose register used for switch table base Christophe Leroy
` (9 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
Those two functions loop over the instructions of a function.
Merge the two loops in order to ease enhancement of table end
in a following patch.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/objtool/check.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5a6a87ddbf27..d51f47c4a3bd 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2097,11 +2097,12 @@ static struct reloc *find_jump_table(struct objtool_file *file,
* First pass: Mark the head of each jump table so that in the next pass,
* we know when a given jump table ends and the next one starts.
*/
-static void mark_func_jump_tables(struct objtool_file *file,
- struct symbol *func)
+static int mark_add_func_jump_tables(struct objtool_file *file,
+ struct symbol *func)
{
- struct instruction *insn, *last = NULL;
+ struct instruction *insn, *last = NULL, *insn_t1 = NULL, *insn_t2;
struct reloc *reloc;
+ int ret = 0;
func_for_each_insn(file, func, insn) {
if (!last)
@@ -2127,17 +2128,7 @@ static void mark_func_jump_tables(struct objtool_file *file,
reloc = find_jump_table(file, func, insn);
if (reloc)
insn->_jump_table = reloc;
- }
-}
-
-static int add_func_jump_tables(struct objtool_file *file,
- struct symbol *func)
-{
- struct instruction *insn, *insn_t1 = NULL, *insn_t2;
- int ret = 0;
-
- func_for_each_insn(file, func, insn) {
- if (!insn_jump_table(insn))
+ else
continue;
if (!insn_t1) {
@@ -2177,8 +2168,7 @@ static int add_jump_table_alts(struct objtool_file *file)
if (func->type != STT_FUNC)
continue;
- mark_func_jump_tables(file, func);
- ret = add_func_jump_tables(file, func);
+ ret = mark_add_func_jump_tables(file, func);
if (ret)
return ret;
}
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 08/15] objtool: Track general purpose register used for switch table base
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (6 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 07/15] objtool: Merge mark_func_jump_tables() and add_func_jump_tables() Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 09/15] objtool: Find end of switch table directly Christophe Leroy
` (8 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
A function can contain nested switch tables using different registers
as base address.
In order to avoid failure in tracking those switch tables, the register
containing the base address needs to be taken into account.
To do so, add a 5 bits field in struct instruction that will hold the
ID of the register containing the base address of the switch table and
take that register into account during the backward search in order to
not stop the walk when encountering a jump related to another switch
table.
On architectures not handling it, the ID stays nul and has no impact
on the search.
To enable that, also provide to arch_find_switch_table() the dynamic
instruction related to a table search.
Also allow prev_insn_same_sec() to be used outside check.c so that
architectures can backward walk through instruction to find out which
register is used as base address for a switch table.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/objtool/arch/powerpc/special.c | 3 ++-
tools/objtool/arch/x86/special.c | 3 ++-
tools/objtool/check.c | 9 +++++----
tools/objtool/include/objtool/check.h | 6 ++++--
tools/objtool/include/objtool/special.h | 3 ++-
5 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/tools/objtool/arch/powerpc/special.c b/tools/objtool/arch/powerpc/special.c
index d33868147196..a7dd2559b536 100644
--- a/tools/objtool/arch/powerpc/special.c
+++ b/tools/objtool/arch/powerpc/special.c
@@ -13,7 +13,8 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
}
struct reloc *arch_find_switch_table(struct objtool_file *file,
- struct instruction *insn)
+ struct instruction *insn,
+ struct instruction *orig_insn)
{
exit(-1);
}
diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index 8e8302fe909f..8cf17d94c69b 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -86,7 +86,8 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
* NOTE: RETPOLINE made it harder still to decode dynamic jumps.
*/
struct reloc *arch_find_switch_table(struct objtool_file *file,
- struct instruction *insn)
+ struct instruction *insn,
+ struct instruction *orig_insn)
{
struct reloc *text_reloc, *rodata_reloc;
struct section *table_sec;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d51f47c4a3bd..be413c578588 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -80,8 +80,8 @@ static struct instruction *next_insn_same_func(struct objtool_file *file,
return find_insn(file, func->cfunc->sec, func->cfunc->offset);
}
-static struct instruction *prev_insn_same_sec(struct objtool_file *file,
- struct instruction *insn)
+struct instruction *prev_insn_same_sec(struct objtool_file *file,
+ struct instruction *insn)
{
if (insn->idx == 0) {
if (insn->prev_len)
@@ -2064,7 +2064,8 @@ static struct reloc *find_jump_table(struct objtool_file *file,
insn && insn_func(insn) && insn_func(insn)->pfunc == func;
insn = insn->first_jump_src ?: prev_insn_same_sym(file, insn)) {
- if (insn != orig_insn && insn->type == INSN_JUMP_DYNAMIC)
+ if (insn != orig_insn && insn->type == INSN_JUMP_DYNAMIC &&
+ insn->gpr == orig_insn->gpr)
break;
/* allow small jumps within the range */
@@ -2074,7 +2075,7 @@ static struct reloc *find_jump_table(struct objtool_file *file,
insn->jump_dest->offset > orig_insn->offset))
break;
- table_reloc = arch_find_switch_table(file, insn);
+ table_reloc = arch_find_switch_table(file, insn, orig_insn);
if (!table_reloc)
continue;
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index daa46f1f0965..660ea9d0393e 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -63,8 +63,9 @@ struct instruction {
noendbr : 1,
unret : 1,
visited : 4,
- no_reloc : 1;
- /* 10 bit hole */
+ no_reloc : 1,
+ gpr : 5;
+ /* 5 bit hole */
struct alt_group *alt_group;
struct instruction *jump_dest;
@@ -115,6 +116,7 @@ struct instruction *find_insn(struct objtool_file *file,
struct section *sec, unsigned long offset);
struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruction *insn);
+struct instruction *prev_insn_same_sec(struct objtool_file *file, struct instruction *insn);
#define sec_for_each_insn(file, _sec, insn) \
for (insn = find_insn(file, _sec, 0); \
diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h
index 86d4af9c5aa9..4128a479d76e 100644
--- a/tools/objtool/include/objtool/special.h
+++ b/tools/objtool/include/objtool/special.h
@@ -38,5 +38,6 @@ 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);
+ struct instruction *insn,
+ struct instruction *orig_insn);
#endif /* _SPECIAL_H */
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 09/15] objtool: Find end of switch table directly
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (7 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 08/15] objtool: Track general purpose register used for switch table base Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 10/15] objtool: When looking for switch tables also follow conditional and dynamic jumps Christophe Leroy
` (7 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
At the time being, the end of a switch table can only be known
once the start of the following switch table has ben located.
This is a problem when switch tables are nested because until the first
switch table is properly added, the second one cannot be located as a
the backward walk will abut on the dynamic switch of the previous one.
So perform a first forward walk in the code in order to locate all
possible relocations to switch tables and build a local table with
those relocations. Later on once one switch table is found, go through
this local table to know where next switch table starts.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/objtool/check.c | 62 ++++++++++++++++++++++++++++++++-----------
1 file changed, 46 insertions(+), 16 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index be413c578588..361c832aefc8 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2094,14 +2094,30 @@ static struct reloc *find_jump_table(struct objtool_file *file,
return NULL;
}
+static struct reloc *find_next_table(struct instruction *insn,
+ struct reloc **table, unsigned int size)
+{
+ unsigned long offset = reloc_offset(insn_jump_table(insn));
+ int i;
+ struct reloc *reloc = NULL;
+
+ for (i = 0; i < size; i++) {
+ if (reloc_offset(table[i]) > offset &&
+ (!reloc || reloc_offset(table[i]) < reloc_offset(reloc)))
+ reloc = table[i];
+ }
+ return reloc;
+}
+
/*
* First pass: Mark the head of each jump table so that in the next pass,
* we know when a given jump table ends and the next one starts.
*/
static int mark_add_func_jump_tables(struct objtool_file *file,
- struct symbol *func)
+ struct symbol *func,
+ struct reloc **table, unsigned int size)
{
- struct instruction *insn, *last = NULL, *insn_t1 = NULL, *insn_t2;
+ struct instruction *insn, *last = NULL;
struct reloc *reloc;
int ret = 0;
@@ -2132,23 +2148,11 @@ static int mark_add_func_jump_tables(struct objtool_file *file,
else
continue;
- if (!insn_t1) {
- insn_t1 = insn;
- continue;
- }
-
- insn_t2 = insn;
-
- ret = add_jump_table(file, insn_t1, insn_jump_table(insn_t2));
+ ret = add_jump_table(file, insn, find_next_table(insn, table, size));
if (ret)
return ret;
-
- insn_t1 = insn_t2;
}
- if (insn_t1)
- ret = add_jump_table(file, insn_t1, NULL);
-
return ret;
}
@@ -2161,15 +2165,41 @@ static int add_jump_table_alts(struct objtool_file *file)
{
struct symbol *func;
int ret;
+ struct instruction *insn;
+ unsigned int size = 0, i = 0;
+ struct reloc **table = NULL;
if (!file->rodata)
return 0;
+ for_each_insn(file, insn) {
+ struct instruction *dest_insn;
+ struct reloc *reloc;
+
+ func = insn_func(insn) ? insn_func(insn)->pfunc : NULL;
+ reloc = arch_find_switch_table(file, insn, NULL);
+ /*
+ * Each table entry has a rela associated with it. The rela
+ * should reference text in the same function as the original
+ * instruction.
+ */
+ if (!reloc)
+ continue;
+ dest_insn = find_insn(file, reloc->sym->sec, reloc_addend(reloc));
+ if (!dest_insn || !insn_func(dest_insn) || insn_func(dest_insn)->pfunc != func)
+ continue;
+ if (i == size) {
+ size += 1024;
+ table = realloc(table, size * sizeof(*table));
+ }
+ table[i++] = reloc;
+ }
+
for_each_sym(file, func) {
if (func->type != STT_FUNC)
continue;
- ret = mark_add_func_jump_tables(file, func);
+ ret = mark_add_func_jump_tables(file, func, table, i);
if (ret)
return ret;
}
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 10/15] objtool: When looking for switch tables also follow conditional and dynamic jumps
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (8 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 09/15] objtool: Find end of switch table directly Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 11/15] objtool: .rodata.cst{2/4/8/16} are not switch tables Christophe Leroy
` (6 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
When walking backward to find the base address of a switch table,
also take into account conditionnal branches and dynamic jumps from
a previous switch table.
To avoid mis-routing, break when stumbling on a function return.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/objtool/check.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 361c832aefc8..ea0945f2195f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2034,6 +2034,8 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
alt->next = insn->alts;
insn->alts = alt;
prev_offset = reloc_offset(reloc);
+ if (!dest_insn->first_jump_src)
+ dest_insn->first_jump_src = insn;
}
if (!prev_offset) {
@@ -2068,6 +2070,9 @@ static struct reloc *find_jump_table(struct objtool_file *file,
insn->gpr == orig_insn->gpr)
break;
+ if (insn->type == INSN_RETURN)
+ break;
+
/* allow small jumps within the range */
if (insn->type == INSN_JUMP_UNCONDITIONAL &&
insn->jump_dest &&
@@ -2130,8 +2135,7 @@ static int mark_add_func_jump_tables(struct objtool_file *file,
* that find_jump_table() can back-track using those and
* avoid some potentially confusing code.
*/
- if (insn->type == INSN_JUMP_UNCONDITIONAL && insn->jump_dest &&
- insn->offset > last->offset &&
+ if (is_static_jump(insn) && insn->jump_dest &&
insn->jump_dest->offset > insn->offset &&
!insn->jump_dest->first_jump_src) {
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 11/15] objtool: .rodata.cst{2/4/8/16} are not switch tables
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (9 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 10/15] objtool: When looking for switch tables also follow conditional and dynamic jumps Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 12/15] objtool: Add support for more complex UACCESS control Christophe Leroy
` (5 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
Exclude sections named
.rodata.cst2
.rodata.cst4
.rodata.cst8
.rodata.cst16
as they won't contain switch tables.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/objtool/check.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index ea0945f2195f..d2a0dfec5909 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2565,7 +2565,8 @@ static void mark_rodata(struct objtool_file *file)
*/
for_each_sec(file, sec) {
if (!strncmp(sec->name, ".rodata", 7) &&
- !strstr(sec->name, ".str1.")) {
+ !strstr(sec->name, ".str1.") &&
+ !strstr(sec->name, ".cst")) {
sec->rodata = true;
found = true;
}
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 12/15] objtool: Add support for more complex UACCESS control
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (10 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 11/15] objtool: .rodata.cst{2/4/8/16} are not switch tables Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 13/15] objtool: Prepare noreturns.h for more architectures Christophe Leroy
` (4 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
On x86, UACCESS is controlled by two instructions: STAC and CLAC.
STAC instruction enables UACCESS while CLAC disables UACCESS.
This is simple enough for objtool to locate UACCESS enable and
disable.
But on powerpc it is a bit more complex, the same instruction is
used for enabling and disabling UACCESS, and the same instruction
can be used for many other things. It would be too complex to use
exclusively instruction decoding.
To help objtool, mark such instruction into .discard.uaccess_begin
and .discard.uaccess_end sections, on the same principle as for
reachable/unreachable instructions. And add ASM_UACCESS_BEGIN
and ASM_UACCESS_END macros to be used in inline assembly code to
annotate UACCESS enable and UACCESS disable instructions.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
include/linux/objtool.h | 14 ++++++++++++++
tools/objtool/check.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 03f82c2c2ebf..d8fde4158a40 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -57,6 +57,18 @@
".long 998b - .\n\t" \
".popsection\n\t"
+#define ASM_UACCESS_BEGIN \
+ "998:\n\t" \
+ ".pushsection .discard.uaccess_begin\n\t" \
+ ".long 998b - .\n\t" \
+ ".popsection\n\t"
+
+#define ASM_UACCESS_END \
+ "998:\n\t" \
+ ".pushsection .discard.uaccess_end\n\t" \
+ ".long 998b - .\n\t" \
+ ".popsection\n\t"
+
#else /* __ASSEMBLY__ */
/*
@@ -156,6 +168,8 @@
#define STACK_FRAME_NON_STANDARD_FP(func)
#define ANNOTATE_NOENDBR
#define ASM_REACHABLE
+#define ASM_UACCESS_BEGIN
+#define ASM_UACCESS_END
#else
#define ANNOTATE_INTRA_FUNCTION_CALL
.macro UNWIND_HINT type:req sp_reg=0 sp_offset=0 signal=0
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d2a0dfec5909..5af6c6c3fbed 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1052,6 +1052,38 @@ static void add_ignores(struct objtool_file *file)
}
}
+static void __add_uaccess(struct objtool_file *file, const char *name,
+ int type, const char *action)
+{
+ struct section *rsec;
+ struct reloc *reloc;
+ struct instruction *insn;
+
+ rsec = find_section_by_name(file->elf, name);
+ if (!rsec)
+ return;
+
+ for_each_reloc(rsec, reloc) {
+ if (reloc->sym->type != STT_SECTION) {
+ WARN("unexpected relocation symbol type in %s: ", rsec->name);
+ continue;
+ }
+ insn = find_insn(file, reloc->sym->sec, reloc_addend(reloc));
+ if (!insn) {
+ WARN("can't find UACCESS %s insn at %s+0x%" PRIx64,
+ action, reloc->sym->sec->name, reloc_addend(reloc));
+ continue;
+ }
+ insn->type = type;
+ }
+}
+
+static void add_uaccess(struct objtool_file *file)
+{
+ __add_uaccess(file, ".rela.discard.uaccess_begin", INSN_STAC, "enable");
+ __add_uaccess(file, ".rela.discard.uaccess_end", INSN_CLAC, "disable");
+}
+
/*
* This is a whitelist of functions that is allowed to be called with AC set.
* The list is meant to be minimal and only contains compiler instrumentation
@@ -2597,6 +2629,7 @@ static int decode_sections(struct objtool_file *file)
return ret;
add_ignores(file);
+ add_uaccess(file);
add_uaccess_safe(file);
ret = add_ignore_alternatives(file);
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 13/15] objtool: Prepare noreturns.h for more architectures
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (11 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 12/15] objtool: Add support for more complex UACCESS control Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 14/15] powerpc/bug: Annotate reachable after warning trap Christophe Leroy
` (3 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
noreturns.h is a mix of x86 specific functions and more generic
core functions.
In preparation of inclusion of powerpc, split x86 functions out of
noreturns.h into arch/noreturns.h
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
.../objtool/arch/x86/include/arch/noreturns.h | 20 +++++++++++++++++++
tools/objtool/noreturns.h | 14 ++-----------
2 files changed, 22 insertions(+), 12 deletions(-)
create mode 100644 tools/objtool/arch/x86/include/arch/noreturns.h
diff --git a/tools/objtool/arch/x86/include/arch/noreturns.h b/tools/objtool/arch/x86/include/arch/noreturns.h
new file mode 100644
index 000000000000..a4262aff3917
--- /dev/null
+++ b/tools/objtool/arch/x86/include/arch/noreturns.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This is a (sorted!) list of all known __noreturn functions in arch/x86.
+ * It's needed for objtool to properly reverse-engineer the control flow graph.
+ *
+ * Yes, this is unfortunate. A better solution is in the works.
+ */
+NORETURN(cpu_bringup_and_idle)
+NORETURN(ex_handler_msr_mce)
+NORETURN(hlt_play_dead)
+NORETURN(hv_ghcb_terminate)
+NORETURN(machine_real_restart)
+NORETURN(rewind_stack_and_make_dead)
+NORETURN(sev_es_terminate)
+NORETURN(snp_abort)
+NORETURN(x86_64_start_kernel)
+NORETURN(x86_64_start_reservations)
+NORETURN(xen_cpu_bringup_again)
+NORETURN(xen_start_kernel)
diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
index e45c7cb1d5bc..b5e0f078dbb6 100644
--- a/tools/objtool/noreturns.h
+++ b/tools/objtool/noreturns.h
@@ -1,5 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include <arch/noreturns.h>
+
/*
* This is a (sorted!) list of all known __noreturn functions in the kernel.
* It's needed for objtool to properly reverse-engineer the control flow graph.
@@ -14,32 +16,20 @@ NORETURN(__stack_chk_fail)
NORETURN(__ubsan_handle_builtin_unreachable)
NORETURN(arch_call_rest_init)
NORETURN(arch_cpu_idle_dead)
-NORETURN(cpu_bringup_and_idle)
NORETURN(cpu_startup_entry)
NORETURN(do_exit)
NORETURN(do_group_exit)
NORETURN(do_task_dead)
-NORETURN(ex_handler_msr_mce)
NORETURN(fortify_panic)
-NORETURN(hlt_play_dead)
-NORETURN(hv_ghcb_terminate)
NORETURN(kthread_complete_and_exit)
NORETURN(kthread_exit)
NORETURN(kunit_try_catch_throw)
-NORETURN(machine_real_restart)
NORETURN(make_task_dead)
NORETURN(mpt_halt_firmware)
NORETURN(nmi_panic_self_stop)
NORETURN(panic)
NORETURN(panic_smp_self_stop)
NORETURN(rest_init)
-NORETURN(rewind_stack_and_make_dead)
-NORETURN(sev_es_terminate)
-NORETURN(snp_abort)
NORETURN(start_kernel)
NORETURN(stop_this_cpu)
NORETURN(usercopy_abort)
-NORETURN(x86_64_start_kernel)
-NORETURN(x86_64_start_reservations)
-NORETURN(xen_cpu_bringup_again)
-NORETURN(xen_start_kernel)
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 14/15] powerpc/bug: Annotate reachable after warning trap
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (12 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 13/15] objtool: Prepare noreturns.h for more architectures Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-11 16:08 ` [PATCH v4 15/15] powerpc: Implement UACCESS validation on PPC32 Christophe Leroy
` (2 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
This commit is copied from commit bfb1a7c91fb7 ("x86/bug: Merge
annotate_reachable() into _BUG_FLAGS() asm")
'twi 31,0,0' is a BUG instruction, which is by default a dead end.
But the same instruction is used for WARNINGs and the execution
resumes with the following instruction. Mark it reachable so
that objtool knows that it is not a dead end in that case.
Also change the unreachable() annotation by __builtin_unreachable()
since objtool already knows that a BUG instruction is a dead end.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/bug.h | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index abb608dff15a..1c204ee4cc03 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -4,6 +4,7 @@
#ifdef __KERNEL__
#include <asm/asm-compat.h>
+#include <linux/objtool.h>
#ifdef CONFIG_BUG
@@ -51,10 +52,11 @@
".previous\n"
#endif
-#define BUG_ENTRY(insn, flags, ...) \
+#define BUG_ENTRY(insn, flags, extra, ...) \
__asm__ __volatile__( \
"1: " insn "\n" \
_EMIT_BUG_ENTRY \
+ extra \
: : "i" (__FILE__), "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
@@ -67,12 +69,12 @@
*/
#define BUG() do { \
- BUG_ENTRY("twi 31, 0, 0", 0); \
- unreachable(); \
+ BUG_ENTRY("twi 31, 0, 0", 0, ""); \
+ __builtin_unreachable(); \
} while (0)
#define HAVE_ARCH_BUG
-#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags))
+#define __WARN_FLAGS(flags) BUG_ENTRY("twi 31, 0, 0", BUGFLAG_WARNING | (flags), ASM_REACHABLE)
#ifdef CONFIG_PPC64
#define BUG_ON(x) do { \
@@ -80,7 +82,7 @@
if (x) \
BUG(); \
} else { \
- BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "r" ((__force long)(x))); \
+ BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "", "r" ((__force long)(x))); \
} \
} while (0)
@@ -92,7 +94,7 @@
} else { \
BUG_ENTRY(PPC_TLNEI " %4, 0", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
- "r" (__ret_warn_on)); \
+ "", "r" (__ret_warn_on)); \
} \
unlikely(__ret_warn_on); \
})
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v4 15/15] powerpc: Implement UACCESS validation on PPC32
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (13 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 14/15] powerpc/bug: Annotate reachable after warning trap Christophe Leroy
@ 2023-07-11 16:08 ` Christophe Leroy
2023-07-12 14:23 ` [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Peter Zijlstra
2023-07-20 13:50 ` (subset) " Michael Ellerman
16 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-11 16:08 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao
Cc: Christophe Leroy, linux-kernel, linuxppc-dev
In order to implement UACCESS validation, objtool support
for powerpc needs to be enhanced to decode more instructions.
It also requires implementation of switch tables finding.
On PPC32 it is similar to x86, switch tables are anonymous in .rodata,
the difference is that the value is relative to its index in the table.
But several switch tables can be nested so the register containing
the table base address also needs to be tracked and taken into account.
Don't activate if for Clang for now because its switch tables are
different from GCC switch tables.
Then comes the UACCESS enabling/disabling instructions. On booke and
8xx it is done with a mtspr instruction. For 8xx that's in SPRN_MD_AP,
for booke that's in SPRN_PID. Annotate those instructions.
No work has been done for ASM files, they are not used for UACCESS
so for the moment just tell objtool to ignore ASM files.
For relocable code, the .got2 relocation preceding each global
function needs to be marked as ignored because some versions of GCC
do this:
120: 00 00 00 00 .long 0x0
120: R_PPC_REL32 .got2+0x7ff0
00000124 <tohex>:
124: 94 21 ff f0 stwu r1,-16(r1)
128: 7c 08 02 a6 mflr r0
12c: 42 9f 00 05 bcl 20,4*cr7+so,130 <tohex+0xc>
130: 39 00 00 00 li r8,0
134: 39 20 00 08 li r9,8
138: 93 c1 00 08 stw r30,8(r1)
13c: 7f c8 02 a6 mflr r30
140: 90 01 00 14 stw r0,20(r1)
144: 80 1e ff f0 lwz r0,-16(r30)
148: 7f c0 f2 14 add r30,r0,r30
14c: 81 5e 80 00 lwz r10,-32768(r30)
150: 80 fe 80 04 lwz r7,-32764(r30)
Also declare longjmp() and start_secondary_resume() as global noreturn
functions, and declare __copy_tofrom_user() and __arch_clear_user()
as UACCESS safe.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/Kconfig | 2 +
arch/powerpc/include/asm/book3s/32/kup.h | 2 +
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 4 +-
arch/powerpc/include/asm/nohash/kup-booke.h | 4 +-
arch/powerpc/kexec/core_32.c | 4 +-
arch/powerpc/mm/nohash/kup.c | 2 +
tools/objtool/arch/powerpc/decode.c | 155 +++++++++++++++++-
.../arch/powerpc/include/arch/noreturns.h | 11 ++
tools/objtool/arch/powerpc/special.c | 36 +++-
tools/objtool/check.c | 6 +-
10 files changed, 211 insertions(+), 15 deletions(-)
create mode 100644 tools/objtool/arch/powerpc/include/arch/noreturns.h
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 0b1172cbeccb..cdaca38868e1 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -159,6 +159,7 @@ config PPC
select ARCH_KEEP_MEMBLOCK
select ARCH_MIGHT_HAVE_PC_PARPORT
select ARCH_MIGHT_HAVE_PC_SERIO
+ select ARCH_OBJTOOL_SKIP_ASM
select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT
select ARCH_SPLIT_ARG64 if PPC32
@@ -257,6 +258,7 @@ config PPC
select HAVE_OPTPROBES
select HAVE_OBJTOOL if PPC32 || MPROFILE_KERNEL
select HAVE_OBJTOOL_MCOUNT if HAVE_OBJTOOL
+ select HAVE_UACCESS_VALIDATION if HAVE_OBJTOOL && PPC_KUAP && PPC32 && CC_IS_GCC
select HAVE_PERF_EVENTS
select HAVE_PERF_EVENTS_NMI if PPC64
select HAVE_PERF_REGS
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 4e14a5427a63..842d9a6f4b7a 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -34,6 +34,7 @@ static __always_inline void uaccess_begin_32s(unsigned long addr)
asm volatile(ASM_MMU_FTR_IFSET(
"mfsrin %0, %1;"
"rlwinm %0, %0, 0, %2;"
+ ASM_UACCESS_BEGIN
"mtsrin %0, %1;"
"isync", "", %3)
: "=&r"(tmp)
@@ -48,6 +49,7 @@ static __always_inline void uaccess_end_32s(unsigned long addr)
asm volatile(ASM_MMU_FTR_IFSET(
"mfsrin %0, %1;"
"oris %0, %0, %2;"
+ ASM_UACCESS_END
"mtsrin %0, %1;"
"isync", "", %3)
: "=&r"(tmp)
diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
index 46bc5925e5fd..38c7ed766445 100644
--- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
@@ -39,13 +39,13 @@ static __always_inline unsigned long __kuap_get_and_assert_locked(void)
static __always_inline void uaccess_begin_8xx(unsigned long val)
{
- asm(ASM_MMU_FTR_IFSET("mtspr %0, %1", "", %2) : :
+ asm(ASM_UACCESS_BEGIN ASM_MMU_FTR_IFSET("mtspr %0, %1", "", %2) : :
"i"(SPRN_MD_AP), "r"(val), "i"(MMU_FTR_KUAP) : "memory");
}
static __always_inline void uaccess_end_8xx(void)
{
- asm(ASM_MMU_FTR_IFSET("mtspr %0, %1", "", %2) : :
+ asm(ASM_UACCESS_END ASM_MMU_FTR_IFSET("mtspr %0, %1", "", %2) : :
"i"(SPRN_MD_AP), "r"(MD_APG_KUAP), "i"(MMU_FTR_KUAP) : "memory");
}
diff --git a/arch/powerpc/include/asm/nohash/kup-booke.h b/arch/powerpc/include/asm/nohash/kup-booke.h
index 0c7c3258134c..26eb6d10a964 100644
--- a/arch/powerpc/include/asm/nohash/kup-booke.h
+++ b/arch/powerpc/include/asm/nohash/kup-booke.h
@@ -63,13 +63,13 @@ static __always_inline unsigned long __kuap_get_and_assert_locked(void)
static __always_inline void uaccess_begin_booke(unsigned long val)
{
- asm(ASM_MMU_FTR_IFSET("mtspr %0, %1; isync", "", %2) : :
+ asm(ASM_UACCESS_BEGIN ASM_MMU_FTR_IFSET("mtspr %0, %1; isync", "", %2) : :
"i"(SPRN_PID), "r"(val), "i"(MMU_FTR_KUAP) : "memory");
}
static __always_inline void uaccess_end_booke(void)
{
- asm(ASM_MMU_FTR_IFSET("mtspr %0, %1; isync", "", %2) : :
+ asm(ASM_UACCESS_END ASM_MMU_FTR_IFSET("mtspr %0, %1; isync", "", %2) : :
"i"(SPRN_PID), "r"(0), "i"(MMU_FTR_KUAP) : "memory");
}
diff --git a/arch/powerpc/kexec/core_32.c b/arch/powerpc/kexec/core_32.c
index c95f96850c9e..6e955f32e7c3 100644
--- a/arch/powerpc/kexec/core_32.c
+++ b/arch/powerpc/kexec/core_32.c
@@ -17,7 +17,7 @@
typedef void (*relocate_new_kernel_t)(
unsigned long indirection_page,
unsigned long reboot_code_buffer,
- unsigned long start_address) __noreturn;
+ unsigned long start_address);
/*
* This is a generic machine_kexec function suitable at least for
@@ -61,6 +61,8 @@ void default_machine_kexec(struct kimage *image)
/* now call it */
rnk = (relocate_new_kernel_t) reboot_code_buffer;
(*rnk)(page_list, reboot_code_buffer_phys, image->start);
+
+ unreachable(); /* For objtool */
}
int machine_kexec_prepare(struct kimage *image)
diff --git a/arch/powerpc/mm/nohash/kup.c b/arch/powerpc/mm/nohash/kup.c
index e1f7de2e54ec..e23c7ca269fe 100644
--- a/arch/powerpc/mm/nohash/kup.c
+++ b/arch/powerpc/mm/nohash/kup.c
@@ -24,6 +24,8 @@ void setup_kuap(bool disabled)
pr_info("Activating Kernel Userspace Access Protection\n");
+ /* Performed a paired allow/prevent to silence objtool warning */
+ allow_user_access(NULL, NULL, 0, KUAP_READ_WRITE);
prevent_user_access(KUAP_READ_WRITE);
}
#endif
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 53b55690f320..62774d2cca33 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -39,28 +39,164 @@ const char *arch_ret_insn(int len)
exit(-1);
}
+static u32 read_instruction(struct objtool_file *file, const struct section *sec,
+ unsigned long offset)
+{
+ return bswap_if_needed(file->elf, *(u32 *)(sec->data->d_buf + offset));
+}
+
+/*
+ * Try to find the register used as base for a table jump.
+ * If not found return r1 which is the stack so can't be valid
+ *
+ * For relative jump tables we expect the following sequence
+ * lwzx rx, reg1, reg2 or lwz rx, 0(reg)
+ * add ry, rx, rbase or add ry, rbase, rx
+ * mtctr ry
+ * bctr
+ *
+ * For absolute jump tables we expect the following sequence
+ * lwzx rx, rbase, rindex
+ * mtctr rx
+ * bctr
+ *
+ * Those sequences might be nested with other code, but we expect
+ * it within the last 16 instructions.
+ */
+static unsigned int arch_decode_jumptable_base(struct objtool_file *file,
+ const struct section *sec,
+ struct instruction *jump_insn)
+{
+ int i;
+ unsigned int td = ~0, ta = ~0, tb = ~0;
+ struct instruction *insn;
+
+ for (insn = jump_insn, i = 0;
+ insn && i < 16;
+ insn = prev_insn_same_sec(file, insn), i++) {
+ u32 ins = read_instruction(file, sec, insn->offset);
+ unsigned int ra = (ins >> 16) & 0x1f;
+ unsigned int rb = (ins >> 11) & 0x1f;
+ unsigned int rd = (ins >> 21) & 0x1f;
+
+ if (td == ~0 && ta == ~0) {
+ if ((ins & 0xfc1ffffe) == 0x7c0903a6) /* mtctr rd */
+ td = rd;
+ continue;
+ }
+ /* lwzx td, ra, rb */
+ if (td != ~0 && (ins & 0xfc0007fe) == 0x7c00002e && rd == td)
+ return ra;
+
+ /* lwzx ta, ra, rb or lwzx tb, ra, rb */
+ if (ta != ~0 && (ins & 0xfc0007fe) == 0x7c00002e && (rd == ta || rd == tb))
+ return rd == ta ? tb : ta;
+
+ /* lwz ta, 0(ra) or lwz tb, 0(ra) */
+ if (ta != ~0 && (ins & 0xfc00ffff) == 0x80000000 && (rd == ta || rd == tb))
+ return rd == ta ? tb : ta;
+
+ /* add td, ta, tb */
+ if (ta == ~0 && (ins & 0xfc0007ff) == 0x7c000214 && rd == td) {
+ ta = ra;
+ tb = rb;
+ td = ~0;
+ }
+ }
+ return 1;
+}
+
int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
unsigned long offset, unsigned int maxlen,
struct instruction *insn)
{
- unsigned int opcode;
+ unsigned int opcode, xop;
+ unsigned int rs, ra, rb, bo, bi, to, uimm, simm, lk, aa;
enum insn_type typ;
unsigned long imm;
- u32 ins;
+ u32 ins = read_instruction(file, sec, offset);
+
+ if (!ins && file->elf->ehdr.e_flags & EF_PPC_RELOCATABLE_LIB) {
+ struct reloc *reloc;
+
+ reloc = find_reloc_by_dest_range(file->elf, insn->sec, insn->offset, 4);
+
+ if (reloc && reloc_type(reloc) == R_PPC_REL32 &&
+ !strncmp(reloc->sym->sec->name, ".got2", 5)) {
+ insn->type = INSN_OTHER;
+ insn->ignore = true;
+ insn->len = 4;
+
+ return 0;
+ }
+ }
- ins = bswap_if_needed(file->elf, *(u32 *)(sec->data->d_buf + offset));
opcode = ins >> 26;
- typ = INSN_OTHER;
- imm = 0;
+ xop = (ins >> 1) & 0x3ff;
+ rs = bo = to = (ins >> 21) & 0x1f;
+ ra = bi = (ins >> 16) & 0x1f;
+ rb = (ins >> 11) & 0x1f;
+ uimm = simm = (ins >> 0) & 0xffff;
+ aa = ins & 2;
+ lk = ins & 1;
switch (opcode) {
+ case 3:
+ if (to == 31 && ra == 0 && simm == 0) /* twi 31, r0, 0 */
+ typ = INSN_BUG;
+ else
+ typ = INSN_OTHER;
+ break;
+ case 16: /* bc[l][a] */
+ if (lk) /* bcl[a] */
+ typ = INSN_OTHER;
+ else /* bc[a] */
+ typ = INSN_JUMP_CONDITIONAL;
+
+ imm = ins & 0xfffc;
+ if (imm & 0x8000)
+ imm -= 0x10000;
+ insn->immediate = imm | aa;
+ break;
case 18: /* b[l][a] */
- if ((ins & 3) == 1) /* bl */
+ if (lk) /* bl[a] */
typ = INSN_CALL;
+ else /* b[a] */
+ typ = INSN_JUMP_UNCONDITIONAL;
imm = ins & 0x3fffffc;
if (imm & 0x2000000)
imm -= 0x4000000;
+ insn->immediate = imm | aa;
+ break;
+ case 19:
+ if (xop == 16 && bo == 20 && bi == 0) /* blr */
+ typ = INSN_RETURN;
+ else if (xop == 16) /* bclr */
+ typ = INSN_RETURN_CONDITIONAL;
+ else if (xop == 50) /* rfi */
+ typ = INSN_JUMP_DYNAMIC;
+ else if (xop == 528 && bo == 20 && bi == 0 && !lk) /* bctr */
+ typ = INSN_JUMP_DYNAMIC;
+ else if (xop == 528 && bo == 20 && bi == 0 && lk) /* bctrl */
+ typ = INSN_CALL_DYNAMIC;
+ else
+ typ = INSN_OTHER;
+ break;
+ case 24:
+ if (rs == 0 && ra == 0 && uimm == 0)
+ typ = INSN_NOP;
+ else
+ typ = INSN_OTHER;
+ break;
+ case 31:
+ if (xop == 4 && to == 31 && ra == 0 && rb == 0) /* trap */
+ typ = INSN_BUG;
+ else
+ typ = INSN_OTHER;
+ break;
+ default:
+ typ = INSN_OTHER;
break;
}
@@ -70,13 +206,18 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
insn->len = 4;
insn->type = typ;
- insn->immediate = imm;
+
+ if (typ == INSN_JUMP_DYNAMIC)
+ insn->gpr = arch_decode_jumptable_base(file, sec, insn);
return 0;
}
unsigned long arch_jump_destination(struct instruction *insn)
{
+ if (insn->immediate & 2)
+ return insn->immediate & ~2;
+
return insn->offset + insn->immediate;
}
diff --git a/tools/objtool/arch/powerpc/include/arch/noreturns.h b/tools/objtool/arch/powerpc/include/arch/noreturns.h
new file mode 100644
index 000000000000..664f17d39026
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/noreturns.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This is a (sorted!) list of all known __noreturn functions in arch/powerpc.
+ * It's needed for objtool to properly reverse-engineer the control flow graph.
+ *
+ * Yes, this is unfortunate. A better solution is in the works.
+ */
+NORETURN(longjmp)
+NORETURN(start_secondary_resume)
+NORETURN(unrecoverable_exception)
diff --git a/tools/objtool/arch/powerpc/special.c b/tools/objtool/arch/powerpc/special.c
index a7dd2559b536..c241a6dca8b0 100644
--- a/tools/objtool/arch/powerpc/special.c
+++ b/tools/objtool/arch/powerpc/special.c
@@ -3,7 +3,7 @@
#include <stdlib.h>
#include <objtool/special.h>
#include <objtool/builtin.h>
-
+#include <objtool/endianness.h>
bool arch_support_alt_relocation(struct special_alt *special_alt,
struct instruction *insn,
@@ -16,5 +16,37 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
struct instruction *insn,
struct instruction *orig_insn)
{
- exit(-1);
+ struct reloc *text_reloc;
+ struct section *table_sec;
+ unsigned long table_offset;
+ u32 ins;
+
+ /* 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 || reloc_type(text_reloc) != R_PPC_ADDR16_LO ||
+ text_reloc->sym->type != STT_SECTION || !text_reloc->sym->sec->rodata)
+ return NULL;
+
+ ins = bswap_if_needed(file->elf, *(u32 *)(insn->sec->data->d_buf + insn->offset));
+ if (orig_insn && ((ins >> 21) & 0x1f) != orig_insn->gpr)
+ return NULL;
+
+ table_offset = reloc_addend(text_reloc);
+ table_sec = text_reloc->sym->sec;
+
+ /*
+ * 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;
+
+ return find_reloc_by_dest(file->elf, table_sec, table_offset);
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5af6c6c3fbed..0d2df15f553a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1259,13 +1259,17 @@ static const char *uaccess_safe_builtin[] = {
"stackleak_track_stack",
/* misc */
"csum_partial_copy_generic",
+ "ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
+ /* misc x86 */
"copy_mc_fragile",
"copy_mc_fragile_handle_tail",
"copy_mc_enhanced_fast_string",
- "ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
"rep_stos_alternative",
"rep_movs_alternative",
"__copy_user_nocache",
+ /* misc powerpc */
+ "__copy_tofrom_user",
+ "__arch_clear_user",
NULL
};
--
2.41.0
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4)
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (14 preceding siblings ...)
2023-07-11 16:08 ` [PATCH v4 15/15] powerpc: Implement UACCESS validation on PPC32 Christophe Leroy
@ 2023-07-12 14:23 ` Peter Zijlstra
2023-07-12 16:29 ` Christophe Leroy
2023-07-20 13:50 ` (subset) " Michael Ellerman
16 siblings, 1 reply; 20+ messages in thread
From: Peter Zijlstra @ 2023-07-12 14:23 UTC (permalink / raw)
To: Christophe Leroy
Cc: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Sathvika Vasireddy, Naveen N Rao, linux-kernel, linuxppc-dev
On Tue, Jul 11, 2023 at 06:08:26PM +0200, Christophe Leroy wrote:
> This series adds UACCESS validation for PPC32. It includes
> a dozen of changes to objtool core.
>
> It applies on top of series "Cleanup/Optimise KUAP (v3)"
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*
That contains:
+static __always_inline void uaccess_begin_32s(unsigned long addr)
+{
+ unsigned long tmp;
+
+ asm volatile(ASM_MMU_FTR_IFSET(
+ "mfsrin %0, %1;"
+ "rlwinm %0, %0, 0, %2;"
+ "mtsrin %0, %1;"
+ "isync", "", %3)
+ : "=&r"(tmp)
+ : "r"(addr), "i"(~SR_KS), "i"(MMU_FTR_KUAP)
+ : "memory");
+}
+
+static __always_inline void uaccess_end_32s(unsigned long addr)
+{
+ unsigned long tmp;
+
+ asm volatile(ASM_MMU_FTR_IFSET(
+ "mfsrin %0, %1;"
+ "oris %0, %0, %2;"
+ "mtsrin %0, %1;"
+ "isync", "", %3)
+ : "=&r"(tmp)
+ : "r"(addr), "i"(SR_KS >> 16), "i"(MMU_FTR_KUAP)
+ : "memory");
+}
And I am a bit puzzled by the isync placement of uaccess_end, should
that not start with the isync, to ensure completion of the uaccess
region before disabling it?
Or is that not the purpose of the isync?
> It is almost mature, performs code analysis for all PPC32.
>
> In this version objtool switch table lookup has been enhanced to
> handle nested switch tables.
>
> Most object files are correctly decoded, only a few
> 'unreachable instruction' warnings remain due to more complex
> fonctions which include back and forth jumps or branches.
>
> It allowed to detect some UACCESS mess in a few files. They've been
> fixed through other patches.
>
> Changes in v4:
> - Split series in two parts, the powerpc uaccess rework is submitted
> separately, see https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*
> - Support of UACCESS on all PPC32 including book3s/32 which was missing in v3.
> - More elaborated switch tables lookup.
> - Patches 2, 7, 8, 9, 10, 11 are new
> - Patch 11 in series v3 is now removed.
The patches look eminently reasonable to me; Josh, could you please have
a look?
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4)
2023-07-12 14:23 ` [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Peter Zijlstra
@ 2023-07-12 16:29 ` Christophe Leroy
0 siblings, 0 replies; 20+ messages in thread
From: Christophe Leroy @ 2023-07-12 16:29 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Sathvika Vasireddy, Naveen N Rao, linux-kernel, linuxppc-dev
Le 12/07/2023 à 16:23, Peter Zijlstra a écrit :
> On Tue, Jul 11, 2023 at 06:08:26PM +0200, Christophe Leroy wrote:
>> This series adds UACCESS validation for PPC32. It includes
>> a dozen of changes to objtool core.
>>
>> It applies on top of series "Cleanup/Optimise KUAP (v3)"
>> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*
>
> That contains:
>
> +static __always_inline void uaccess_begin_32s(unsigned long addr)
> +{
> + unsigned long tmp;
> +
> + asm volatile(ASM_MMU_FTR_IFSET(
> + "mfsrin %0, %1;"
> + "rlwinm %0, %0, 0, %2;"
> + "mtsrin %0, %1;"
> + "isync", "", %3)
> + : "=&r"(tmp)
> + : "r"(addr), "i"(~SR_KS), "i"(MMU_FTR_KUAP)
> + : "memory");
> +}
> +
> +static __always_inline void uaccess_end_32s(unsigned long addr)
> +{
> + unsigned long tmp;
> +
> + asm volatile(ASM_MMU_FTR_IFSET(
> + "mfsrin %0, %1;"
> + "oris %0, %0, %2;"
> + "mtsrin %0, %1;"
> + "isync", "", %3)
> + : "=&r"(tmp)
> + : "r"(addr), "i"(SR_KS >> 16), "i"(MMU_FTR_KUAP)
> + : "memory");
> +}
>
> And I am a bit puzzled by the isync placement of uaccess_end, should
> that not start with the isync, to ensure completion of the uaccess
> region before disabling it?
>
> Or is that not the purpose of the isync?
Good question.
The C function is:
static __always_inline void kuap_lock_one(unsigned long addr)
{
mtsr(mfsr(addr) | SR_KS, addr);
isync(); /* Context sync required after mtsr() */
}
static __always_inline void kuap_unlock_one(unsigned long addr)
{
mtsr(mfsr(addr) & ~SR_KS, addr);
isync(); /* Context sync required after mtsr() */
}
So uaccess_begin_32s() and uaccess_end_32s() are built from that.
Looking at the history I have never put an isync up front even in the
very first implementation back in 2019 in commit a68c31fc01ef
("powerpc/32s: Implement Kernel Userspace Access Protection")
Well just before that commit we have commit 31ed2b13c48d ("powerpc/32s:
Implement Kernel Userspace Execution Prevention.") which for sure
doesn't require the isync according to the "Programming Environments
Manual for 32-Bit Implementations of the PowerPC™ Architecture".
But for data access the same manual says, in the previous table, that
isync is required both before and after mtsr. So, did I miss something
at that time ? I can't remember but for sure we have been in this
situation from the begining, and nobody has reported any problem with
that yet. So not sure what to do here, as it may have an impact on
performance. Will handle it in a followup patch if deamed necessary.
>
>> It is almost mature, performs code analysis for all PPC32.
>>
>> In this version objtool switch table lookup has been enhanced to
>> handle nested switch tables.
>>
>> Most object files are correctly decoded, only a few
>> 'unreachable instruction' warnings remain due to more complex
>> fonctions which include back and forth jumps or branches.
>>
>> It allowed to detect some UACCESS mess in a few files. They've been
>> fixed through other patches.
>>
>> Changes in v4:
>> - Split series in two parts, the powerpc uaccess rework is submitted
>> separately, see https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*
>> - Support of UACCESS on all PPC32 including book3s/32 which was missing in v3.
>> - More elaborated switch tables lookup.
>> - Patches 2, 7, 8, 9, 10, 11 are new
>> - Patch 11 in series v3 is now removed.
>
> The patches look eminently reasonable to me; Josh, could you please have
> a look?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (subset) [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4)
2023-07-11 16:08 [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Christophe Leroy
` (15 preceding siblings ...)
2023-07-12 14:23 ` [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4) Peter Zijlstra
@ 2023-07-20 13:50 ` Michael Ellerman
2023-07-21 5:00 ` Michael Ellerman
16 siblings, 1 reply; 20+ messages in thread
From: Michael Ellerman @ 2023-07-20 13:50 UTC (permalink / raw)
To: Nicholas Piggin, Josh Poimboeuf, Peter Zijlstra,
Sathvika Vasireddy, Naveen N Rao, Christophe Leroy
Cc: linux-kernel, linuxppc-dev
On Tue, 11 Jul 2023 18:08:26 +0200, Christophe Leroy wrote:
> This series adds UACCESS validation for PPC32. It includes
> a dozen of changes to objtool core.
>
> It applies on top of series "Cleanup/Optimise KUAP (v3)"
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*
>
> It is almost mature, performs code analysis for all PPC32.
>
> [...]
Applied to powerpc/fixes.
[01/15] Revert "powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto"
https://git.kernel.org/powerpc/c/b49e578b9314db051da0ad72bba24094193f9bd0
cheers
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: (subset) [PATCH v4 00/15] powerpc/objtool: uaccess validation for PPC32 (v4)
2023-07-20 13:50 ` (subset) " Michael Ellerman
@ 2023-07-21 5:00 ` Michael Ellerman
0 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2023-07-21 5:00 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Josh Poimboeuf,
Peter Zijlstra, Sathvika Vasireddy, Naveen N Rao,
Christophe Leroy
Cc: linux-kernel, linuxppc-dev
Michael Ellerman <patch-notifications@ellerman.id.au> writes:
> On Tue, 11 Jul 2023 18:08:26 +0200, Christophe Leroy wrote:
>> This series adds UACCESS validation for PPC32. It includes
>> a dozen of changes to objtool core.
>>
>> It applies on top of series "Cleanup/Optimise KUAP (v3)"
>> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=363368&state=*
>>
>> It is almost mature, performs code analysis for all PPC32.
>>
>> [...]
>
> Applied to powerpc/fixes.
>
> [01/15] Revert "powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto"
> https://git.kernel.org/powerpc/c/b49e578b9314db051da0ad72bba24094193f9bd0
Sorry that's b4 getting confused, I actually applied the v5 that I sent:
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20230712134552.534955-1-mpe@ellerman.id.au/
cheers
^ permalink raw reply [flat|nested] 20+ messages in thread