objtool: Detect non-relocated text references

When kernel IBT is enabled, objtool detects all text references in order
to determine which functions can be indirectly branched to.

In text, such references look like one of the following:

   mov    $0x0,%rax        R_X86_64_32S     .init.text+0x7e0a0
   lea    0x0(%rip),%rax   R_X86_64_PC32    autoremove_wake_function-0x4

Either way the function pointer is denoted by a relocation, so objtool
just reads that.

However there are some "lea xxx(%rip)" cases which don't use relocations
because they're referencing code in the same translation unit.  Objtool
doesn't have visibility to those.

The only currently known instances of that are a few hand-coded asm text
references which don't actually need ENDBR.  So it's not actually a
problem at the moment.

However if we enable -fpie, the compiler would start generating them and
there would definitely be bugs in the IBT sealing.

Detect non-relocated text references and handle them appropriately.

[ Note: I removed the manual static_call_tramp check -- that should
  already be handled by the noendbr check. ]

Reported-by: Ard Biesheuvel <ardb@kernel.org>
Tested-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S
index 94ff83f3d3fe927dbd67a00f8ad2cb91eb276486..b200a193beeb354c547f3755c66decbadea6b543 100644 (file)
--- a/arch/x86/kernel/acpi/wakeup_64.S
+++ b/arch/x86/kernel/acpi/wakeup_64.S
@@ -87,6 +87,7 @@ SYM_FUNC_START(do_suspend_lowlevel)
 
        .align 4
 .Lresume_point:
+       ANNOTATE_NOENDBR
        /* We don't restore %rax, it must be 0 anyway */
        movq    $saved_context, %rax
        movq    saved_context_cr4(%rax), %rbx

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 16752b8dfa89923f1f76e59ab1addc203c0e43b2..56163e2124cf3aa6018bc249813a44045551c035 100644 (file)
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -77,6 +77,7 @@ SYM_CODE_START_NOALIGN(startup_64)
        lretq
 
 .Lon_kernel_cs:
+       ANNOTATE_NOENDBR
        UNWIND_HINT_END_OF_STACK
 
 #ifdef CONFIG_AMD_MEM_ENCRYPT

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index ed6bff0e01dcd5fc0317f0654127a10e79f79122..fe1362c345647e966e7f20969ae0f7720055c66e 100644 (file)
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -456,10 +456,6 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
                if (!rex_w)
                        break;
 
-               /* skip RIP relative displacement */
-               if (is_RIP())
-                       break;
-
                /* skip nontrivial SIB */
                if (have_SIB()) {
                        modrm_rm = sib_base;
@@ -467,6 +463,12 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
                                break;
                }
 
+               /* lea disp(%rip), %dst */
+               if (is_RIP()) {
+                       insn->type = INSN_LEA_RIP;
+                       break;
+               }
+
                /* lea disp(%src), %dst */
                ADD_OP(op) {
                        op->src.offset = ins.displacement.value;
@@ -737,7 +739,10 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
                break;
        }
 
-       insn->immediate = ins.immediate.nbytes ? ins.immediate.value : 0;
+       if (ins.immediate.nbytes)
+               insn->immediate = ins.immediate.value;
+       else if (ins.displacement.nbytes)
+               insn->immediate = ins.displacement.value;
 
        return 0;
 }

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 6604f5d038aadfff6e4465bda50662c32c9ff067..7fc96c30b79cf5f88cdd12709d27c86fd6b239b3 100644 (file)
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4392,6 +4392,51 @@ static bool noendbr_range(struct objtool_file *file, struct instruction *insn)
        return insn->offset == sym->offset + sym->len;
 }
 
