On Thu, Mar 10, 2022 at 09:22:59AM +0000, David Laight wrote:
> From: Peter Zijlstra
> > Sent: 10 March 2022 09:05
> >
> > On Wed, Mar 09, 2022 at 04:30:28PM -0800, Nick Desaulniers wrote:
> >
> > > I observed the following error when building with
> > > CONFIG_LTO_CLANG_FULL=y enabled:
> > >
> > > ld.lld: error: ld-temp.o <inline asm>:7:2: symbol 'ibt_selftest_ip' is
> > > already defined
> > > ibt_selftest_ip:
> > > ^
> > >
> > > Seems to come from
> > > commit a802350ba65a ("x86/ibt: Add IBT feature, MSR and #CP handling")
> > >
> > > Commenting out the label in the inline asm, I then observed:
> > > vmlinux.o: warning: objtool: identify_cpu()+0x6d0: sibling call from
> > > callable instruction with modified stack frame
> > > vmlinux.o: warning: objtool: identify_cpu()+0x6e0: stack state
> > > mismatch: cfa1=4+64 cfa2=4+8
> > > These seemed to disappear when I kept CONFIG_LTO_CLANG_FULL=y but then
> > > disabled CONFIG_X86_KERNEL_IBT. (perhaps due to the way I hacked out
> > > the ibt_selftest_ip label).
> >
> > Urgh.. I'm thikning this is a clang bug :/
> >
> > The code in question is:
> >
> >
> > void ibt_selftest_ip(void); /* code label defined in asm below */
> >
> > DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
> > {
> > /* ... */
> >
> > if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
> > regs->ax = 0;
> > return;
> > }
> >
> > /* ... */
> > }
> >
> > bool ibt_selftest(void)
> > {
> > unsigned long ret;
> >
> > asm (" lea ibt_selftest_ip(%%rip), %%rax\n\t"
> > ANNOTATE_RETPOLINE_SAFE
> > " jmp *%%rax\n\t"
> > "ibt_selftest_ip:\n\t"
> > UNWIND_HINT_FUNC
> > ANNOTATE_NOENDBR
> > " nop\n\t"
> >
> > : "=a" (ret) : : "memory");
> >
> > return !ret;
> > }
> >
> > There is only a single definition of that symbol, the one in the asm.
> > The other is a declaration, which is used in the exception handler to
> > compare against regs->ip.
>
> LTO has probably inlined it twice.
Indeed, adding noinline to ibt_selftest() makes it work.
---
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index d8bbc705efe5..0c737cc31ee5 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -781,7 +781,8 @@ int3_exception_notify(struct notifier_block *self, unsigned long val, void *data
return NOTIFY_STOP;
}
-static void __init int3_selftest(void)
+/* Must be noinline to ensure uniqueness of int3_selftest_ip. */
+static noinline void __init int3_selftest(void)
{
static __initdata struct notifier_block int3_exception_nb = {
.notifier_call = int3_exception_notify,
@@ -794,9 +795,8 @@ static void __init int3_selftest(void)
/*
* Basically: int3_magic(&val); but really complicated :-)
*
- * Stick the address of the INT3 instruction into int3_selftest_ip,
- * then trigger the INT3, padded with NOPs to match a CALL instruction
- * length.
+ * INT3 padded with NOP to CALL_INSN_SIZE. The int3_exception_nb
+ * notifier above will emulate CALL for us.
*/
asm volatile ("int3_selftest_ip:\n\t"
ANNOTATE_NOENDBR
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 837cc3c7d4f4..fb89a2f1011f 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -214,7 +214,7 @@ DEFINE_IDTENTRY(exc_overflow)
static __ro_after_init bool ibt_fatal = true;
-void ibt_selftest_ip(void); /* code label defined in asm below */
+extern void ibt_selftest_ip(void); /* code label defined in asm below */
enum cp_error_code {
CP_EC = (1 << 15) - 1,
@@ -238,7 +238,7 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
if (WARN_ON_ONCE(user_mode(regs) || (error_code & CP_EC) != CP_ENDBR))
return;
- if (unlikely(regs->ip == (unsigned long)ibt_selftest_ip)) {
+ if (unlikely(regs->ip == (unsigned long)&ibt_selftest_ip)) {
regs->ax = 0;
return;
}
@@ -252,7 +252,8 @@ DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
BUG();
}
-bool ibt_selftest(void)
+/* Must be noinline to ensure uniqueness of ibt_selftest_ip. */
+noinline bool ibt_selftest(void)
{
unsigned long ret;
