* [GIT PULL] objtool/urgent for v5.15-rc4 @ 2021-10-03 9:43 Borislav Petkov 2021-10-03 18:20 ` pr-tracker-bot 2021-10-03 18:38 ` Linus Torvalds 0 siblings, 2 replies; 7+ messages in thread From: Borislav Petkov @ 2021-10-03 9:43 UTC (permalink / raw) To: Linus Torvalds; +Cc: x86-ml, lkml Hi Linus, please pull a single objtool urgent fix for 5.15. Thx. --- The following changes since commit e4e737bb5c170df6135a127739a9e6148ee3da82: Linux 5.15-rc2 (2021-09-19 17:28:22 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git tags/objtool_urgent_for_v5.15_rc4 for you to fetch changes up to 24ff652573754fe4c03213ebd26b17e86842feb3: objtool: Teach get_alt_entry() about more relocation types (2021-10-01 13:57:47 +0200) ---------------------------------------------------------------- - Handle symbol relocations properly due to changes in the toolchains which remove section symbols now ---------------------------------------------------------------- Peter Zijlstra (1): objtool: Teach get_alt_entry() about more relocation types tools/objtool/special.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] objtool/urgent for v5.15-rc4 2021-10-03 9:43 [GIT PULL] objtool/urgent for v5.15-rc4 Borislav Petkov @ 2021-10-03 18:20 ` pr-tracker-bot 2021-10-03 18:38 ` Linus Torvalds 1 sibling, 0 replies; 7+ messages in thread From: pr-tracker-bot @ 2021-10-03 18:20 UTC (permalink / raw) To: Borislav Petkov; +Cc: Linus Torvalds, x86-ml, lkml The pull request you sent on Sun, 3 Oct 2021 11:43:33 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git tags/objtool_urgent_for_v5.15_rc4 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/52c3c170623d994c468c1ee9cc36c56bbd6d6e56 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] objtool/urgent for v5.15-rc4 2021-10-03 9:43 [GIT PULL] objtool/urgent for v5.15-rc4 Borislav Petkov 2021-10-03 18:20 ` pr-tracker-bot @ 2021-10-03 18:38 ` Linus Torvalds 2021-10-03 19:02 ` Linus Torvalds 1 sibling, 1 reply; 7+ messages in thread From: Linus Torvalds @ 2021-10-03 18:38 UTC (permalink / raw) To: Borislav Petkov, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel Cc: x86-ml, lkml, KVM list On Sun, Oct 3, 2021 at 2:43 AM Borislav Petkov <bp@suse.de> wrote: > > - Handle symbol relocations properly due to changes in the toolchains > which remove section symbols now Ugh. This actually causes a new warning for me: arch/x86/kvm/emulate.o: warning: objtool: __ex_table+0x4: don't know how to handle reloc symbol type: kvm_fastop_exception on an x86-64 allmodconfig build (and my normal clang build for my actual default config too). 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. Anyway, that thing is in my public tree now, because it's better to get it out and fixed and have the kvm people look at it. Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] objtool/urgent for v5.15-rc4 2021-10-03 18:38 ` Linus Torvalds @ 2021-10-03 19:02 ` Linus Torvalds 2021-10-03 19:10 ` Linus Torvalds 0 siblings, 1 reply; 7+ messages in thread From: Linus Torvalds @ 2021-10-03 19:02 UTC (permalink / raw) To: Borislav Petkov, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel Cc: x86-ml, lkml, KVM list [-- 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; } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [GIT PULL] objtool/urgent for v5.15-rc4 2021-10-03 19:02 ` Linus Torvalds @ 2021-10-03 19:10 ` Linus Torvalds 2021-10-03 23:02 ` Josh Poimboeuf 0 siblings, 1 reply; 7+ messages in thread From: Linus Torvalds @ 2021-10-03 19:10 UTC (permalink / raw) To: Borislav Petkov, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Peter Zijlstra (Intel), Josh Poimboeuf, Nathan Chancellor Cc: x86-ml, lkml, KVM list Replying to myself just to add more proper people to the cc. I'm also wondering how I could possibly be the only person who saw the warning. I don't think I am, and I think that people who signed off on commit 24ff65257375 ("objtool: Teach get_alt_entry() about more relocation types") and claimed to have "tested" it, clearly didn't actually do so. PeterZ/Josh/Nathan: see the thread at https://lore.kernel.org/lkml/CAHk-=wiZwq-0LknKhXN4M+T8jbxn_2i9mcKpO+OaBSSq_Eh7tg@mail.gmail.com/ if you need more context, but I suspect you can figure it out just from this email too. Linus On Sun, Oct 3, 2021 at 12:02 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] objtool/urgent for v5.15-rc4 2021-10-03 19:10 ` Linus Torvalds @ 2021-10-03 23:02 ` Josh Poimboeuf 2021-10-04 7:28 ` Paolo Bonzini 0 siblings, 1 reply; 7+ messages in thread From: Josh Poimboeuf @ 2021-10-03 23:02 UTC (permalink / raw) To: Linus Torvalds Cc: Borislav Petkov, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Peter Zijlstra (Intel), Nathan Chancellor, x86-ml, lkml, KVM list On Sun, Oct 03, 2021 at 12:10:38PM -0700, Linus Torvalds wrote: > Replying to myself just to add more proper people to the cc. > > I'm also wondering how I could possibly be the only person who saw the warning. > > I don't think I am, and I think that people who signed off on commit > 24ff65257375 ("objtool: Teach get_alt_entry() about more relocation > types") and claimed to have "tested" it, clearly didn't actually do > so. > > PeterZ/Josh/Nathan: see the thread at > > https://lore.kernel.org/lkml/CAHk-=wiZwq-0LknKhXN4M+T8jbxn_2i9mcKpO+OaBSSq_Eh7tg@mail.gmail.com/ > > if you need more context, but I suspect you can figure it out just > from this email too. Sorry about that. I think Peter and I failed to run this through regression testing. We can work on tightening up our process. Definitely *not* Nathan's fault. His 'Tested-by' only means that this fixed his particular issue. > > > 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. Right, basically objtool's complaining that it doesn't know how to handle the NOTYPE symbol. But really, any non-object symbol should be straightforward. I may just remove these warnings altogether in favor of something much simpler (something like the patch below). > > In the meantime, I think the exception handling for kvm > > divide/multiply emulation is badly broken right now. Hmm? The warning is harmless, so it doesn't necessarily mean anything's broken. That said, I have no idea what's going in that code or why kvm_fastop_exception() is clearing %esi. diff --git a/tools/objtool/special.c b/tools/objtool/special.c index f58ecc50fb10..0217ac3fa7ff 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -58,22 +58,11 @@ void __weak arch_handle_alternative(unsigned short feature, struct special_alt * { } -static bool reloc2sec_off(struct reloc *reloc, struct section **sec, unsigned long *off) +static void reloc_to_sec_off(struct reloc *reloc, struct section **sec, + unsigned long *off) { - switch (reloc->sym->type) { - case STT_FUNC: - *sec = reloc->sym->sec; - *off = reloc->sym->offset + reloc->addend; - return true; - - case STT_SECTION: - *sec = reloc->sym->sec; - *off = reloc->addend; - return true; - - default: - return false; - } + *sec = reloc->sym->sec; + *off = reloc->sym->offset + reloc->addend; } static int get_alt_entry(struct elf *elf, struct special_entry *entry, @@ -109,11 +98,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry, WARN_FUNC("can't find orig reloc", sec, offset + entry->orig); 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); - return -1; - } + reloc_to_sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off); if (!entry->group || alt->new_len) { new_reloc = find_reloc_by_dest(elf, sec, offset + entry->new); @@ -131,11 +116,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry, if (arch_is_retpoline(new_reloc->sym)) 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); - return -1; - } + reloc_to_sec_off(new_reloc, &alt->new_sec, &alt->new_off); /* _ASM_EXTABLE_EX hack */ if (alt->new_off >= 0x7ffffff0) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [GIT PULL] objtool/urgent for v5.15-rc4 2021-10-03 23:02 ` Josh Poimboeuf @ 2021-10-04 7:28 ` Paolo Bonzini 0 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2021-10-04 7:28 UTC (permalink / raw) To: Josh Poimboeuf, Linus Torvalds Cc: Borislav Petkov, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Peter Zijlstra (Intel), Nathan Chancellor, x86-ml, lkml, KVM list On 04/10/21 01:02, Josh Poimboeuf wrote: > That said, I have no idea what's going in that code or why > kvm_fastop_exception() is clearing %esi. It's handled here (which definitely qualifies as "funky sh*t"): asm("push %[flags]; popf; " CALL_NOSPEC " ; pushf; pop %[flags]\n" : "+a"(ctxt->dst.val), "+d"(ctxt->src.val), [flags]"+D"(flags), [thunk_target]"+S"(fop), ASM_CALL_CONSTRAINT : "c"(ctxt->src2.val)); ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK); if (!fop) /* exception is returned in fop variable */ return emulate_de(ctxt); and documented here: /* * fastop functions have a special calling convention: * * dst: rax (in/out) * src: rdx (in/out) * src2: rcx (in) * flags: rflags (in/out) * ex: rsi (in:fastop pointer, out:zero if exception) * * Moreover, they are all exactly FASTOP_SIZE bytes long, so functions for * different operand sizes can be reached by calculation, rather than a jump * table (which would be bigger than the code). */ The fastop stuff saves quite a few clock cycles and lines of code, by avoiding complicated emulation of x86 flags. I'll check out the .global annotations, since they are indeed unnecessary. Paolo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-04 7:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-03 9:43 [GIT PULL] objtool/urgent for v5.15-rc4 Borislav Petkov 2021-10-03 18:20 ` pr-tracker-bot 2021-10-03 18:38 ` Linus Torvalds 2021-10-03 19:02 ` Linus Torvalds 2021-10-03 19:10 ` Linus Torvalds 2021-10-03 23:02 ` Josh Poimboeuf 2021-10-04 7:28 ` Paolo Bonzini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.