* [PATCH 1/4] x86/uaccess: Don't jump between functions
2022-04-11 23:10 [PATCH 0/4] objtool: Fix fallthrough detection for vmlinux Josh Poimboeuf
@ 2022-04-11 23:10 ` Josh Poimboeuf
2022-04-19 20:08 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 2/4] objtool: Don't set 'jump_dest' for sibling calls Josh Poimboeuf
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Josh Poimboeuf @ 2022-04-11 23:10 UTC (permalink / raw)
To: x86; +Cc: Peter Zijlstra, linux-kernel
For unwinding sanity, a function shouldn't jump to the middle of another
function. Move the short string user copy code out to a separate
non-function code snippet.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
arch/x86/lib/copy_user_64.S | 87 ++++++++++++++++++++++---------------
1 file changed, 52 insertions(+), 35 deletions(-)
diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
index 8ca5ecf16dc4..9dec1b38a98f 100644
--- a/arch/x86/lib/copy_user_64.S
+++ b/arch/x86/lib/copy_user_64.S
@@ -53,12 +53,12 @@
SYM_FUNC_START(copy_user_generic_unrolled)
ASM_STAC
cmpl $8,%edx
- jb 20f /* less then 8 bytes, go to byte copy loop */
+ jb .Lcopy_user_short_string_bytes
ALIGN_DESTINATION
movl %edx,%ecx
andl $63,%edx
shrl $6,%ecx
- jz .L_copy_short_string
+ jz copy_user_short_string
1: movq (%rsi),%r8
2: movq 1*8(%rsi),%r9
3: movq 2*8(%rsi),%r10
@@ -79,37 +79,11 @@ SYM_FUNC_START(copy_user_generic_unrolled)
leaq 64(%rdi),%rdi
decl %ecx
jnz 1b
-.L_copy_short_string:
- movl %edx,%ecx
- andl $7,%edx
- shrl $3,%ecx
- jz 20f
-18: movq (%rsi),%r8
-19: movq %r8,(%rdi)
- leaq 8(%rsi),%rsi
- leaq 8(%rdi),%rdi
- decl %ecx
- jnz 18b
-20: andl %edx,%edx
- jz 23f
- movl %edx,%ecx
-21: movb (%rsi),%al
-22: movb %al,(%rdi)
- incq %rsi
- incq %rdi
- decl %ecx
- jnz 21b
-23: xor %eax,%eax
- ASM_CLAC
- RET
+ jmp copy_user_short_string
30: shll $6,%ecx
addl %ecx,%edx
- jmp 60f
-40: leal (%rdx,%rcx,8),%edx
- jmp 60f
-50: movl %ecx,%edx
-60: jmp .Lcopy_user_handle_tail /* ecx is zerorest also */
+ jmp .Lcopy_user_handle_tail
_ASM_EXTABLE_CPY(1b, 30b)
_ASM_EXTABLE_CPY(2b, 30b)
@@ -127,10 +101,6 @@ SYM_FUNC_START(copy_user_generic_unrolled)
_ASM_EXTABLE_CPY(14b, 30b)
_ASM_EXTABLE_CPY(15b, 30b)
_ASM_EXTABLE_CPY(16b, 30b)
- _ASM_EXTABLE_CPY(18b, 40b)
- _ASM_EXTABLE_CPY(19b, 40b)
- _ASM_EXTABLE_CPY(21b, 50b)
- _ASM_EXTABLE_CPY(22b, 50b)
SYM_FUNC_END(copy_user_generic_unrolled)
EXPORT_SYMBOL(copy_user_generic_unrolled)
@@ -191,7 +161,7 @@ EXPORT_SYMBOL(copy_user_generic_string)
SYM_FUNC_START(copy_user_enhanced_fast_string)
ASM_STAC
/* CPUs without FSRM should avoid rep movsb for short copies */
- ALTERNATIVE "cmpl $64, %edx; jb .L_copy_short_string", "", X86_FEATURE_FSRM
+ ALTERNATIVE "cmpl $64, %edx; jb copy_user_short_string", "", X86_FEATURE_FSRM
movl %edx,%ecx
1: rep movsb
xorl %eax,%eax
@@ -243,6 +213,53 @@ SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail)
SYM_CODE_END(.Lcopy_user_handle_tail)
+/*
+ * Finish memcpy of less than 64 bytes. #AC should already be set.
+ *
+ * Input:
+ * rdi destination
+ * rsi source
+ * rdx count (< 64)
+ *
+ * Output:
+ * eax uncopied bytes or 0 if successful.
+ */
+SYM_CODE_START_LOCAL(copy_user_short_string)
+ movl %edx,%ecx
+ andl $7,%edx
+ shrl $3,%ecx
+ jz .Lcopy_user_short_string_bytes
+18: movq (%rsi),%r8
+19: movq %r8,(%rdi)
+ leaq 8(%rsi),%rsi
+ leaq 8(%rdi),%rdi
+ decl %ecx
+ jnz 18b
+.Lcopy_user_short_string_bytes:
+ andl %edx,%edx
+ jz 23f
+ movl %edx,%ecx
+21: movb (%rsi),%al
+22: movb %al,(%rdi)
+ incq %rsi
+ incq %rdi
+ decl %ecx
+ jnz 21b
+23: xor %eax,%eax
+ ASM_CLAC
+ RET
+
+40: leal (%rdx,%rcx,8),%edx
+ jmp 60f
+50: movl %ecx,%edx /* ecx is zerorest also */
+60: jmp .Lcopy_user_handle_tail
+
+ _ASM_EXTABLE_CPY(18b, 40b)
+ _ASM_EXTABLE_CPY(19b, 40b)
+ _ASM_EXTABLE_CPY(21b, 50b)
+ _ASM_EXTABLE_CPY(22b, 50b)
+SYM_CODE_END(copy_user_short_string)
+
/*
* copy_user_nocache - Uncached memory copy with exception handling
* This will force destination out of cache for more performance.
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [tip: objtool/urgent] x86/uaccess: Don't jump between functions
2022-04-11 23:10 ` [PATCH 1/4] x86/uaccess: Don't jump between functions Josh Poimboeuf
@ 2022-04-19 20:08 ` tip-bot2 for Josh Poimboeuf
0 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2022-04-19 20:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: Josh Poimboeuf, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: 02041b32256628aef0d18ec15d3658fe41bc1afe
Gitweb: https://git.kernel.org/tip/02041b32256628aef0d18ec15d3658fe41bc1afe
Author: Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Mon, 11 Apr 2022 16:10:29 -07:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 19 Apr 2022 21:58:53 +02:00
x86/uaccess: Don't jump between functions
For unwinding sanity, a function shouldn't jump to the middle of another
function. Move the short string user copy code out to a separate
non-function code snippet.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/9519e4853148b765e047967708f2b61e56c93186.1649718562.git.jpoimboe@redhat.com
---
arch/x86/lib/copy_user_64.S | 87 +++++++++++++++++++++---------------
1 file changed, 52 insertions(+), 35 deletions(-)
diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
index 8ca5ecf..9dec1b3 100644
--- a/arch/x86/lib/copy_user_64.S
+++ b/arch/x86/lib/copy_user_64.S
@@ -53,12 +53,12 @@
SYM_FUNC_START(copy_user_generic_unrolled)
ASM_STAC
cmpl $8,%edx
- jb 20f /* less then 8 bytes, go to byte copy loop */
+ jb .Lcopy_user_short_string_bytes
ALIGN_DESTINATION
movl %edx,%ecx
andl $63,%edx
shrl $6,%ecx
- jz .L_copy_short_string
+ jz copy_user_short_string
1: movq (%rsi),%r8
2: movq 1*8(%rsi),%r9
3: movq 2*8(%rsi),%r10
@@ -79,37 +79,11 @@ SYM_FUNC_START(copy_user_generic_unrolled)
leaq 64(%rdi),%rdi
decl %ecx
jnz 1b
-.L_copy_short_string:
- movl %edx,%ecx
- andl $7,%edx
- shrl $3,%ecx
- jz 20f
-18: movq (%rsi),%r8
-19: movq %r8,(%rdi)
- leaq 8(%rsi),%rsi
- leaq 8(%rdi),%rdi
- decl %ecx
- jnz 18b
-20: andl %edx,%edx
- jz 23f
- movl %edx,%ecx
-21: movb (%rsi),%al
-22: movb %al,(%rdi)
- incq %rsi
- incq %rdi
- decl %ecx
- jnz 21b
-23: xor %eax,%eax
- ASM_CLAC
- RET
+ jmp copy_user_short_string
30: shll $6,%ecx
addl %ecx,%edx
- jmp 60f
-40: leal (%rdx,%rcx,8),%edx
- jmp 60f
-50: movl %ecx,%edx
-60: jmp .Lcopy_user_handle_tail /* ecx is zerorest also */
+ jmp .Lcopy_user_handle_tail
_ASM_EXTABLE_CPY(1b, 30b)
_ASM_EXTABLE_CPY(2b, 30b)
@@ -127,10 +101,6 @@ SYM_FUNC_START(copy_user_generic_unrolled)
_ASM_EXTABLE_CPY(14b, 30b)
_ASM_EXTABLE_CPY(15b, 30b)
_ASM_EXTABLE_CPY(16b, 30b)
- _ASM_EXTABLE_CPY(18b, 40b)
- _ASM_EXTABLE_CPY(19b, 40b)
- _ASM_EXTABLE_CPY(21b, 50b)
- _ASM_EXTABLE_CPY(22b, 50b)
SYM_FUNC_END(copy_user_generic_unrolled)
EXPORT_SYMBOL(copy_user_generic_unrolled)
@@ -191,7 +161,7 @@ EXPORT_SYMBOL(copy_user_generic_string)
SYM_FUNC_START(copy_user_enhanced_fast_string)
ASM_STAC
/* CPUs without FSRM should avoid rep movsb for short copies */
- ALTERNATIVE "cmpl $64, %edx; jb .L_copy_short_string", "", X86_FEATURE_FSRM
+ ALTERNATIVE "cmpl $64, %edx; jb copy_user_short_string", "", X86_FEATURE_FSRM
movl %edx,%ecx
1: rep movsb
xorl %eax,%eax
@@ -244,6 +214,53 @@ SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail)
SYM_CODE_END(.Lcopy_user_handle_tail)
/*
+ * Finish memcpy of less than 64 bytes. #AC should already be set.
+ *
+ * Input:
+ * rdi destination
+ * rsi source
+ * rdx count (< 64)
+ *
+ * Output:
+ * eax uncopied bytes or 0 if successful.
+ */
+SYM_CODE_START_LOCAL(copy_user_short_string)
+ movl %edx,%ecx
+ andl $7,%edx
+ shrl $3,%ecx
+ jz .Lcopy_user_short_string_bytes
+18: movq (%rsi),%r8
+19: movq %r8,(%rdi)
+ leaq 8(%rsi),%rsi
+ leaq 8(%rdi),%rdi
+ decl %ecx
+ jnz 18b
+.Lcopy_user_short_string_bytes:
+ andl %edx,%edx
+ jz 23f
+ movl %edx,%ecx
+21: movb (%rsi),%al
+22: movb %al,(%rdi)
+ incq %rsi
+ incq %rdi
+ decl %ecx
+ jnz 21b
+23: xor %eax,%eax
+ ASM_CLAC
+ RET
+
+40: leal (%rdx,%rcx,8),%edx
+ jmp 60f
+50: movl %ecx,%edx /* ecx is zerorest also */
+60: jmp .Lcopy_user_handle_tail
+
+ _ASM_EXTABLE_CPY(18b, 40b)
+ _ASM_EXTABLE_CPY(19b, 40b)
+ _ASM_EXTABLE_CPY(21b, 50b)
+ _ASM_EXTABLE_CPY(22b, 50b)
+SYM_CODE_END(copy_user_short_string)
+
+/*
* copy_user_nocache - Uncached memory copy with exception handling
* This will force destination out of cache for more performance.
*
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] objtool: Don't set 'jump_dest' for sibling calls
2022-04-11 23:10 [PATCH 0/4] objtool: Fix fallthrough detection for vmlinux Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 1/4] x86/uaccess: Don't jump between functions Josh Poimboeuf
@ 2022-04-11 23:10 ` Josh Poimboeuf
2022-04-19 20:08 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 3/4] objtool: Fix sibling call detection in alternatives Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 4/4] objtool: Fix function fallthrough detection for vmlinux Josh Poimboeuf
3 siblings, 1 reply; 10+ messages in thread
From: Josh Poimboeuf @ 2022-04-11 23:10 UTC (permalink / raw)
To: x86; +Cc: Peter Zijlstra, linux-kernel
For most sibling calls, 'jump_dest' is NULL because objtool treats the
jump like a call and sets 'call_dest'. But there are a few edge cases
where that's not true. Make it consistent to avoid unexpected behavior.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
tools/objtool/check.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index bd0c2c828940..6f492789c8c0 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1271,7 +1271,7 @@ static bool is_first_func_insn(struct objtool_file *file, struct instruction *in
*/
static int add_jump_destinations(struct objtool_file *file)
{
- struct instruction *insn;
+ struct instruction *insn, *jump_dest;
struct reloc *reloc;
struct section *dest_sec;
unsigned long dest_off;
@@ -1291,7 +1291,10 @@ static int add_jump_destinations(struct objtool_file *file)
add_retpoline_call(file, insn);
continue;
} else if (insn->func) {
- /* internal or external sibling call (with reloc) */
+ /*
+ * External sibling call or internal sibling call with
+ * STT_FUNC reloc.
+ */
add_call_dest(file, insn, reloc->sym, true);
continue;
} else if (reloc->sym->sec->idx) {
@@ -1303,8 +1306,8 @@ static int add_jump_destinations(struct objtool_file *file)
continue;
}
- insn->jump_dest = find_insn(file, dest_sec, dest_off);
- if (!insn->jump_dest) {
+ jump_dest = find_insn(file, dest_sec, dest_off);
+ if (!jump_dest) {
/*
* This is a special case where an alt instruction
@@ -1323,8 +1326,8 @@ static int add_jump_destinations(struct objtool_file *file)
/*
* Cross-function jump.
*/
- if (insn->func && insn->jump_dest->func &&
- insn->func != insn->jump_dest->func) {
+ if (insn->func && jump_dest->func &&
+ insn->func != jump_dest->func) {
/*
* For GCC 8+, create parent/child links for any cold
@@ -1342,16 +1345,22 @@ static int add_jump_destinations(struct objtool_file *file)
* subfunction is through a jump table.
*/
if (!strstr(insn->func->name, ".cold") &&
- strstr(insn->jump_dest->func->name, ".cold")) {
- insn->func->cfunc = insn->jump_dest->func;
- insn->jump_dest->func->pfunc = insn->func;
+ strstr(jump_dest->func->name, ".cold")) {
+ insn->func->cfunc = jump_dest->func;
+ jump_dest->func->pfunc = insn->func;
- } else if (!same_function(insn, insn->jump_dest) &&
- is_first_func_insn(file, insn->jump_dest)) {
- /* internal sibling call (without reloc) */
- add_call_dest(file, insn, insn->jump_dest->func, true);
+ } else if (!same_function(insn, jump_dest) &&
+ is_first_func_insn(file, jump_dest)) {
+ /*
+ * Internal sibling call without reloc or with
+ * STT_SECTION reloc.
+ */
+ add_call_dest(file, insn, jump_dest->func, true);
+ continue;
}
}
+
+ insn->jump_dest = jump_dest;
}
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [tip: objtool/urgent] objtool: Don't set 'jump_dest' for sibling calls
2022-04-11 23:10 ` [PATCH 2/4] objtool: Don't set 'jump_dest' for sibling calls Josh Poimboeuf
@ 2022-04-19 20:08 ` tip-bot2 for Josh Poimboeuf
0 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2022-04-19 20:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: Josh Poimboeuf, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: 26ff604102c98df79c3fe2614d1b9bb068d4c28c
Gitweb: https://git.kernel.org/tip/26ff604102c98df79c3fe2614d1b9bb068d4c28c
Author: Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Mon, 11 Apr 2022 16:10:30 -07:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 19 Apr 2022 21:58:53 +02:00
objtool: Don't set 'jump_dest' for sibling calls
For most sibling calls, 'jump_dest' is NULL because objtool treats the
jump like a call and sets 'call_dest'. But there are a few edge cases
where that's not true. Make it consistent to avoid unexpected behavior.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/8737d6b9d1691831aed73375f444f0f42da3e2c9.1649718562.git.jpoimboe@redhat.com
---
tools/objtool/check.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index bd0c2c8..6f49278 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1271,7 +1271,7 @@ static bool is_first_func_insn(struct objtool_file *file, struct instruction *in
*/
static int add_jump_destinations(struct objtool_file *file)
{
- struct instruction *insn;
+ struct instruction *insn, *jump_dest;
struct reloc *reloc;
struct section *dest_sec;
unsigned long dest_off;
@@ -1291,7 +1291,10 @@ static int add_jump_destinations(struct objtool_file *file)
add_retpoline_call(file, insn);
continue;
} else if (insn->func) {
- /* internal or external sibling call (with reloc) */
+ /*
+ * External sibling call or internal sibling call with
+ * STT_FUNC reloc.
+ */
add_call_dest(file, insn, reloc->sym, true);
continue;
} else if (reloc->sym->sec->idx) {
@@ -1303,8 +1306,8 @@ static int add_jump_destinations(struct objtool_file *file)
continue;
}
- insn->jump_dest = find_insn(file, dest_sec, dest_off);
- if (!insn->jump_dest) {
+ jump_dest = find_insn(file, dest_sec, dest_off);
+ if (!jump_dest) {
/*
* This is a special case where an alt instruction
@@ -1323,8 +1326,8 @@ static int add_jump_destinations(struct objtool_file *file)
/*
* Cross-function jump.
*/
- if (insn->func && insn->jump_dest->func &&
- insn->func != insn->jump_dest->func) {
+ if (insn->func && jump_dest->func &&
+ insn->func != jump_dest->func) {
/*
* For GCC 8+, create parent/child links for any cold
@@ -1342,16 +1345,22 @@ static int add_jump_destinations(struct objtool_file *file)
* subfunction is through a jump table.
*/
if (!strstr(insn->func->name, ".cold") &&
- strstr(insn->jump_dest->func->name, ".cold")) {
- insn->func->cfunc = insn->jump_dest->func;
- insn->jump_dest->func->pfunc = insn->func;
+ strstr(jump_dest->func->name, ".cold")) {
+ insn->func->cfunc = jump_dest->func;
+ jump_dest->func->pfunc = insn->func;
- } else if (!same_function(insn, insn->jump_dest) &&
- is_first_func_insn(file, insn->jump_dest)) {
- /* internal sibling call (without reloc) */
- add_call_dest(file, insn, insn->jump_dest->func, true);
+ } else if (!same_function(insn, jump_dest) &&
+ is_first_func_insn(file, jump_dest)) {
+ /*
+ * Internal sibling call without reloc or with
+ * STT_SECTION reloc.
+ */
+ add_call_dest(file, insn, jump_dest->func, true);
+ continue;
}
}
+
+ insn->jump_dest = jump_dest;
}
return 0;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] objtool: Fix sibling call detection in alternatives
2022-04-11 23:10 [PATCH 0/4] objtool: Fix fallthrough detection for vmlinux Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 1/4] x86/uaccess: Don't jump between functions Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 2/4] objtool: Don't set 'jump_dest' for sibling calls Josh Poimboeuf
@ 2022-04-11 23:10 ` Josh Poimboeuf
2022-04-19 20:08 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 4/4] objtool: Fix function fallthrough detection for vmlinux Josh Poimboeuf
3 siblings, 1 reply; 10+ messages in thread
From: Josh Poimboeuf @ 2022-04-11 23:10 UTC (permalink / raw)
To: x86; +Cc: Peter Zijlstra, linux-kernel
In add_jump_destinations(), sibling call detection requires 'insn->func'
to be valid. But alternative instructions get their 'func' set in
handle_group_alt(), which runs *after* add_jump_destinations(). So
sibling calls in alternatives code don't get properly detected.
Fix that by changing the initialization order: call
add_special_section_alts() *before* add_jump_destinations().
This also means the special case for a missing 'jump_dest' in
add_jump_destinations() can be removed, as it has already been dealt
with.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
tools/objtool/check.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 6f492789c8c0..0f5d3de30e0d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1277,6 +1277,13 @@ static int add_jump_destinations(struct objtool_file *file)
unsigned long dest_off;
for_each_insn(file, insn) {
+ if (insn->jump_dest) {
+ /*
+ * handle_group_alt() may have previously set
+ * 'jump_dest' for some alternatives.
+ */
+ continue;
+ }
if (!is_static_jump(insn))
continue;
@@ -1308,15 +1315,6 @@ static int add_jump_destinations(struct objtool_file *file)
jump_dest = find_insn(file, dest_sec, dest_off);
if (!jump_dest) {
-
- /*
- * This is a special case where an alt instruction
- * jumps past the end of the section. These are
- * handled later in handle_group_alt().
- */
- if (!strcmp(insn->sec->name, ".altinstr_replacement"))
- continue;
-
WARN_FUNC("can't find jump dest instruction at %s+0x%lx",
insn->sec, insn->offset, dest_sec->name,
dest_off);
@@ -1549,13 +1547,13 @@ static int handle_group_alt(struct objtool_file *file,
continue;
dest_off = arch_jump_destination(insn);
- if (dest_off == special_alt->new_off + special_alt->new_len)
+ if (dest_off == special_alt->new_off + special_alt->new_len) {
insn->jump_dest = next_insn_same_sec(file, last_orig_insn);
-
- if (!insn->jump_dest) {
- WARN_FUNC("can't find alternative jump destination",
- insn->sec, insn->offset);
- return -1;
+ if (!insn->jump_dest) {
+ WARN_FUNC("can't find alternative jump destination",
+ insn->sec, insn->offset);
+ return -1;
+ }
}
}
@@ -2254,14 +2252,14 @@ static int decode_sections(struct objtool_file *file)
return ret;
/*
- * Must be before add_special_section_alts() as that depends on
- * jump_dest being set.
+ * Must be before add_jump_destinations(), which depends on 'func'
+ * being set for alternatives, to enable proper sibling call detection.
*/
- ret = add_jump_destinations(file);
+ ret = add_special_section_alts(file);
if (ret)
return ret;
- ret = add_special_section_alts(file);
+ ret = add_jump_destinations(file);
if (ret)
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [tip: objtool/urgent] objtool: Fix sibling call detection in alternatives
2022-04-11 23:10 ` [PATCH 3/4] objtool: Fix sibling call detection in alternatives Josh Poimboeuf
@ 2022-04-19 20:08 ` tip-bot2 for Josh Poimboeuf
0 siblings, 0 replies; 10+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2022-04-19 20:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: Josh Poimboeuf, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: 34c861e806478ac2ea4032721defbf1d6967df08
Gitweb: https://git.kernel.org/tip/34c861e806478ac2ea4032721defbf1d6967df08
Author: Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Mon, 11 Apr 2022 16:10:31 -07:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 19 Apr 2022 21:58:53 +02:00
objtool: Fix sibling call detection in alternatives
In add_jump_destinations(), sibling call detection requires 'insn->func'
to be valid. But alternative instructions get their 'func' set in
handle_group_alt(), which runs *after* add_jump_destinations(). So
sibling calls in alternatives code don't get properly detected.
Fix that by changing the initialization order: call
add_special_section_alts() *before* add_jump_destinations().
This also means the special case for a missing 'jump_dest' in
add_jump_destinations() can be removed, as it has already been dealt
with.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/c02e0a0a2a4286b5f848d17c77fdcb7e0caf709c.1649718562.git.jpoimboe@redhat.com
---
tools/objtool/check.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 6f49278..0f5d3de 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1277,6 +1277,13 @@ static int add_jump_destinations(struct objtool_file *file)
unsigned long dest_off;
for_each_insn(file, insn) {
+ if (insn->jump_dest) {
+ /*
+ * handle_group_alt() may have previously set
+ * 'jump_dest' for some alternatives.
+ */
+ continue;
+ }
if (!is_static_jump(insn))
continue;
@@ -1308,15 +1315,6 @@ static int add_jump_destinations(struct objtool_file *file)
jump_dest = find_insn(file, dest_sec, dest_off);
if (!jump_dest) {
-
- /*
- * This is a special case where an alt instruction
- * jumps past the end of the section. These are
- * handled later in handle_group_alt().
- */
- if (!strcmp(insn->sec->name, ".altinstr_replacement"))
- continue;
-
WARN_FUNC("can't find jump dest instruction at %s+0x%lx",
insn->sec, insn->offset, dest_sec->name,
dest_off);
@@ -1549,13 +1547,13 @@ static int handle_group_alt(struct objtool_file *file,
continue;
dest_off = arch_jump_destination(insn);
- if (dest_off == special_alt->new_off + special_alt->new_len)
+ if (dest_off == special_alt->new_off + special_alt->new_len) {
insn->jump_dest = next_insn_same_sec(file, last_orig_insn);
-
- if (!insn->jump_dest) {
- WARN_FUNC("can't find alternative jump destination",
- insn->sec, insn->offset);
- return -1;
+ if (!insn->jump_dest) {
+ WARN_FUNC("can't find alternative jump destination",
+ insn->sec, insn->offset);
+ return -1;
+ }
}
}
@@ -2254,14 +2252,14 @@ static int decode_sections(struct objtool_file *file)
return ret;
/*
- * Must be before add_special_section_alts() as that depends on
- * jump_dest being set.
+ * Must be before add_jump_destinations(), which depends on 'func'
+ * being set for alternatives, to enable proper sibling call detection.
*/
- ret = add_jump_destinations(file);
+ ret = add_special_section_alts(file);
if (ret)
return ret;
- ret = add_special_section_alts(file);
+ ret = add_jump_destinations(file);
if (ret)
return ret;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] objtool: Fix function fallthrough detection for vmlinux
2022-04-11 23:10 [PATCH 0/4] objtool: Fix fallthrough detection for vmlinux Josh Poimboeuf
` (2 preceding siblings ...)
2022-04-11 23:10 ` [PATCH 3/4] objtool: Fix sibling call detection in alternatives Josh Poimboeuf
@ 2022-04-11 23:10 ` Josh Poimboeuf
2022-04-19 15:52 ` Josh Poimboeuf
2022-04-19 20:08 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
3 siblings, 2 replies; 10+ messages in thread
From: Josh Poimboeuf @ 2022-04-11 23:10 UTC (permalink / raw)
To: x86; +Cc: Peter Zijlstra, linux-kernel
Objtool's function fallthrough detection only works on C objects.
The distinction between C and assembly objects no longer makes sense
with objtool running on vmlinux.o.
Now that copy_user_64.S has been fixed up, and an objtool sibling call
detection bug has been fixed, the asm code is in "compliance" and this
hack is no longer needed. Remove it.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
tools/objtool/check.c | 2 +-
tools/objtool/include/objtool/objtool.h | 2 +-
tools/objtool/objtool.c | 1 -
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0f5d3de30e0d..5f10653eb5c2 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3310,7 +3310,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
while (1) {
next_insn = next_insn_to_validate(file, insn);
- if (file->c_file && func && insn->func && func != insn->func->pfunc) {
+ if (func && insn->func && func != insn->func->pfunc) {
WARN("%s() falls through to next function %s()",
func->name, insn->func->name);
return 1;
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index 7a5c13a78f87..a6e72d916807 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -27,7 +27,7 @@ struct objtool_file {
struct list_head static_call_list;
struct list_head mcount_loc_list;
struct list_head endbr_list;
- bool ignore_unreachables, c_file, hints, rodata;
+ bool ignore_unreachables, hints, rodata;
unsigned int nr_endbr;
unsigned int nr_endbr_int;
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index b09946f4e1d6..843ff3c2f28e 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -129,7 +129,6 @@ struct objtool_file *objtool_open_read(const char *_objname)
INIT_LIST_HEAD(&file.static_call_list);
INIT_LIST_HEAD(&file.mcount_loc_list);
INIT_LIST_HEAD(&file.endbr_list);
- file.c_file = !vmlinux && find_section_by_name(file.elf, ".comment");
file.ignore_unreachables = no_unreachable;
file.hints = false;
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] objtool: Fix function fallthrough detection for vmlinux
2022-04-11 23:10 ` [PATCH 4/4] objtool: Fix function fallthrough detection for vmlinux Josh Poimboeuf
@ 2022-04-19 15:52 ` Josh Poimboeuf
2022-04-19 20:08 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
1 sibling, 0 replies; 10+ messages in thread
From: Josh Poimboeuf @ 2022-04-19 15:52 UTC (permalink / raw)
To: x86; +Cc: Peter Zijlstra, linux-kernel
On Mon, Apr 11, 2022 at 04:10:32PM -0700, Josh Poimboeuf wrote:
> Objtool's function fallthrough detection only works on C objects.
> The distinction between C and assembly objects no longer makes sense
> with objtool running on vmlinux.o.
>
> Now that copy_user_64.S has been fixed up, and an objtool sibling call
> detection bug has been fixed, the asm code is in "compliance" and this
> hack is no longer needed. Remove it.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Fixes: ed53a0d97192 ("x86/alternative: Use .ibt_endbr_seal to seal indirect calls")
--
Josh
^ permalink raw reply [flat|nested] 10+ messages in thread* [tip: objtool/urgent] objtool: Fix function fallthrough detection for vmlinux
2022-04-11 23:10 ` [PATCH 4/4] objtool: Fix function fallthrough detection for vmlinux Josh Poimboeuf
2022-04-19 15:52 ` Josh Poimboeuf
@ 2022-04-19 20:08 ` tip-bot2 for Josh Poimboeuf
1 sibling, 0 replies; 10+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2022-04-19 20:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: Josh Poimboeuf, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: 08feafe8d1958febf3a9733a3d1564d8fc23340e
Gitweb: https://git.kernel.org/tip/08feafe8d1958febf3a9733a3d1564d8fc23340e
Author: Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Mon, 11 Apr 2022 16:10:32 -07:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 19 Apr 2022 21:58:53 +02:00
objtool: Fix function fallthrough detection for vmlinux
Objtool's function fallthrough detection only works on C objects.
The distinction between C and assembly objects no longer makes sense
with objtool running on vmlinux.o.
Now that copy_user_64.S has been fixed up, and an objtool sibling call
detection bug has been fixed, the asm code is in "compliance" and this
hack is no longer needed. Remove it.
Fixes: ed53a0d97192 ("x86/alternative: Use .ibt_endbr_seal to seal indirect calls")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/b434cff98eca3a60dcc64c620d7d5d405a0f441c.1649718562.git.jpoimboe@redhat.com
---
tools/objtool/check.c | 2 +-
tools/objtool/include/objtool/objtool.h | 2 +-
tools/objtool/objtool.c | 1 -
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0f5d3de..5f10653 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3310,7 +3310,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
while (1) {
next_insn = next_insn_to_validate(file, insn);
- if (file->c_file && func && insn->func && func != insn->func->pfunc) {
+ if (func && insn->func && func != insn->func->pfunc) {
WARN("%s() falls through to next function %s()",
func->name, insn->func->name);
return 1;
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index 7a5c13a..a6e72d9 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -27,7 +27,7 @@ struct objtool_file {
struct list_head static_call_list;
struct list_head mcount_loc_list;
struct list_head endbr_list;
- bool ignore_unreachables, c_file, hints, rodata;
+ bool ignore_unreachables, hints, rodata;
unsigned int nr_endbr;
unsigned int nr_endbr_int;
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index b09946f..843ff3c 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -129,7 +129,6 @@ struct objtool_file *objtool_open_read(const char *_objname)
INIT_LIST_HEAD(&file.static_call_list);
INIT_LIST_HEAD(&file.mcount_loc_list);
INIT_LIST_HEAD(&file.endbr_list);
- file.c_file = !vmlinux && find_section_by_name(file.elf, ".comment");
file.ignore_unreachables = no_unreachable;
file.hints = false;
^ permalink raw reply related [flat|nested] 10+ messages in thread