* [PATCH 0/4] objtool: Fix fallthrough detection for vmlinux
@ 2022-04-11 23:10 Josh Poimboeuf
2022-04-11 23:10 ` [PATCH 1/4] x86/uaccess: Don't jump between functions Josh Poimboeuf
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Josh Poimboeuf @ 2022-04-11 23:10 UTC (permalink / raw)
To: x86; +Cc: Peter Zijlstra, linux-kernel
Remove the 'c_file' hack and fix function fallthrough detection for
vmlinux.
Josh Poimboeuf (4):
x86/uaccess: Don't jump between functions
objtool: Don't set 'jump_dest' for sibling calls
objtool: Fix sibling call detection in alternatives
objtool: Fix function fallthrough detection for vmlinux
arch/x86/lib/copy_user_64.S | 87 +++++++++++++++----------
tools/objtool/check.c | 73 +++++++++++----------
tools/objtool/include/objtool/objtool.h | 2 +-
tools/objtool/objtool.c | 1 -
4 files changed, 93 insertions(+), 70 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [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
* [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
* [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
* [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
* [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
* [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
* [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
end of thread, other threads:[~2022-04-19 20:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
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-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
2022-04-19 15:52 ` Josh Poimboeuf
2022-04-19 20:08 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.