objtool: Improve retpoline alternative handling
authorJosh Poimboeuf <jpoimboe@redhat.com>
Tue, 30 Jan 2018 04:00:39 +0000 (22:00 -0600)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 7 Feb 2018 19:12:21 +0000 (11:12 -0800)
commit a845c7cf4b4cb5e9e3b2823867892b27646f3a98

Currently objtool requires all retpolines to be:

  a) patched in with alternatives; and

  b) annotated with ANNOTATE_NOSPEC_ALTERNATIVE.

If you forget to do both of the above, objtool segfaults trying to
dereference a NULL 'insn->call_dest' pointer.

Avoid that situation and print a more helpful error message:

  quirks.o: warning: objtool: efi_delete_dummy_variable()+0x99: unsupported intra-function call
  quirks.o: warning: objtool: If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.

Future improvements can be made to make objtool smarter with respect to
retpolines, but this is a good incremental improvement for now.

Reported-and-tested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/819e50b6d9c2e1a22e34c1a636c0b2057cc8c6e5.1517284349.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
tools/objtool/check.c

index f40d46e..bc3490d 100644 (file)
@@ -543,18 +543,14 @@ static int add_call_destinations(struct objtool_file *file)
                        dest_off = insn->offset + insn->len + insn->immediate;
                        insn->call_dest = find_symbol_by_offset(insn->sec,
                                                                dest_off);
-                       /*
-                        * FIXME: Thanks to retpolines, it's now considered
-                        * normal for a function to call within itself.  So
-                        * disable this warning for now.
-                        */
-#if 0
-                       if (!insn->call_dest) {
-                               WARN_FUNC("can't find call dest symbol at offset 0x%lx",
-                                         insn->sec, insn->offset, dest_off);
+
+                       if (!insn->call_dest && !insn->ignore) {
+                               WARN_FUNC("unsupported intra-function call",
+                                         insn->sec, insn->offset);
+                               WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.");
                                return -1;
                        }
-#endif
+
                } else if (rela->sym->type == STT_SECTION) {
                        insn->call_dest = find_symbol_by_offset(rela->sym->sec,
                                                                rela->addend+4);
@@ -648,6 +644,8 @@ static int handle_group_alt(struct objtool_file *file,
 
                last_new_insn = insn;
 
+               insn->ignore = orig_insn->ignore_alts;
+
                if (insn->type != INSN_JUMP_CONDITIONAL &&
                    insn->type != INSN_JUMP_UNCONDITIONAL)
                        continue;
@@ -729,10 +727,6 @@ static int add_special_section_alts(struct objtool_file *file)
                        goto out;
                }
 
-               /* Ignore retpoline alternatives. */
-               if (orig_insn->ignore_alts)
-                       continue;
-
                new_insn = NULL;
                if (!special_alt->group || special_alt->new_len) {
                        new_insn = find_insn(file, special_alt->new_sec,
@@ -1089,11 +1083,11 @@ static int decode_sections(struct objtool_file *file)
        if (ret)
                return ret;
 
-       ret = add_call_destinations(file);
+       ret = add_special_section_alts(file);
        if (ret)
                return ret;
 
-       ret = add_special_section_alts(file);
+       ret = add_call_destinations(file);
        if (ret)
                return ret;
 
@@ -1720,10 +1714,12 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
 
                insn->visited = true;
 
-               list_for_each_entry(alt, &insn->alts, list) {
-                       ret = validate_branch(file, alt->insn, state);
-                       if (ret)
-                               return 1;
+               if (!insn->ignore_alts) {
+                       list_for_each_entry(alt, &insn->alts, list) {
+                               ret = validate_branch(file, alt->insn, state);
+                               if (ret)
+                                       return 1;
+                       }
                }
 
                switch (insn->type) {