+static int __validate_ibt_insn(struct objtool_file *file, struct instruction *insn,
+                              struct instruction *dest)
+{
+       if (dest->type == INSN_ENDBR) {
+               mark_endbr_used(dest);
+               return 0;
+       }
+
+       if (insn_func(dest) && insn_func(insn) &&
+           insn_func(dest)->pfunc == insn_func(insn)->pfunc) {
+               /*
+                * Anything from->to self is either _THIS_IP_ or
+                * IRET-to-self.
+                *
+                * There is no sane way to annotate _THIS_IP_ since the
+                * compiler treats the relocation as a constant and is
+                * happy to fold in offsets, skewing any annotation we
+                * do, leading to vast amounts of false-positives.
+                *
+                * There's also compiler generated _THIS_IP_ through
+                * KCOV and such which we have no hope of annotating.
+                *
+                * As such, blanket accept self-references without
+                * issue.
+                */
+               return 0;
+       }
+
+       /*
+        * Accept anything ANNOTATE_NOENDBR.
+        */
+       if (dest->noendbr)
+               return 0;
+
+       /*
+        * Accept if this is the instruction after a symbol
+        * that is (no)endbr -- typical code-range usage.
+        */
+       if (noendbr_range(file, dest))
+               return 0;
+
+       WARN_INSN(insn, "relocation to !ENDBR: %s", offstr(dest->sec, dest->offset));
+       return 1;
+}
+
 static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn)
 {
        struct instruction *dest;
@@ -4404,6 +4449,7 @@ static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn
         * direct/indirect branches:
         */
        switch (insn->type) {
+
        case INSN_CALL:
        case INSN_CALL_DYNAMIC:
        case INSN_JUMP_CONDITIONAL:
@@ -4413,6 +4459,23 @@ static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn
        case INSN_RETURN:
        case INSN_NOP:
                return 0;
+
+       case INSN_LEA_RIP:
+               if (!insn_reloc(file, insn)) {
+                       /* local function pointer reference without reloc */
+
+                       off = arch_jump_destination(insn);
+
+                       dest = find_insn(file, insn->sec, off);
+                       if (!dest) {
+                               WARN_INSN(insn, "corrupt function pointer reference");
+                               return 1;
+                       }
+
+                       return __validate_ibt_insn(file, insn, dest);
+               }
+               break;
+
        default:
                break;
        }
@@ -4423,13 +4486,6 @@ static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn
                                              reloc_offset(reloc) + 1,
                                              (insn->offset + insn->len) - (reloc_offset(reloc) + 1))) {
 
-               /*
-                * static_call_update() references the trampoline, which
-                * doesn't have (or need) ENDBR.  Skip warning in that case.
-                */
-               if (reloc->sym->static_call_tramp)
-                       continue;
-
                off = reloc->sym->offset;
                if (reloc_type(reloc) == R_X86_64_PC32 ||
                    reloc_type(reloc) == R_X86_64_PLT32)
@@ -4441,47 +4497,7 @@ static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn
                if (!dest)
                        continue;
 
-               if (dest->type == INSN_ENDBR) {
-                       mark_endbr_used(dest);
-                       continue;
-               }
-
-               if (insn_func(dest) && insn_func(insn) &&
-                   insn_func(dest)->pfunc == insn_func(insn)->pfunc) {
-                       /*
-                        * Anything from->to self is either _THIS_IP_ or
-                        * IRET-to-self.
-                        *
-                        * There is no sane way to annotate _THIS_IP_ since the
-                        * compiler treats the relocation as a constant and is
-                        * happy to fold in offsets, skewing any annotation we
-                        * do, leading to vast amounts of false-positives.
-                        *
-                        * There's also compiler generated _THIS_IP_ through
-                        * KCOV and such which we have no hope of annotating.
-                        *
-                        * As such, blanket accept self-references without
-                        * issue.
-                        */
-                       continue;
-               }
-
-               /*
-                * Accept anything ANNOTATE_NOENDBR.
-                */
-               if (dest->noendbr)
-                       continue;
-
-               /*
-                * Accept if this is the instruction after a symbol
-                * that is (no)endbr -- typical code-range usage.
-                */
-               if (noendbr_range(file, dest))
-                       continue;
-
-               WARN_INSN(insn, "relocation to !ENDBR: %s", offstr(dest->sec, dest->offset));
-
-               warnings++;
+               warnings += __validate_ibt_insn(file, insn, dest);
        }
 
        return warnings;

diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 0b303eba660e4625757682544a3b4a555e288511..d63b46a19f39792cc00b7da96b005e8c71a93af9 100644 (file)
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -28,6 +28,7 @@ enum insn_type {
        INSN_CLD,
        INSN_TRAP,
        INSN_ENDBR,
+       INSN_LEA_RIP,
        INSN_OTHER,
 };