* [PATCH v3 0/8] Improve objtool jump table handling
@ 2024-10-11 17:08 Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 1/8] objtool: Deal with relative jump tables correctly Ard Biesheuvel
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
Jump table handling has faded into the background a little due to the
fact that jump tables are [currently] disabled when enabling retpoline
mitigations and/or IBT on x86.
However, this is likely to come back and bite us later, so it still
needs to be addressed. Given the difficulty in identifying jump tables
from .rodata references and indirect jump instructions that often have
no obvious correlation, it would be better to do this in the compiler.
This series implements [on the objtool side] the suggestion made at GNU
Cauldron this year to annotate the indirect jump with a R_X86_64_NONE
relocation that refers to the jump table, and ensure that it is covered
by a STT_OBJECT symbol whose size accurately reflects the size of the
jump table.
This can be wired up in objtool with minimal effort. The only
complication is that indirect jumps may be direct jumps in disguise, if
they target retpoline thunks. This will result in more than one
relocation attached to the same instruction, which needs careful
handling in objtool.
Other than that, changes are rather straight-forward.
Patches #6 - #8 update the CRC32C driver, which has a jump table
implemented in assembler, to
a) use a relative jump table, for compatibility with linking in PIE mode
b) add the jump table annotation
c) make the jump table more difficult to identify by objtool's existing
heuristics, so that it will fail to identify it without the
annotation.
Changes since v2:
- drastic refactoring of the annotation handling so that generic users
(non-x86) get it as well, with the x86 heuristics moved to a x86
specific source file
- use generic reloc type identifiers where appropriate
- update insn->no_reloc where appropriate
Changes since v1:
- tweak logic in patch #1 to ensure that all jump table entries are
covered by the same type of relocation
- use the corrected addend when validating IBT targets
- add patches #2 - #5
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: "Jose E. Marchesi" <jemarch@gnu.org>
Cc: Kees Cook <kees@kernel.org>
Ard Biesheuvel (8):
objtool: Deal with relative jump tables correctly
objtool: Allow arch code to discover jump table size
objtool: Make some helper functions globally accessible
objtool: Move jump table heuristics to a x86 specific source file
objtool: Add generic support for jump table annotations
crypto: x86/crc32c - Use idiomatic relative jump table
crypto: x86/crc32c - Add jump table annotation
crypto: x86/crc32c-intel - Tweaks to make objtool's life harder
arch/x86/crypto/crc32c-pcl-intel-asm_64.S | 50 +++--
tools/objtool/arch/loongarch/special.c | 6 -
tools/objtool/arch/powerpc/special.c | 6 -
tools/objtool/arch/x86/special.c | 160 ++++++++++++----
tools/objtool/check.c | 199 ++++++++++----------
tools/objtool/include/objtool/check.h | 25 ++-
tools/objtool/include/objtool/elf.h | 6 +
tools/objtool/include/objtool/special.h | 8 +-
8 files changed, 287 insertions(+), 173 deletions(-)
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/8] objtool: Deal with relative jump tables correctly
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 2/8] objtool: Allow arch code to discover jump table size Ard Biesheuvel
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
Relative jump tables contain entries that carry the offset between the
target of the jump and the start of the jump table. This permits the use
of the PIC idiom of
leaq jump_table(%rip), %tbl
movslq (%tbl,%idx,4), %offset
addq %offset, %tbl
jmp *%tbl
The jump table entries are decorated with PC32 relocations, which record
the offset of the referenced symbol relative to the target of the
relocation, which is the individual entry in the table. This means that
only the first entry produces the correct value directly; the subsequent
ones need to be corrected to produce the offset relative to the start of
the table, by applying an addend.
Given that the referenced symbols are anonymous, and thus already
expressed in terms of sections and addends, e.g., .text+0x5df9, the
correction is incorporated into the existing addend. The upshot of this
is that chasing the reference to find the target instruction needs to
take this second addend into account as well.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
tools/objtool/arch/x86/special.c | 8 -------
tools/objtool/check.c | 24 +++++++++++++++++---
tools/objtool/include/objtool/elf.h | 6 +++++
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index 4ea0f9815fda..415e4d035e53 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -150,13 +150,5 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
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 (reloc_type(text_reloc) == R_X86_64_PC32)
- file->ignore_unreachables = true;
-
return rodata_reloc;
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2b0965153b25..aa07fdf1cf13 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2101,6 +2101,8 @@ 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);
+ unsigned int rtype = reloc_type(table);
+ bool pcrel = (rtype == elf_text_rela_type(file->elf));
struct instruction *dest_insn;
unsigned int prev_offset = 0;
struct reloc *reloc = table;
@@ -2111,13 +2113,18 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
* instruction.
*/
for_each_reloc_from(table->sec, reloc) {
+ unsigned long addend = reloc_addend(reloc);
/* Check for the end of the table: */
if (reloc != table && reloc == next_table)
break;
+ /* Each entry in the jump table should use the same relocation type */
+ if (reloc_type(reloc) != rtype)
+ 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 + (pcrel ? 4 : 8))
break;
/* Detect function pointers from contiguous objects: */
@@ -2125,7 +2132,15 @@ 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));
+ /*
+ * Place-relative jump tables carry offsets relative to the
+ * start of the jump table, not to the entry itself. So correct
+ * the addend for the location of the entry in the table.
+ */
+ if (pcrel)
+ addend -= reloc_offset(reloc) - reloc_offset(table);
+
+ dest_insn = find_insn(file, reloc->sym->sec, addend);
if (!dest_insn)
break;
@@ -2133,6 +2148,9 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
break;
+ if (pcrel)
+ reloc->sym_offset = addend;
+
alt = malloc(sizeof(*alt));
if (!alt) {
WARN("malloc failed");
@@ -4535,7 +4553,7 @@ static int validate_ibt_data_reloc(struct objtool_file *file,
struct instruction *dest;
dest = find_insn(file, reloc->sym->sec,
- reloc->sym->offset + reloc_addend(reloc));
+ reloc->sym->offset + reloc_sym_offset(reloc));
if (!dest)
return 0;
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index d7e815c2fd15..f4a6307f4c08 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -78,6 +78,7 @@ struct reloc {
struct section *sec;
struct symbol *sym;
struct reloc *sym_next_reloc;
+ s64 sym_offset;
};
struct elf {
@@ -251,6 +252,11 @@ static inline s64 reloc_addend(struct reloc *reloc)
return __get_reloc_field(reloc, r_addend);
}
+static inline s64 reloc_sym_offset(struct reloc *reloc)
+{
+ return reloc->sym_offset ?: reloc_addend(reloc);
+}
+
static inline void set_reloc_addend(struct elf *elf, struct reloc *reloc, s64 addend)
{
__set_reloc_field(reloc, r_addend, addend);
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/8] objtool: Allow arch code to discover jump table size
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 1/8] objtool: Deal with relative jump tables correctly Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 3/8] objtool: Make some helper functions globally accessible Ard Biesheuvel
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
In preparation for adding support for annotated jump tables, where
ELF relocations and symbols are used to describe the locations of jump
tables in the executable, refactor the jump table discovery logic so the
table size can be returned from arch_find_switch_table().
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
tools/objtool/arch/loongarch/special.c | 3 +-
tools/objtool/arch/powerpc/special.c | 3 +-
tools/objtool/arch/x86/special.c | 4 ++-
tools/objtool/check.c | 31 +++++++++++++-------
tools/objtool/include/objtool/check.h | 5 +++-
tools/objtool/include/objtool/special.h | 3 +-
6 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 9bba1e9318e0..87230ed570fd 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -9,7 +9,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,
+ unsigned long *table_size)
{
return NULL;
}
diff --git a/tools/objtool/arch/powerpc/special.c b/tools/objtool/arch/powerpc/special.c
index d33868147196..51610689abf7 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,
+ unsigned long *table_size)
{
exit(-1);
}
diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index 415e4d035e53..f8fb67636384 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -109,7 +109,8 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
* NOTE: MITIGATION_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,
+ unsigned long *table_size)
{
struct reloc *text_reloc, *rodata_reloc;
struct section *table_sec;
@@ -150,5 +151,6 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
if (!rodata_reloc)
return NULL;
+ *table_size = 0;
return rodata_reloc;
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index aa07fdf1cf13..b73e43b9b9e3 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -150,6 +150,15 @@ static inline struct reloc *insn_jump_table(struct instruction *insn)
return NULL;
}
+static inline unsigned long insn_jump_table_size(struct instruction *insn)
+{
+ if (insn->type == INSN_JUMP_DYNAMIC ||
+ insn->type == INSN_CALL_DYNAMIC)
+ return insn->_jump_table_size;
+
+ return 0;
+}
+
static bool is_jump_table_jump(struct instruction *insn)
{
struct alt_group *alt_group = insn->alt_group;
@@ -2099,6 +2108,7 @@ static int add_special_section_alts(struct objtool_file *file)
static int add_jump_table(struct objtool_file *file, struct instruction *insn,
struct reloc *next_table)
{
+ unsigned long table_size = insn_jump_table_size(insn);
struct symbol *pfunc = insn_func(insn)->pfunc;
struct reloc *table = insn_jump_table(insn);
unsigned int rtype = reloc_type(table);
@@ -2116,6 +2126,8 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
unsigned long addend = reloc_addend(reloc);
/* Check for the end of the table: */
+ if (table_size && reloc_offset(reloc) - reloc_offset(table) >= table_size)
+ break;
if (reloc != table && reloc == next_table)
break;
@@ -2175,12 +2187,12 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
* find_jump_table() - Given a dynamic jump, find the switch jump table
* associated with it.
*/
-static struct reloc *find_jump_table(struct objtool_file *file,
- struct symbol *func,
- struct instruction *insn)
+static void find_jump_table(struct objtool_file *file, struct symbol *func,
+ struct instruction *insn)
{
struct reloc *table_reloc;
struct instruction *dest_insn, *orig_insn = insn;
+ unsigned long table_size;
/*
* Backward search using the @first_jump_src links, these help avoid
@@ -2201,17 +2213,17 @@ 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, &table_size);
if (!table_reloc)
continue;
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;
- return table_reloc;
+ orig_insn->_jump_table = table_reloc;
+ orig_insn->_jump_table_size = table_size;
+ break;
}
-
- return NULL;
}
/*
@@ -2222,7 +2234,6 @@ static void mark_func_jump_tables(struct objtool_file *file,
struct symbol *func)
{
struct instruction *insn, *last = NULL;
- struct reloc *reloc;
func_for_each_insn(file, func, insn) {
if (!last)
@@ -2245,9 +2256,7 @@ static void mark_func_jump_tables(struct objtool_file *file,
if (insn->type != INSN_JUMP_DYNAMIC)
continue;
- reloc = find_jump_table(file, func, insn);
- if (reloc)
- insn->_jump_table = reloc;
+ find_jump_table(file, func, insn);
}
}
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index daa46f1f0965..e1cd13cd28a3 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -71,7 +71,10 @@ struct instruction {
struct instruction *first_jump_src;
union {
struct symbol *_call_dest;
- struct reloc *_jump_table;
+ struct {
+ struct reloc *_jump_table;
+ unsigned long _jump_table_size;
+ };
};
struct alternative *alts;
struct symbol *sym;
diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h
index 86d4af9c5aa9..e7ee7ffccefd 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,
+ unsigned long *table_size);
#endif /* _SPECIAL_H */
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/8] objtool: Make some helper functions globally accessible
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 1/8] objtool: Deal with relative jump tables correctly Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 2/8] objtool: Allow arch code to discover jump table size Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 4/8] objtool: Move jump table heuristics to a x86 specific source file Ard Biesheuvel
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
Move some helpers around so they can be used from arch specific jump
table code that is getting refactored in the next patch.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
tools/objtool/check.c | 22 ++++----------------
tools/objtool/include/objtool/check.h | 16 ++++++++++++++
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index b73e43b9b9e3..fbb05e973acc 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -61,8 +61,8 @@ struct instruction *next_insn_same_sec(struct objtool_file *file,
return insn;
}
-static struct instruction *next_insn_same_func(struct objtool_file *file,
- struct instruction *insn)
+struct instruction *next_insn_same_func(struct objtool_file *file,
+ struct instruction *insn)
{
struct instruction *next = next_insn_same_sec(file, insn);
struct symbol *func = insn_func(insn);
@@ -93,8 +93,8 @@ static struct instruction *prev_insn_same_sec(struct objtool_file *file,
return insn - 1;
}
-static struct instruction *prev_insn_same_sym(struct objtool_file *file,
- struct instruction *insn)
+struct instruction *prev_insn_same_sym(struct objtool_file *file,
+ struct instruction *insn)
{
struct instruction *prev = prev_insn_same_sec(file, insn);
@@ -110,11 +110,6 @@ static struct instruction *prev_insn_same_sym(struct objtool_file *file,
for_each_sec(file, __sec) \
sec_for_each_insn(file, __sec, insn)
-#define func_for_each_insn(file, func, insn) \
- for (insn = find_insn(file, func->sec, func->offset); \
- insn; \
- insn = next_insn_same_func(file, insn))
-
#define sym_for_each_insn(file, sym, insn) \
for (insn = find_insn(file, sym->sec, sym->offset); \
insn && insn->offset < sym->offset + sym->len; \
@@ -141,15 +136,6 @@ static inline struct symbol *insn_call_dest(struct instruction *insn)
return insn->_call_dest;
}
-static inline struct reloc *insn_jump_table(struct instruction *insn)
-{
- if (insn->type == INSN_JUMP_DYNAMIC ||
- insn->type == INSN_CALL_DYNAMIC)
- return insn->_jump_table;
-
- return NULL;
-}
-
static inline unsigned long insn_jump_table_size(struct instruction *insn)
{
if (insn->type == INSN_JUMP_DYNAMIC ||
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index e1cd13cd28a3..e2f755484c4a 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -114,14 +114,30 @@ static inline bool is_jump(struct instruction *insn)
return is_static_jump(insn) || is_dynamic_jump(insn);
}
+static inline struct reloc *insn_jump_table(struct instruction *insn)
+{
+ if (insn->type == INSN_JUMP_DYNAMIC ||
+ insn->type == INSN_CALL_DYNAMIC)
+ return insn->_jump_table;
+
+ return NULL;
+}
+
struct instruction *find_insn(struct objtool_file *file,
struct section *sec, unsigned long offset);
+struct instruction *prev_insn_same_sym(struct objtool_file *file, struct instruction *insn);
struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruction *insn);
+struct instruction *next_insn_same_func(struct objtool_file *file, struct instruction *insn);
#define sec_for_each_insn(file, _sec, insn) \
for (insn = find_insn(file, _sec, 0); \
insn && insn->sec == _sec; \
insn = next_insn_same_sec(file, insn))
+#define func_for_each_insn(file, func, insn) \
+ for (insn = find_insn(file, func->sec, func->offset); \
+ insn; \
+ insn = next_insn_same_func(file, insn))
+
#endif /* _CHECK_H */
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/8] objtool: Move jump table heuristics to a x86 specific source file
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
` (2 preceding siblings ...)
2024-10-11 17:08 ` [PATCH v3 3/8] objtool: Make some helper functions globally accessible Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 5/8] objtool: Add generic support for jump table annotations Ard Biesheuvel
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
In preparation for implementing support for the use of compiler emitted
jump table annotations, move the existing code out of the generic
sources. This will permit a clean separation between the two approaches,
where the old one will not be wired up for architectures other than x86.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
tools/objtool/arch/loongarch/special.c | 7 --
tools/objtool/arch/powerpc/special.c | 7 --
tools/objtool/arch/x86/special.c | 114 +++++++++++++++++++-
tools/objtool/check.c | 112 +------------------
tools/objtool/include/objtool/special.h | 9 +-
5 files changed, 122 insertions(+), 127 deletions(-)
diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 87230ed570fd..acf3a391a2f9 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -7,10 +7,3 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
{
return false;
}
-
-struct reloc *arch_find_switch_table(struct objtool_file *file,
- struct instruction *insn,
- unsigned long *table_size)
-{
- return NULL;
-}
diff --git a/tools/objtool/arch/powerpc/special.c b/tools/objtool/arch/powerpc/special.c
index 51610689abf7..3a108437cfa6 100644
--- a/tools/objtool/arch/powerpc/special.c
+++ b/tools/objtool/arch/powerpc/special.c
@@ -11,10 +11,3 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
{
exit(-1);
}
-
-struct reloc *arch_find_switch_table(struct objtool_file *file,
- struct instruction *insn,
- unsigned long *table_size)
-{
- exit(-1);
-}
diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index f8fb67636384..cd964b85e2b1 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -108,9 +108,9 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
*
* NOTE: MITIGATION_RETPOLINE made it harder still to decode dynamic jumps.
*/
-struct reloc *arch_find_switch_table(struct objtool_file *file,
- struct instruction *insn,
- unsigned long *table_size)
+static struct reloc *find_switch_table(struct objtool_file *file,
+ struct instruction *insn,
+ unsigned long *table_size)
{
struct reloc *text_reloc, *rodata_reloc;
struct section *table_sec;
@@ -154,3 +154,111 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
*table_size = 0;
return rodata_reloc;
}
+
+/*
+ * find_jump_table() - Given a dynamic jump, find the switch jump table
+ * associated with it.
+ */
+static void find_jump_table(struct objtool_file *file,
+ struct symbol *func,
+ struct instruction *insn)
+{
+ struct reloc *table_reloc;
+ struct instruction *dest_insn, *orig_insn = insn;
+ unsigned long table_size;
+
+ /*
+ * Backward search using the @first_jump_src links, these help avoid
+ * much of the 'in between' code. Which avoids us getting confused by
+ * it.
+ */
+ for (;
+ 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)
+ break;
+
+ /* allow small jumps within the range */
+ if (insn->type == INSN_JUMP_UNCONDITIONAL &&
+ insn->jump_dest &&
+ (insn->jump_dest->offset <= insn->offset ||
+ insn->jump_dest->offset > orig_insn->offset))
+ break;
+
+ table_reloc = find_switch_table(file, insn, &table_size);
+ if (!table_reloc)
+ continue;
+ 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;
+
+ orig_insn->_jump_table = table_reloc;
+ orig_insn->_jump_table_size = table_size;
+ break;
+ }
+}
+
+/*
+ * 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)
+{
+ struct instruction *insn, *last = NULL;
+
+ func_for_each_insn(file, func, insn) {
+ if (!last)
+ last = insn;
+
+ /*
+ * Store back-pointers for unconditional forward jumps such
+ * 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 &&
+ insn->jump_dest->offset > insn->offset &&
+ !insn->jump_dest->first_jump_src) {
+
+ insn->jump_dest->first_jump_src = insn;
+ last = insn->jump_dest;
+ }
+
+ if (insn->type == INSN_JUMP_DYNAMIC)
+ find_jump_table(file, func, insn);
+ }
+}
+
+int add_func_jump_tables(struct objtool_file *file,
+ struct symbol *func)
+{
+ struct instruction *insn, *insn_t1 = NULL, *insn_t2;
+ int ret = 0;
+
+ mark_func_jump_tables(file, func);
+
+ func_for_each_insn(file, func, insn) {
+ if (!insn_jump_table(insn))
+ continue;
+
+ if (!insn_t1) {
+ insn_t1 = insn;
+ continue;
+ }
+
+ insn_t2 = insn;
+
+ ret = add_jump_table(file, insn_t1, insn_jump_table(insn_t2));
+ if (ret)
+ return ret;
+
+ insn_t1 = insn_t2;
+ }
+
+ if (insn_t1)
+ ret = add_jump_table(file, insn_t1, NULL);
+
+ return ret;
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index fbb05e973acc..389475dde47c 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2091,8 +2091,8 @@ static int add_special_section_alts(struct objtool_file *file)
return ret;
}
-static int add_jump_table(struct objtool_file *file, struct instruction *insn,
- struct reloc *next_table)
+int add_jump_table(struct objtool_file *file, struct instruction *insn,
+ struct reloc *next_table)
{
unsigned long table_size = insn_jump_table_size(insn);
struct symbol *pfunc = insn_func(insn)->pfunc;
@@ -2169,111 +2169,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
return 0;
}
-/*
- * find_jump_table() - Given a dynamic jump, find the switch jump table
- * associated with it.
- */
-static void find_jump_table(struct objtool_file *file, struct symbol *func,
- struct instruction *insn)
+int __weak add_func_jump_tables(struct objtool_file *file,
+ struct symbol *func)
{
- struct reloc *table_reloc;
- struct instruction *dest_insn, *orig_insn = insn;
- unsigned long table_size;
-
- /*
- * Backward search using the @first_jump_src links, these help avoid
- * much of the 'in between' code. Which avoids us getting confused by
- * it.
- */
- for (;
- 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)
- break;
-
- /* allow small jumps within the range */
- if (insn->type == INSN_JUMP_UNCONDITIONAL &&
- insn->jump_dest &&
- (insn->jump_dest->offset <= insn->offset ||
- insn->jump_dest->offset > orig_insn->offset))
- break;
-
- table_reloc = arch_find_switch_table(file, insn, &table_size);
- if (!table_reloc)
- continue;
- 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;
-
- orig_insn->_jump_table = table_reloc;
- orig_insn->_jump_table_size = table_size;
- break;
- }
-}
-
-/*
- * 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)
-{
- struct instruction *insn, *last = NULL;
-
- func_for_each_insn(file, func, insn) {
- if (!last)
- last = insn;
-
- /*
- * Store back-pointers for unconditional forward jumps such
- * 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 &&
- insn->jump_dest->offset > insn->offset &&
- !insn->jump_dest->first_jump_src) {
-
- insn->jump_dest->first_jump_src = insn;
- last = insn->jump_dest;
- }
-
- if (insn->type != INSN_JUMP_DYNAMIC)
- continue;
-
- find_jump_table(file, func, insn);
- }
-}
-
-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))
- continue;
-
- if (!insn_t1) {
- insn_t1 = insn;
- continue;
- }
-
- insn_t2 = insn;
-
- ret = add_jump_table(file, insn_t1, insn_jump_table(insn_t2));
- if (ret)
- return ret;
-
- insn_t1 = insn_t2;
- }
-
- if (insn_t1)
- ret = add_jump_table(file, insn_t1, NULL);
-
- return ret;
+ return 0;
}
/*
@@ -2293,7 +2192,6 @@ 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);
if (ret)
return ret;
diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h
index e7ee7ffccefd..019b511eca6e 100644
--- a/tools/objtool/include/objtool/special.h
+++ b/tools/objtool/include/objtool/special.h
@@ -37,7 +37,10 @@ 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,
- unsigned long *table_size);
+
+int add_func_jump_tables(struct objtool_file *file, struct symbol *func);
+
+int add_jump_table(struct objtool_file *file, struct instruction *insn,
+ struct reloc *next_table);
+
#endif /* _SPECIAL_H */
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 5/8] objtool: Add generic support for jump table annotations
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
` (3 preceding siblings ...)
2024-10-11 17:08 ` [PATCH v3 4/8] objtool: Move jump table heuristics to a x86 specific source file Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 6/8] crypto: x86/crc32c - Use idiomatic relative jump table Ard Biesheuvel
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
Refactor the jump table handling code so that a generic code path is
provided that can identify jump tables attached to indirect jumps based
only on compiler provided annotations. This will be used by non-x86
architectures which do not support jump tables at all at this point.
Refactor the x86 code to share the logic that follows relocations on
instructions into the .rodata section and finds the associated symbols.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
tools/objtool/arch/x86/special.c | 46 ++++------
tools/objtool/check.c | 88 +++++++++++++++++++-
tools/objtool/include/objtool/check.h | 4 +
3 files changed, 106 insertions(+), 32 deletions(-)
diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index cd964b85e2b1..08a5ce662974 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -112,46 +112,34 @@ static struct reloc *find_switch_table(struct objtool_file *file,
struct instruction *insn,
unsigned long *table_size)
{
- 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 = reloc_addend(text_reloc);
- table_sec = text_reloc->sym->sec;
+ struct reloc *rodata_reloc;
+ struct symbol *sym = NULL;
- if (reloc_type(text_reloc) == R_X86_64_PC32)
- table_offset += 4;
+ /*
+ * 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_rodata_sym_reference(file, insn, &sym);
/*
- * Make sure the .rodata address isn't associated with a
- * symbol. GCC jump tables are anonymous data.
+ * Annotations, if present, are attached to the indirect jump
+ * instruction directly. In this case, a symbol annotation is
+ * expected.
+ *
+ * Otherwise, make sure the .rodata address isn't associated with
+ * a symbol. Unannotated 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)
+ if (insn->type != INSN_JUMP_DYNAMIC && sym &&
+ strcmp(sym->sec->name, C_JUMP_TABLE_SECTION))
return NULL;
- *table_size = 0;
+ *table_size = sym ? sym->len : 0;
return rodata_reloc;
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 389475dde47c..b923d4a4efcb 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1372,6 +1372,8 @@ __weak const char *arch_nop_fentry_call(int len)
static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
{
+ unsigned long offset = insn->offset;
+ unsigned int len = insn->len;
struct reloc *reloc;
if (insn->no_reloc)
@@ -1380,8 +1382,12 @@ static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *i
if (!file)
return NULL;
- reloc = find_reloc_by_dest_range(file->elf, insn->sec,
- insn->offset, insn->len);
+ do {
+ /* Skip any R_*_NONE relocations */
+ reloc = find_reloc_by_dest_range(file->elf, insn->sec,
+ offset++, len--);
+ } while (len && reloc && reloc_type(reloc) == R_NONE);
+
if (!reloc) {
insn->no_reloc = 1;
return NULL;
@@ -2169,10 +2175,86 @@ int add_jump_table(struct objtool_file *file, struct instruction *insn,
return 0;
}
+struct reloc *find_rodata_sym_reference(struct objtool_file *file,
+ struct instruction *insn,
+ struct symbol **table_sym)
+{
+ struct reloc *text_reloc, *rodata_reloc;
+ unsigned long addend;
+ struct symbol *sym;
+
+ /*
+ * Look for a relocation which references .rodata. We must use
+ * find_reloc_by_dest_range() directly here, as insn_reloc() filters
+ * out R_*_NONE relocations which are used for jump table annotations.
+ */
+ text_reloc = find_reloc_by_dest_range(file->elf, insn->sec,
+ insn->offset, insn->len);
+ if (!text_reloc) {
+ insn->no_reloc = 1;
+ return NULL;
+ }
+
+ sym = text_reloc->sym;
+ if (!sym->sec->rodata)
+ return NULL;
+
+ if (reloc_type(text_reloc) == elf_data_rela_type(file->elf))
+ addend = arch_dest_reloc_offset(reloc_addend(text_reloc));
+ else
+ addend = reloc_addend(text_reloc);
+
+ rodata_reloc = find_reloc_by_dest(file->elf, sym->sec,
+ sym->offset + addend);
+ if (!rodata_reloc)
+ return NULL;
+
+ /*
+ * Find the ELF symbol covering the destination of the relocation. This
+ * is trivial if the reloc refers to a STT_OBJECT directly, but it may
+ * have been emitted as section relative as well.
+ */
+ if (sym->type == STT_SECTION)
+ sym = find_symbol_containing(sym->sec, addend);
+
+ *table_sym = sym;
+ return rodata_reloc;
+}
+
+/*
+ * Generic version of jump table handling, relying strictly on annotations
+ * provided by the compiler. Overridden for x86 using heuristics that attempt
+ * to correlate indirect jump instructions with preceding .rodata references.
+ */
int __weak add_func_jump_tables(struct objtool_file *file,
struct symbol *func)
{
- return 0;
+ struct instruction *insn;
+ int ret = 0;
+
+ func_for_each_insn(file, func, insn) {
+ struct reloc *reloc;
+ struct symbol *sym;
+
+ if (insn->type != INSN_JUMP_DYNAMIC)
+ continue;
+
+ /*
+ * Look for a relocation attached to this indirect jump that
+ * references an ELF object in .rodata. This should be the jump
+ * table annotation emitted by the compiler.
+ */
+ reloc = find_rodata_sym_reference(file, insn, &sym);
+ if (reloc && sym && sym->len) {
+ insn->_jump_table = reloc;
+ insn->_jump_table_size = sym->len;
+
+ ret = add_jump_table(file, insn, NULL);
+ if (ret)
+ break;
+ }
+ }
+ return ret;
}
/*
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index e2f755484c4a..7781100c9340 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -140,4 +140,8 @@ struct instruction *next_insn_same_func(struct objtool_file *file, struct instru
insn; \
insn = next_insn_same_func(file, insn))
+struct reloc *find_rodata_sym_reference(struct objtool_file *file,
+ struct instruction *insn,
+ struct symbol **sym);
+
#endif /* _CHECK_H */
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 6/8] crypto: x86/crc32c - Use idiomatic relative jump table
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
` (4 preceding siblings ...)
2024-10-11 17:08 ` [PATCH v3 5/8] objtool: Add generic support for jump table annotations Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
2024-10-14 4:28 ` Eric Biggers
2024-10-11 17:08 ` [PATCH v3 7/8] crypto: x86/crc32c - Add jump table annotation Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 8/8] crypto: x86/crc32c-intel - Tweaks to make objtool's life harder Ard Biesheuvel
7 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
The original crc32c code used a place-relative jump table but with a
slightly awkward use of two separate symbols. To help objtool, this was
replaced with a bog-standard position dependent jump table call, which
was subsequently tweaked to use a RIP-relative reference to the table,
but still populate it with absolute 64-bit references.
Given that objtool will need to be taught about the jump table idiom
that compilers use when running with -fpie enabled, let's update the
jump table in the crc32c code once again to use this standard idiom,
where the jump table carries 32-bit references relative to the start of
the table, and the destination address can be obtained by adding the
two.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/x86/crypto/crc32c-pcl-intel-asm_64.S | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
index bbcff1fb78cb..45b005935194 100644
--- a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
+++ b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
@@ -53,7 +53,7 @@
.endm
.macro JMPTBL_ENTRY i
-.quad .Lcrc_\i
+.long .Lcrc_\i - jump_table
.endm
.macro JNC_LESS_THAN j
@@ -169,7 +169,8 @@ SYM_FUNC_START(crc_pcl)
## branch into array
leaq jump_table(%rip), %bufp
- mov (%bufp,%rax,8), %bufp
+ movslq (%bufp,%rax,4), len
+ addq len, %bufp
JMP_NOSPEC bufp
################################################################
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 7/8] crypto: x86/crc32c - Add jump table annotation
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
` (5 preceding siblings ...)
2024-10-11 17:08 ` [PATCH v3 6/8] crypto: x86/crc32c - Use idiomatic relative jump table Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 8/8] crypto: x86/crc32c-intel - Tweaks to make objtool's life harder Ard Biesheuvel
7 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
Annotate the indirect jump with a relocation that correlates it with the
jump table emitted into .rodata. This helps objtool identify the jump
table, allowing it to infer the places in the code that are reachable
from the jump.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/x86/crypto/crc32c-pcl-intel-asm_64.S | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
index 45b005935194..7292090e76dd 100644
--- a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
+++ b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
@@ -171,6 +171,7 @@ SYM_FUNC_START(crc_pcl)
leaq jump_table(%rip), %bufp
movslq (%bufp,%rax,4), len
addq len, %bufp
+ .reloc ., R_X86_64_NONE, jump_table
JMP_NOSPEC bufp
################################################################
@@ -327,6 +328,8 @@ JMPTBL_ENTRY %i
i=i+1
.endr
+.size jump_table, . - jump_table
+.type jump_table, @object
################################################################
## PCLMULQDQ tables
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 8/8] crypto: x86/crc32c-intel - Tweaks to make objtool's life harder
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
` (6 preceding siblings ...)
2024-10-11 17:08 ` [PATCH v3 7/8] crypto: x86/crc32c - Add jump table annotation Ard Biesheuvel
@ 2024-10-11 17:08 ` Ard Biesheuvel
7 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-11 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: llvm, keescook, linux-hardening, nathan, Ard Biesheuvel,
Josh Poimboeuf, Peter Zijlstra, Jan Beulich, Jose E. Marchesi,
Kees Cook
From: Ard Biesheuvel <ardb@kernel.org>
Tweak the asm crc32c asm code so that the jump table is more difficult
to decipher for objtool:
- load the address in the prologue
- move the jump table to the middle of .rodata, so that the section
offset and the symbol offset differ
- emit an entry following the jump table that is unrelated
- add unconditional ENDBRs so we can test --ibt regardless of the kernel
config
Test code only. Not intended for merging.
---
arch/x86/crypto/crc32c-pcl-intel-asm_64.S | 50 +++++++++++---------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
index 7292090e76dd..cbedf5820e30 100644
--- a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
+++ b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
@@ -93,10 +93,14 @@ SYM_FUNC_START(crc_pcl)
#define crc1 %r9
#define crc2 %r10
+ endbr64
+ pushq %rbp
pushq %rbx
pushq %rdi
pushq %rsi
+ leaq jump_table(%rip), %rbp
+
## Move crc_init for Linux to a different
mov crc_init_arg, crc_init
@@ -168,9 +172,8 @@ SYM_FUNC_START(crc_pcl)
xor crc2, crc2
## branch into array
- leaq jump_table(%rip), %bufp
- movslq (%bufp,%rax,4), len
- addq len, %bufp
+ movslq (%rbp,%rax,4), %bufp
+ addq %rbp, %bufp
.reloc ., R_X86_64_NONE, jump_table
JMP_NOSPEC bufp
@@ -197,7 +200,7 @@ SYM_FUNC_START(crc_pcl)
.altmacro
LABEL crc_ %i
.noaltmacro
- ENDBR
+ endbr64
crc32q -i*8(block_0), crc_init
crc32q -i*8(block_1), crc1
crc32q -i*8(block_2), crc2
@@ -207,7 +210,7 @@ LABEL crc_ %i
.altmacro
LABEL crc_ %i
.noaltmacro
- ENDBR
+ endbr64
crc32q -i*8(block_0), crc_init
crc32q -i*8(block_1), crc1
# SKIP crc32 -i*8(block_2), crc2 ; Don't do this one yet
@@ -241,7 +244,7 @@ LABEL crc_ %i
################################################################
LABEL crc_ 0
- ENDBR
+ endbr64
mov tmp, len
cmp $128*24, tmp
jae .Lfull_block
@@ -311,26 +314,11 @@ LABEL less_than_ %j # less_than_j: Length should be in
popq %rsi
popq %rdi
popq %rbx
+ popq %rbp
RET
SYM_FUNC_END(crc_pcl)
.section .rodata, "a", @progbits
- ################################################################
- ## jump table Table is 129 entries x 2 bytes each
- ################################################################
-.align 4
-jump_table:
- i=0
-.rept 129
-.altmacro
-JMPTBL_ENTRY %i
-.noaltmacro
- i=i+1
-.endr
-
-.size jump_table, . - jump_table
-.type jump_table, @object
-
################################################################
## PCLMULQDQ tables
## Table is 128 entries x 2 words (8 bytes) each
@@ -465,3 +453,21 @@ K_table:
.long 0x45cddf4e, 0xe0ac139e
.long 0xacfa3103, 0x6c23e841
.long 0xa51b6135, 0x170076fa
+
+ ################################################################
+ ## jump table Table is 129 entries x 2 bytes each
+ ################################################################
+.align 4
+jump_table:
+ i=0
+.rept 129
+.altmacro
+JMPTBL_ENTRY %i
+.noaltmacro
+ i=i+1
+.endr
+
+.size jump_table, . - jump_table
+.type jump_table, @object
+
+ .long crc_pcl - .
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 6/8] crypto: x86/crc32c - Use idiomatic relative jump table
2024-10-11 17:08 ` [PATCH v3 6/8] crypto: x86/crc32c - Use idiomatic relative jump table Ard Biesheuvel
@ 2024-10-14 4:28 ` Eric Biggers
2024-10-14 9:36 ` Ard Biesheuvel
0 siblings, 1 reply; 11+ messages in thread
From: Eric Biggers @ 2024-10-14 4:28 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-kernel, llvm, keescook, linux-hardening, nathan,
Ard Biesheuvel, Josh Poimboeuf, Peter Zijlstra, Jan Beulich,
Jose E. Marchesi, Kees Cook
On Fri, Oct 11, 2024 at 07:08:54PM +0200, Ard Biesheuvel wrote:
> diff --git a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
> index bbcff1fb78cb..45b005935194 100644
> --- a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
> +++ b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
> @@ -53,7 +53,7 @@
> .endm
>
> .macro JMPTBL_ENTRY i
> -.quad .Lcrc_\i
> +.long .Lcrc_\i - jump_table
> .endm
>
> .macro JNC_LESS_THAN j
> @@ -169,7 +169,8 @@ SYM_FUNC_START(crc_pcl)
>
> ## branch into array
> leaq jump_table(%rip), %bufp
> - mov (%bufp,%rax,8), %bufp
> + movslq (%bufp,%rax,4), len
> + addq len, %bufp
> JMP_NOSPEC bufp
I think it would be much better to just get rid of the jump table here.
I have done this at
https://lore.kernel.org/linux-crypto/20241014042447.50197-4-ebiggers@kernel.org/
- Eric
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 6/8] crypto: x86/crc32c - Use idiomatic relative jump table
2024-10-14 4:28 ` Eric Biggers
@ 2024-10-14 9:36 ` Ard Biesheuvel
0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2024-10-14 9:36 UTC (permalink / raw)
To: Eric Biggers
Cc: Ard Biesheuvel, linux-kernel, llvm, keescook, linux-hardening,
nathan, Josh Poimboeuf, Peter Zijlstra, Jan Beulich,
Jose E. Marchesi, Kees Cook
On Mon, 14 Oct 2024 at 06:28, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Fri, Oct 11, 2024 at 07:08:54PM +0200, Ard Biesheuvel wrote:
> > diff --git a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
> > index bbcff1fb78cb..45b005935194 100644
> > --- a/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
> > +++ b/arch/x86/crypto/crc32c-pcl-intel-asm_64.S
> > @@ -53,7 +53,7 @@
> > .endm
> >
> > .macro JMPTBL_ENTRY i
> > -.quad .Lcrc_\i
> > +.long .Lcrc_\i - jump_table
> > .endm
> >
> > .macro JNC_LESS_THAN j
> > @@ -169,7 +169,8 @@ SYM_FUNC_START(crc_pcl)
> >
> > ## branch into array
> > leaq jump_table(%rip), %bufp
> > - mov (%bufp,%rax,8), %bufp
> > + movslq (%bufp,%rax,4), len
> > + addq len, %bufp
> > JMP_NOSPEC bufp
>
> I think it would be much better to just get rid of the jump table here.
>
Good point.
> I have done this at
> https://lore.kernel.org/linux-crypto/20241014042447.50197-4-ebiggers@kernel.org/
>
I'll go and take a look - thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-14 9:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-11 17:08 [PATCH v3 0/8] Improve objtool jump table handling Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 1/8] objtool: Deal with relative jump tables correctly Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 2/8] objtool: Allow arch code to discover jump table size Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 3/8] objtool: Make some helper functions globally accessible Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 4/8] objtool: Move jump table heuristics to a x86 specific source file Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 5/8] objtool: Add generic support for jump table annotations Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 6/8] crypto: x86/crc32c - Use idiomatic relative jump table Ard Biesheuvel
2024-10-14 4:28 ` Eric Biggers
2024-10-14 9:36 ` Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 7/8] crypto: x86/crc32c - Add jump table annotation Ard Biesheuvel
2024-10-11 17:08 ` [PATCH v3 8/8] crypto: x86/crc32c-intel - Tweaks to make objtool's life harder Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).