* [PATCH v4 1/4] objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()
2025-04-08 7:02 [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Josh Poimboeuf
@ 2025-04-08 7:02 ` Josh Poimboeuf
2025-04-08 8:12 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
2025-04-08 7:02 ` [PATCH v4 2/4] objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET Josh Poimboeuf
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2025-04-08 7:02 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Juergen Gross,
Andrew Cooper
The !CONFIG_IA32_EMULATION version of xen_entry_SYSCALL_compat() ends
with a SYSCALL instruction which is classified by objtool as
INSN_CONTEXT_SWITCH.
Unlike validate_branch(), validate_unret() doesn't consider
INSN_CONTEXT_SWITCH in a non-function to be a dead end, so it keeps
going past the end of xen_entry_SYSCALL_compat(), resulting in the
following warning:
vmlinux.o: warning: objtool: xen_reschedule_interrupt+0x2a: RET before UNTRAIN
Fix that by adding INSN_CONTEXT_SWITCH handling to validate_unret() to
match what validate_branch() is already doing.
Fixes: a09a6e2399ba ("objtool: Add entry UNRET validation")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
tools/objtool/check.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4a1f6c3169b3..c81b070ca495 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3886,6 +3886,11 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
WARN_INSN(insn, "RET before UNTRAIN");
return 1;
+ case INSN_CONTEXT_SWITCH:
+ if (insn_func(insn))
+ break;
+ return 0;
+
case INSN_NOP:
if (insn->retpoline_safe)
return 0;
--
2.49.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [tip: objtool/urgent] objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()
2025-04-08 7:02 ` [PATCH v4 1/4] objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret() Josh Poimboeuf
@ 2025-04-08 8:12 ` tip-bot2 for Josh Poimboeuf
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2025-04-08 8:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: Andrew Cooper, Josh Poimboeuf, Ingo Molnar, Linus Torvalds, x86,
linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: a8df7d0ef92eca28c610206c6748daf537ac0586
Gitweb: https://git.kernel.org/tip/a8df7d0ef92eca28c610206c6748daf537ac0586
Author: Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate: Tue, 08 Apr 2025 00:02:13 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 08 Apr 2025 09:14:11 +02:00
objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()
The !CONFIG_IA32_EMULATION version of xen_entry_SYSCALL_compat() ends
with a SYSCALL instruction which is classified by objtool as
INSN_CONTEXT_SWITCH.
Unlike validate_branch(), validate_unret() doesn't consider
INSN_CONTEXT_SWITCH in a non-function to be a dead end, so it keeps
going past the end of xen_entry_SYSCALL_compat(), resulting in the
following warning:
vmlinux.o: warning: objtool: xen_reschedule_interrupt+0x2a: RET before UNTRAIN
Fix that by adding INSN_CONTEXT_SWITCH handling to validate_unret() to
match what validate_branch() is already doing.
Fixes: a09a6e2399ba ("objtool: Add entry UNRET validation")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/f5eda46fd09f15b1f5cde3d9ae3b92b958342add.1744095216.git.jpoimboe@kernel.org
---
tools/objtool/check.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4a1f6c3..c81b070 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3886,6 +3886,11 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
WARN_INSN(insn, "RET before UNTRAIN");
return 1;
+ case INSN_CONTEXT_SWITCH:
+ if (insn_func(insn))
+ break;
+ return 0;
+
case INSN_NOP:
if (insn->retpoline_safe)
return 0;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 2/4] objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET
2025-04-08 7:02 [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Josh Poimboeuf
2025-04-08 7:02 ` [PATCH v4 1/4] objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret() Josh Poimboeuf
@ 2025-04-08 7:02 ` Josh Poimboeuf
2025-04-08 8:12 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
2025-04-08 7:02 ` [PATCH v4 3/4] objtool: Stop UNRET validation on UD2 Josh Poimboeuf
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2025-04-08 7:02 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Juergen Gross
INSN_CONTEXT_SWITCH is ambiguous. It can represent both call semantics
(SYSCALL, SYSENTER) and return semantics (SYSRET, IRET, RETS, RETU).
Those differ significantly: calls preserve control flow whereas returns
terminate it.
Objtool uses an arbitrary rule for INSN_CONTEXT_SWITCH that almost works
by accident: if in a function, keep going; otherwise stop. It should
instead be based on the semantics of the underlying instruction.
In preparation for improving that, split INSN_CONTEXT_SWITCH into
INSN_SYCALL and INSN_SYSRET.
No functional change.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
tools/objtool/arch/x86/decode.c | 18 +++++++++++-------
tools/objtool/check.c | 6 ++++--
tools/objtool/include/objtool/arch.h | 3 ++-
3 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 33d861c04ebd..3ce7b54003c2 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -522,7 +522,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
case INAT_PFX_REPNE:
if (modrm == 0xca)
/* eretu/erets */
- insn->type = INSN_CONTEXT_SWITCH;
+ insn->type = INSN_SYSRET;
break;
default:
if (modrm == 0xca)
@@ -535,11 +535,15 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
insn->type = INSN_JUMP_CONDITIONAL;
- } else if (op2 == 0x05 || op2 == 0x07 || op2 == 0x34 ||
- op2 == 0x35) {
+ } else if (op2 == 0x05 || op2 == 0x34) {
- /* sysenter, sysret */
- insn->type = INSN_CONTEXT_SWITCH;
+ /* syscall, sysenter */
+ insn->type = INSN_SYSCALL;
+
+ } else if (op2 == 0x07 || op2 == 0x35) {
+
+ /* sysret, sysexit */
+ insn->type = INSN_SYSRET;
} else if (op2 == 0x0b || op2 == 0xb9) {
@@ -676,7 +680,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
case 0xca: /* retf */
case 0xcb: /* retf */
- insn->type = INSN_CONTEXT_SWITCH;
+ insn->type = INSN_SYSRET;
break;
case 0xe0: /* loopne */
@@ -721,7 +725,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
} else if (modrm_reg == 5) {
/* jmpf */
- insn->type = INSN_CONTEXT_SWITCH;
+ insn->type = INSN_SYSRET;
} else if (modrm_reg == 6) {
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c81b070ca495..2c703b960420 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3684,7 +3684,8 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
break;
- case INSN_CONTEXT_SWITCH:
+ case INSN_SYSCALL:
+ case INSN_SYSRET:
if (func) {
if (!next_insn || !next_insn->hint) {
WARN_INSN(insn, "unsupported instruction in callable function");
@@ -3886,7 +3887,8 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
WARN_INSN(insn, "RET before UNTRAIN");
return 1;
- case INSN_CONTEXT_SWITCH:
+ case INSN_SYSCALL:
+ case INSN_SYSRET:
if (insn_func(insn))
break;
return 0;
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 089a1acc48a8..01ef6f415adf 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -19,7 +19,8 @@ enum insn_type {
INSN_CALL,
INSN_CALL_DYNAMIC,
INSN_RETURN,
- INSN_CONTEXT_SWITCH,
+ INSN_SYSCALL,
+ INSN_SYSRET,
INSN_BUG,
INSN_NOP,
INSN_STAC,
--
2.49.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [tip: objtool/urgent] objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET
2025-04-08 7:02 ` [PATCH v4 2/4] objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET Josh Poimboeuf
@ 2025-04-08 8:12 ` tip-bot2 for Josh Poimboeuf
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2025-04-08 8:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: Josh Poimboeuf, Ingo Molnar, Linus Torvalds, x86, linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: fe1042b1ef79e4d5df33d5c0f0ce936493714eec
Gitweb: https://git.kernel.org/tip/fe1042b1ef79e4d5df33d5c0f0ce936493714eec
Author: Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate: Tue, 08 Apr 2025 00:02:14 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 08 Apr 2025 09:14:11 +02:00
objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET
INSN_CONTEXT_SWITCH is ambiguous. It can represent both call semantics
(SYSCALL, SYSENTER) and return semantics (SYSRET, IRET, RETS, RETU).
Those differ significantly: calls preserve control flow whereas returns
terminate it.
Objtool uses an arbitrary rule for INSN_CONTEXT_SWITCH that almost works
by accident: if in a function, keep going; otherwise stop. It should
instead be based on the semantics of the underlying instruction.
In preparation for improving that, split INSN_CONTEXT_SWITCH into
INSN_SYCALL and INSN_SYSRET.
No functional change.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/19a76c74d2c051d3bc9a775823cafc65ad267a7a.1744095216.git.jpoimboe@kernel.org
---
tools/objtool/arch/x86/decode.c | 18 +++++++++++-------
tools/objtool/check.c | 6 ++++--
tools/objtool/include/objtool/arch.h | 3 ++-
3 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 33d861c..3ce7b54 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -522,7 +522,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
case INAT_PFX_REPNE:
if (modrm == 0xca)
/* eretu/erets */
- insn->type = INSN_CONTEXT_SWITCH;
+ insn->type = INSN_SYSRET;
break;
default:
if (modrm == 0xca)
@@ -535,11 +535,15 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
insn->type = INSN_JUMP_CONDITIONAL;
- } else if (op2 == 0x05 || op2 == 0x07 || op2 == 0x34 ||
- op2 == 0x35) {
+ } else if (op2 == 0x05 || op2 == 0x34) {
- /* sysenter, sysret */
- insn->type = INSN_CONTEXT_SWITCH;
+ /* syscall, sysenter */
+ insn->type = INSN_SYSCALL;
+
+ } else if (op2 == 0x07 || op2 == 0x35) {
+
+ /* sysret, sysexit */
+ insn->type = INSN_SYSRET;
} else if (op2 == 0x0b || op2 == 0xb9) {
@@ -676,7 +680,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
case 0xca: /* retf */
case 0xcb: /* retf */
- insn->type = INSN_CONTEXT_SWITCH;
+ insn->type = INSN_SYSRET;
break;
case 0xe0: /* loopne */
@@ -721,7 +725,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
} else if (modrm_reg == 5) {
/* jmpf */
- insn->type = INSN_CONTEXT_SWITCH;
+ insn->type = INSN_SYSRET;
} else if (modrm_reg == 6) {
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c81b070..2c703b9 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3684,7 +3684,8 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
break;
- case INSN_CONTEXT_SWITCH:
+ case INSN_SYSCALL:
+ case INSN_SYSRET:
if (func) {
if (!next_insn || !next_insn->hint) {
WARN_INSN(insn, "unsupported instruction in callable function");
@@ -3886,7 +3887,8 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
WARN_INSN(insn, "RET before UNTRAIN");
return 1;
- case INSN_CONTEXT_SWITCH:
+ case INSN_SYSCALL:
+ case INSN_SYSRET:
if (insn_func(insn))
break;
return 0;
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 089a1ac..01ef6f4 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -19,7 +19,8 @@ enum insn_type {
INSN_CALL,
INSN_CALL_DYNAMIC,
INSN_RETURN,
- INSN_CONTEXT_SWITCH,
+ INSN_SYSCALL,
+ INSN_SYSRET,
INSN_BUG,
INSN_NOP,
INSN_STAC,
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 3/4] objtool: Stop UNRET validation on UD2
2025-04-08 7:02 [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Josh Poimboeuf
2025-04-08 7:02 ` [PATCH v4 1/4] objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret() Josh Poimboeuf
2025-04-08 7:02 ` [PATCH v4 2/4] objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET Josh Poimboeuf
@ 2025-04-08 7:02 ` Josh Poimboeuf
2025-04-08 8:12 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
2025-04-08 7:02 ` [PATCH v4 4/4] objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics Josh Poimboeuf
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2025-04-08 7:02 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Juergen Gross
In preparation for simplifying INSN_SYSCALL, make validate_unret()
terminate control flow on UD2 just like validate_branch() already does.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
tools/objtool/check.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2c703b960420..2dd89b0f4d5e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3902,6 +3902,9 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
break;
}
+ if (insn->dead_end)
+ return 0;
+
if (!next) {
WARN_INSN(insn, "teh end!");
return 1;
--
2.49.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [tip: objtool/urgent] objtool: Stop UNRET validation on UD2
2025-04-08 7:02 ` [PATCH v4 3/4] objtool: Stop UNRET validation on UD2 Josh Poimboeuf
@ 2025-04-08 8:12 ` tip-bot2 for Josh Poimboeuf
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2025-04-08 8:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: Josh Poimboeuf, Ingo Molnar, Linus Torvalds, x86, linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: 9f9cc012c2cbac4833746a0182e06a8eec940d19
Gitweb: https://git.kernel.org/tip/9f9cc012c2cbac4833746a0182e06a8eec940d19
Author: Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate: Tue, 08 Apr 2025 00:02:15 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 08 Apr 2025 09:14:11 +02:00
objtool: Stop UNRET validation on UD2
In preparation for simplifying INSN_SYSCALL, make validate_unret()
terminate control flow on UD2 just like validate_branch() already does.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/ce841269e7e28c8b7f32064464a9821034d724ff.1744095216.git.jpoimboe@kernel.org
---
tools/objtool/check.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2c703b9..2dd89b0 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3902,6 +3902,9 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
break;
}
+ if (insn->dead_end)
+ return 0;
+
if (!next) {
WARN_INSN(insn, "teh end!");
return 1;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 4/4] objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics
2025-04-08 7:02 [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Josh Poimboeuf
` (2 preceding siblings ...)
2025-04-08 7:02 ` [PATCH v4 3/4] objtool: Stop UNRET validation on UD2 Josh Poimboeuf
@ 2025-04-08 7:02 ` Josh Poimboeuf
2025-04-08 8:12 ` [tip: objtool/urgent] " tip-bot2 for Josh Poimboeuf
2025-04-08 7:11 ` [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Ingo Molnar
2025-04-09 14:43 ` Peter Zijlstra
5 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2025-04-08 7:02 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Juergen Gross
Objtool uses an arbitrary rule for INSN_SYSCALL and INSN_SYSRET that
almost works by accident: if it's in a function, control flow continues
after the instruction, otherwise it terminates.
That behavior should instead be based on the semantics of the underlying
instruction. Change INSN_SYSCALL to always preserve control flow and
INSN_SYSRET to always terminate it.
The changed semantic for INSN_SYSCALL requires a tweak to the
!CONFIG_IA32_EMULATION version of xen_entry_SYSCALL_compat(). In Xen,
SYSCALL is a hypercall which usually returns. But in this case it's a
hypercall to IRET which doesn't return. Add UD2 to tell objtool to
terminate control flow, and to prevent undefined behavior at runtime.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
arch/x86/xen/xen-asm.S | 4 +---
tools/objtool/check.c | 23 ++++++++++++++---------
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/arch/x86/xen/xen-asm.S b/arch/x86/xen/xen-asm.S
index 109af12f7647..461bb1526502 100644
--- a/arch/x86/xen/xen-asm.S
+++ b/arch/x86/xen/xen-asm.S
@@ -226,9 +226,7 @@ SYM_CODE_END(xen_early_idt_handler_array)
push %rax
mov $__HYPERVISOR_iret, %eax
syscall /* Do the IRET. */
-#ifdef CONFIG_MITIGATION_SLS
- int3
-#endif
+ ud2 /* The SYSCALL should never return. */
.endm
SYM_CODE_START(xen_iret)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2dd89b0f4d5e..69f94bc47499 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3685,14 +3685,19 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
break;
case INSN_SYSCALL:
- case INSN_SYSRET:
- if (func) {
- if (!next_insn || !next_insn->hint) {
- WARN_INSN(insn, "unsupported instruction in callable function");
- return 1;
- }
- break;
+ if (func && (!next_insn || !next_insn->hint)) {
+ WARN_INSN(insn, "unsupported instruction in callable function");
+ return 1;
}
+
+ break;
+
+ case INSN_SYSRET:
+ if (func && (!next_insn || !next_insn->hint)) {
+ WARN_INSN(insn, "unsupported instruction in callable function");
+ return 1;
+ }
+
return 0;
case INSN_STAC:
@@ -3888,9 +3893,9 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
return 1;
case INSN_SYSCALL:
+ break;
+
case INSN_SYSRET:
- if (insn_func(insn))
- break;
return 0;
case INSN_NOP:
--
2.49.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [tip: objtool/urgent] objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics
2025-04-08 7:02 ` [PATCH v4 4/4] objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics Josh Poimboeuf
@ 2025-04-08 8:12 ` tip-bot2 for Josh Poimboeuf
0 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Josh Poimboeuf @ 2025-04-08 8:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: Josh Poimboeuf, Ingo Molnar, Juergen Gross, Linus Torvalds, x86,
linux-kernel
The following commit has been merged into the objtool/urgent branch of tip:
Commit-ID: 2dbbca9be4e5ed68d0972a2bcf4561d9cb85b7b7
Gitweb: https://git.kernel.org/tip/2dbbca9be4e5ed68d0972a2bcf4561d9cb85b7b7
Author: Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate: Tue, 08 Apr 2025 00:02:16 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 08 Apr 2025 09:14:12 +02:00
objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics
Objtool uses an arbitrary rule for INSN_SYSCALL and INSN_SYSRET that
almost works by accident: if it's in a function, control flow continues
after the instruction, otherwise it terminates.
That behavior should instead be based on the semantics of the underlying
instruction. Change INSN_SYSCALL to always preserve control flow and
INSN_SYSRET to always terminate it.
The changed semantic for INSN_SYSCALL requires a tweak to the
!CONFIG_IA32_EMULATION version of xen_entry_SYSCALL_compat(). In Xen,
SYSCALL is a hypercall which usually returns. But in this case it's a
hypercall to IRET which doesn't return. Add UD2 to tell objtool to
terminate control flow, and to prevent undefined behavior at runtime.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Juergen Gross <jgross@suse.com> # for the Xen part
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/19453dfe9a0431b7f016e9dc16d031cad3812a50.1744095216.git.jpoimboe@kernel.org
---
arch/x86/xen/xen-asm.S | 4 +---
tools/objtool/check.c | 21 +++++++++++++--------
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/arch/x86/xen/xen-asm.S b/arch/x86/xen/xen-asm.S
index 109af12..461bb15 100644
--- a/arch/x86/xen/xen-asm.S
+++ b/arch/x86/xen/xen-asm.S
@@ -226,9 +226,7 @@ SYM_CODE_END(xen_early_idt_handler_array)
push %rax
mov $__HYPERVISOR_iret, %eax
syscall /* Do the IRET. */
-#ifdef CONFIG_MITIGATION_SLS
- int3
-#endif
+ ud2 /* The SYSCALL should never return. */
.endm
SYM_CODE_START(xen_iret)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2dd89b0..69f94bc 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3685,14 +3685,19 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
break;
case INSN_SYSCALL:
+ if (func && (!next_insn || !next_insn->hint)) {
+ WARN_INSN(insn, "unsupported instruction in callable function");
+ return 1;
+ }
+
+ break;
+
case INSN_SYSRET:
- if (func) {
- if (!next_insn || !next_insn->hint) {
- WARN_INSN(insn, "unsupported instruction in callable function");
- return 1;
- }
- break;
+ if (func && (!next_insn || !next_insn->hint)) {
+ WARN_INSN(insn, "unsupported instruction in callable function");
+ return 1;
}
+
return 0;
case INSN_STAC:
@@ -3888,9 +3893,9 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)
return 1;
case INSN_SYSCALL:
+ break;
+
case INSN_SYSRET:
- if (insn_func(insn))
- break;
return 0;
case INSN_NOP:
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH
2025-04-08 7:02 [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Josh Poimboeuf
` (3 preceding siblings ...)
2025-04-08 7:02 ` [PATCH v4 4/4] objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics Josh Poimboeuf
@ 2025-04-08 7:11 ` Ingo Molnar
2025-04-08 7:15 ` Ingo Molnar
2025-04-09 14:43 ` Peter Zijlstra
5 siblings, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2025-04-08 7:11 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: x86, linux-kernel, Peter Zijlstra, Juergen Gross
* Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> I decided to keep the "unsupported instruction in callable function"
> warning, it's not hurting anything. As a result we now have
> INSN_SYSCALL and INSN_SYSRET.
>
> v4:
> - split up patches
> - don't get rid of "unsupported instruction in callable function" warning
> - split INSN_CONTEXT_SWITCH -> INSN_SYSCALL / INSN_SYSRET
>
> v3: https://lore.kernel.org/9b23e4413873bee38961e628b0c73f6d3a26d494.1743799705.git.jpoimboe@kernel.org
>
> Josh Poimboeuf (4):
> objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()
> objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET
> objtool: Stop UNRET validation on UD2
> objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics
>
> arch/x86/xen/xen-asm.S | 4 +---
> tools/objtool/arch/x86/decode.c | 18 ++++++++++-------
> tools/objtool/check.c | 29 +++++++++++++++++++++-------
> tools/objtool/include/objtool/arch.h | 3 ++-
> 4 files changed, 36 insertions(+), 18 deletions(-)
I'm wondering about the timing: can this wait for v6.16, or does it
trigger on some popular config/build-tools combination?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH
2025-04-08 7:11 ` [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Ingo Molnar
@ 2025-04-08 7:15 ` Ingo Molnar
0 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2025-04-08 7:15 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: x86, linux-kernel, Peter Zijlstra, Juergen Gross
* Ingo Molnar <mingo@kernel.org> wrote:
>
> * Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> > I decided to keep the "unsupported instruction in callable function"
> > warning, it's not hurting anything. As a result we now have
> > INSN_SYSCALL and INSN_SYSRET.
> >
> > v4:
> > - split up patches
> > - don't get rid of "unsupported instruction in callable function" warning
> > - split INSN_CONTEXT_SWITCH -> INSN_SYSCALL / INSN_SYSRET
> >
> > v3: https://lore.kernel.org/9b23e4413873bee38961e628b0c73f6d3a26d494.1743799705.git.jpoimboe@kernel.org
> >
> > Josh Poimboeuf (4):
> > objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()
> > objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET
> > objtool: Stop UNRET validation on UD2
> > objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics
> >
> > arch/x86/xen/xen-asm.S | 4 +---
> > tools/objtool/arch/x86/decode.c | 18 ++++++++++-------
> > tools/objtool/check.c | 29 +++++++++++++++++++++-------
> > tools/objtool/include/objtool/arch.h | 3 ++-
> > 4 files changed, 36 insertions(+), 18 deletions(-)
>
> I'm wondering about the timing: can this wait for v6.16, or does it
> trigger on some popular config/build-tools combination?
On a second look, I think these fixes should go upstream now, so I've
applied them to tip:objtool/urgent.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH
2025-04-08 7:02 [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Josh Poimboeuf
` (4 preceding siblings ...)
2025-04-08 7:11 ` [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH Ingo Molnar
@ 2025-04-09 14:43 ` Peter Zijlstra
5 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2025-04-09 14:43 UTC (permalink / raw)
To: Josh Poimboeuf; +Cc: x86, linux-kernel, Ingo Molnar, Juergen Gross
On Tue, Apr 08, 2025 at 12:02:12AM -0700, Josh Poimboeuf wrote:
> I decided to keep the "unsupported instruction in callable function"
> warning, it's not hurting anything. As a result we now have
> INSN_SYSCALL and INSN_SYSRET.
>
> v4:
> - split up patches
> - don't get rid of "unsupported instruction in callable function" warning
> - split INSN_CONTEXT_SWITCH -> INSN_SYSCALL / INSN_SYSRET
>
> v3: https://lore.kernel.org/9b23e4413873bee38961e628b0c73f6d3a26d494.1743799705.git.jpoimboe@kernel.org
>
> Josh Poimboeuf (4):
> objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()
> objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET
> objtool: Stop UNRET validation on UD2
> objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics
This looks nice, thanks for doing that. Belated:
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 12+ messages in thread