Re: [GIT PULL] objtool/urgent for v5.15-rc4

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]

On Sun, Oct 3, 2021 at 11:38 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Looking at the kvm code, that kvm_fastop_exception thing is some funky sh*t.
>
> I _think_ the problem is that 'kvm_fastop_exception' is done with bare
> asm at the top-level and that triggers some odd interaction with other
> section data, but I really don't know.

No, it's the fact that it is marked as a global function (why?) that
it then causes problems.

Now, I don't actually see why it would cause problems (the same way I
don't see why it's marked global). But removing that

     ".global kvm_fastop_exception \n"

works.

I suspect it makes the linker do the relocation for us before objtool
runs, because now that it's a local name, there is no worry about
multiply defined symbols of the same name or anything like that.

I also suspect that the reason for the warning is that the symbol type
has never been declared, so it's not marked as a STT_FUNC in the
relocation information.

So independently of this kvm_fastop_exception issue, I'd suggest the
attached patch for objtool to make the warning more informative for
people who try to debug this.

So I have a fix ("remove the global declaration"), but I really don't
like how random this is.

I also tried to instead keep the symbol global, and just mark
kvm_fastop_exception as a function (and add the proper size
annotation), but that only causes more objtool warnings for the
(generated asm) functions that *use* that symbol. Because they also
don't seem to be properly annotated.

Again, removing the global annotation works around the problem, but
the real underlying issue does seem to be that "funky sh*t" going on
in arch/x86/kvm/emulate.c.

So I'd like more people to look at this.

In the meantime, I think the exception handling for kvm
divide/multiply emulation is badly broken right now. Hmm?

                Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1211 bytes --]

 tools/objtool/special.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index f58ecc50fb10..f1428e32a505 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -110,8 +110,10 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
 		return -1;
 	}
 	if (!reloc2sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off)) {
-		WARN_FUNC("don't know how to handle reloc symbol type: %s",
-			   sec, offset + entry->orig, orig_reloc->sym->name);
+		WARN_FUNC("don't know how to handle reloc symbol type %d: %s",
+			   sec, offset + entry->orig,
+			   orig_reloc->sym->type,
+			   orig_reloc->sym->name);
 		return -1;
 	}
 
@@ -132,8 +134,10 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
 			return 1;
 
 		if (!reloc2sec_off(new_reloc, &alt->new_sec, &alt->new_off)) {
-			WARN_FUNC("don't know how to handle reloc symbol type: %s",
-				  sec, offset + entry->new, new_reloc->sym->name);
+			WARN_FUNC("don't know how to handle reloc symbol type %d: %s",
+				  sec, offset + entry->new,
+				  new_reloc->sym->type,
+				  new_reloc->sym->name);
 			return -1;
 		}