Re: [PATCH] objtool: Fix SYSCALL instruction handling and INSN_CONTEXT_SWITCH

From: Jürgen Groß
Date: Fri Apr 04 2025 - 03:40:39 EST


On 03.04.25 20:48, Josh Poimboeuf wrote:
The !CONFIG_IA32_EMULATION version of xen_entry_SYSCALL_compat() ends
with a SYSCALL instruction, which in reality is a hypervisor call to
trigger an IRET.

Objtool doesn't know that, so it falls through to the next function,
triggering a false positive:

vmlinux.o: warning: objtool: xen_reschedule_interrupt+0x2a: RET before UNTRAIN

Fix that by adding UD2 after the SYSCALL to avoid the undefined behavior
and prevent the objtool fallthrough, and teach validate_unret() to stop
control flow on the UD2 like validate_branch() already does.

Unfortunately that's not the whole story. While that works for
validate_unret(), it breaks validate_branch() which terminates control
flow after the SYSCALL, triggering an unreachable instruction warning on
the UD2.

The real problem here is that 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.

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

INSN_CONTEXT_SWITCH's original purpose was to enable the "unsupported
instruction in callable function" warning. But that warning really has
no reason to exist. It has never found any bugs, and those instructions
are only in entry code anyway. So just get rid of it.

That in turn allows objtool to stop caring about SYSCALL or SYSENTER.
Their call semantic means they usually don't affect control flow in the
containing function/code, and can just be INSN_OTHER. The far
returns/jumps can also be ignored as those aren't used anywhere.

With SYSCALL and SYSENTER, INSN_CONTEXT_SWITCH now has a sane
well-defined return semantic.

Fixes: a2796dff62d6 ("x86/xen: don't do PV iret hypercall through hypercall page")
Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Tested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

For the xen part:

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature