[PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH

[PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH

[Josh Poimboeuf]

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(-)

-- 
2.49.0

[PATCH v4 1/4] objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()

[Josh Poimboeuf]

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

[PATCH v4 2/4] objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET

[Josh Poimboeuf]

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

[PATCH v4 3/4] objtool: Stop UNRET validation on UD2

[Josh Poimboeuf]

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

[PATCH v4 4/4] objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics

[Josh Poimboeuf]

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

Re: [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH

[Ingo Molnar]

* 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

Re: [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH

[Ingo Molnar]

* 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

[tip: objtool/urgent] objtool, xen: Fix INSN_SYSCALL / INSN_SYSRET semantics

[tip-bot2 for Josh Poimboeuf]

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:

[tip: objtool/urgent] objtool: Stop UNRET validation on UD2

[tip-bot2 for Josh Poimboeuf]

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;

[tip: objtool/urgent] objtool: Fix INSN_CONTEXT_SWITCH handling in validate_unret()

[tip-bot2 for Josh Poimboeuf]

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;

[tip: objtool/urgent] objtool: Split INSN_CONTEXT_SWITCH into INSN_SYSCALL and INSN_SYSRET

[tip-bot2 for Josh Poimboeuf]

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,

Re: [PATCH v4 0/4] objtool: Fix INSN_CONTEXT_SWITCH

[Peter Zijlstra]

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>