* [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f
@ 2018-02-07 11:44 kbuild test robot
2018-02-07 18:13 ` Linus Torvalds
0 siblings, 1 reply; 35+ messages in thread
From: kbuild test robot @ 2018-02-07 11:44 UTC (permalink / raw)
To: Linus Torvalds; +Cc: LKML
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
branch HEAD: a2e5790d841658485d642196dbb0927303d6c22f Merge branch 'akpm' (patches from Andrew)
Regressions in current branch:
arch/x86/tools/insn_decoder_test: warning: ffffffff810010d7: 0f ff eb ud0 %ebx,%ebp
arch/x86/tools/insn_decoder_test: warning: ffffffff81001152: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi
arch/x86/tools/insn_decoder_test: warning: ffffffff81001275: 0f ff eb ud0 %ebx,%ebp
arch/x86/tools/insn_decoder_test: warning: ffffffff810013fd: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi
arch/x86/tools/insn_decoder_test: warning: ffffffff81001499: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi
arch/x86/tools/insn_decoder_test: warning: ffffffff812296eb: 0f ff e8 ud0 %eax,%ebp
Error ids grouped by kconfigs:
recent_errors
├── x86_64-randconfig-r0-02071059
│ ├── arch-x86-tools-insn_decoder_test:warning:ffffffff810013fd:0f-ff-bf-ud0-(-rdi)-edi
│ └── arch-x86-tools-insn_decoder_test:warning:ffffffff81001499:0f-ff-bf-ud0-(-rdi)-edi
└── x86_64-randconfig-s3-02070914
├── arch-x86-tools-insn_decoder_test:warning:ffffffff810010d7:0f-ff-eb-ud0-ebx-ebp
├── arch-x86-tools-insn_decoder_test:warning:ffffffff81001152:0f-ff-bf-ud0-(-rdi)-edi
├── arch-x86-tools-insn_decoder_test:warning:ffffffff81001275:0f-ff-eb-ud0-ebx-ebp
└── arch-x86-tools-insn_decoder_test:warning:ffffffff812296eb:0f-ff-e8-ud0-eax-ebp
elapsed time: 302m
configs tested: 124
x86_64 allmodconfig
i386 tinyconfig
i386 randconfig-x019-201805
i386 randconfig-x010-201805
i386 randconfig-x015-201805
i386 randconfig-x014-201805
i386 randconfig-x011-201805
i386 randconfig-x012-201805
i386 randconfig-x017-201805
i386 randconfig-x016-201805
i386 randconfig-x018-201805
i386 randconfig-x013-201805
i386 randconfig-n0-201805
i386 randconfig-i0-201805
i386 randconfig-i1-201805
parisc c3000_defconfig
parisc b180_defconfig
parisc defconfig
alpha defconfig
parisc allnoconfig
i386 allmodconfig
i386 randconfig-a1-201805
i386 randconfig-a0-201805
x86_64 acpi-redef
x86_64 allyesdebian
x86_64 nfsroot
cris etrax-100lx_v2_defconfig
x86_64 randconfig-x017-201805
x86_64 randconfig-x018-201805
x86_64 randconfig-x013-201805
x86_64 randconfig-x016-201805
x86_64 randconfig-x011-201805
x86_64 randconfig-x014-201805
x86_64 randconfig-x012-201805
x86_64 randconfig-x015-201805
x86_64 randconfig-x010-201805
x86_64 randconfig-x019-201805
i386 randconfig-s0-201805
i386 randconfig-s1-201805
x86_64 randconfig-i0-02051046
microblaze mmu_defconfig
microblaze nommu_defconfig
mn10300 asb2364_defconfig
openrisc or1ksim_defconfig
um x86_64_defconfig
um i386_defconfig
frv defconfig
tile tilegx_defconfig
i386 randconfig-x071-201805
i386 randconfig-x079-201805
i386 randconfig-x072-201805
i386 randconfig-x077-201805
i386 randconfig-x076-201805
i386 randconfig-x073-201805
i386 randconfig-x070-201805
i386 randconfig-x075-201805
i386 randconfig-x078-201805
i386 randconfig-x074-201805
sh titan_defconfig
sh rsk7269_defconfig
sh sh7785lcr_32bit_defconfig
sh allnoconfig
i386 randconfig-x004-201805
i386 randconfig-x007-201805
i386 randconfig-x002-201805
i386 randconfig-x009-201805
i386 randconfig-x003-201805
i386 randconfig-x006-201805
i386 randconfig-x005-201805
i386 randconfig-x000-201805
i386 randconfig-x008-201805
i386 randconfig-x001-201805
m68k sun3_defconfig
m68k multi_defconfig
m68k m5475evb_defconfig
sparc defconfig
sparc64 allnoconfig
sparc64 defconfig
s390 default_defconfig
arm at91_dt_defconfig
arm allnoconfig
arm efm32_defconfig
arm64 defconfig
arm multi_v5_defconfig
arm sunxi_defconfig
arm64 allnoconfig
arm exynos_defconfig
arm shmobile_defconfig
arm multi_v7_defconfig
c6x evmc6678_defconfig
xtensa common_defconfig
m32r m32104ut_defconfig
score spct6600_defconfig
xtensa iss_defconfig
m32r opsput_defconfig
m32r usrv_defconfig
m32r mappi3.smp_defconfig
nios2 10m50_defconfig
h8300 h8300h-sim_defconfig
i386 allnoconfig
i386 defconfig
i386 alldefconfig
x86_64 kexec
x86_64 rhel
x86_64 randconfig-x008-201805
x86_64 randconfig-x000-201805
x86_64 randconfig-x005-201805
x86_64 randconfig-x002-201805
x86_64 randconfig-x007-201805
x86_64 randconfig-x009-201805
x86_64 randconfig-x001-201805
x86_64 randconfig-x006-201805
x86_64 randconfig-x003-201805
x86_64 randconfig-x004-201805
ia64 allnoconfig
ia64 defconfig
ia64 alldefconfig
mips jz4740
mips malta_kvm_defconfig
mips 64r6el_defconfig
mips 32r2_defconfig
mips allnoconfig
mips fuloong2e_defconfig
mips txx9
Thanks,
Fengguang
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 11:44 [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f kbuild test robot @ 2018-02-07 18:13 ` Linus Torvalds 2018-02-07 18:35 ` Borislav Petkov 2018-02-07 18:38 ` [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f Randy Dunlap 0 siblings, 2 replies; 35+ messages in thread From: Linus Torvalds @ 2018-02-07 18:13 UTC (permalink / raw) To: kbuild test robot, Peter Zijlstra, Ingo Molnar, Thomas Gleixner Cc: LKML, the arch/x86 maintainers Adding more people for this funky warning from the kbuild robot. Something is confused. UD0 is 0f ff, the bytes after that shouldn't matter. But I guess they can be interpreted as modrm bytes, and somebody started doing that. That said, intel only _documents_ UD2 (0f 0b). Maybe we should avoid using UD0/UD1 entirely. Linus On Wed, Feb 7, 2018 at 3:44 AM, kbuild test robot <fengguang.wu@intel.com> wrote: > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > branch HEAD: a2e5790d841658485d642196dbb0927303d6c22f Merge branch 'akpm' (patches from Andrew) > > Regressions in current branch: > > arch/x86/tools/insn_decoder_test: warning: ffffffff810010d7: 0f ff eb ud0 %ebx,%ebp > arch/x86/tools/insn_decoder_test: warning: ffffffff81001152: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi > arch/x86/tools/insn_decoder_test: warning: ffffffff81001275: 0f ff eb ud0 %ebx,%ebp > arch/x86/tools/insn_decoder_test: warning: ffffffff810013fd: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi > arch/x86/tools/insn_decoder_test: warning: ffffffff81001499: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi > arch/x86/tools/insn_decoder_test: warning: ffffffff812296eb: 0f ff e8 ud0 %eax,%ebp ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 18:13 ` Linus Torvalds @ 2018-02-07 18:35 ` Borislav Petkov 2018-02-07 18:49 ` Peter Zijlstra 2018-02-07 18:38 ` [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f Randy Dunlap 1 sibling, 1 reply; 35+ messages in thread From: Borislav Petkov @ 2018-02-07 18:35 UTC (permalink / raw) To: Linus Torvalds Cc: kbuild test robot, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 07, 2018 at 10:13:35AM -0800, Linus Torvalds wrote: > Adding more people for this funky warning from the kbuild robot. > > Something is confused. UD0 is 0f ff, the bytes after that shouldn't > matter. But I guess they can be interpreted as modrm bytes, and > somebody started doing that. > > That said, intel only _documents_ UD2 (0f 0b). They documented UD0 and UD1 a year ago or so: 0F FF /r UD0¹ r32, r/m32 RM Valid Valid Raise invalid opcode exception 0F B9 /r UD1 r32, r/m32 RM Valid Valid Raise invalid opcode exception. and the footnote says "1. Some older processors decode the UD0 instruction without a ModR/M byte. As a result, those processors would deliver an invalid- opcode exception instead of a fault on instruction fetch when the instruction with a ModR/M byte (and any implied bytes) would cross a page or segment boundary." So those two take a ModRM byte. And we chose UD0 for WARN, see arch/x86/include/asm/bug.h for the reasoning. Except objdump can't handle that insn because it doesn't have it in its insn tables. Thus it says: b3: 0f ff (bad) b5: eb .byte 0xeb > Maybe we should avoid using UD0/UD1 entirely. Or that test should ignore UD0. Or we should add UD0 only *decoding* support to binutils - not generating. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 18:35 ` Borislav Petkov @ 2018-02-07 18:49 ` Peter Zijlstra 2018-02-07 19:03 ` Linus Torvalds 0 siblings, 1 reply; 35+ messages in thread From: Peter Zijlstra @ 2018-02-07 18:49 UTC (permalink / raw) To: Borislav Petkov Cc: Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 07, 2018 at 07:35:43PM +0100, Borislav Petkov wrote: > On Wed, Feb 07, 2018 at 10:13:35AM -0800, Linus Torvalds wrote: > > Adding more people for this funky warning from the kbuild robot. > > > > Something is confused. UD0 is 0f ff, the bytes after that shouldn't > > matter. But I guess they can be interpreted as modrm bytes, and > > somebody started doing that. > > > > That said, intel only _documents_ UD2 (0f 0b). > > They documented UD0 and UD1 a year ago or so: > > 0F FF /r UD0¹ r32, r/m32 RM Valid Valid Raise invalid opcode exception > 0F B9 /r UD1 r32, r/m32 RM Valid Valid Raise invalid opcode exception. > > and the footnote says > > "1. Some older processors decode the UD0 instruction without a ModR/M > byte. As a result, those processors would deliver an invalid- opcode > exception instead of a fault on instruction fetch when the instruction > with a ModR/M byte (and any implied bytes) would cross a page or segment > boundary." > > So those two take a ModRM byte. Argh. So the SDM I'm looking at (March 2017) doesn't list UD0 as having a ModR/M byte, it doesn't have that footnote. > And we chose UD0 for WARN, see arch/x86/include/asm/bug.h for the > reasoning. Right, we picked UD0 because we _thought_ everybody agreed it being 2 bytes, just like UD2. This is now not true anymore? ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 18:49 ` Peter Zijlstra @ 2018-02-07 19:03 ` Linus Torvalds 2018-02-07 19:14 ` Peter Zijlstra 0 siblings, 1 reply; 35+ messages in thread From: Linus Torvalds @ 2018-02-07 19:03 UTC (permalink / raw) To: Peter Zijlstra Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 7, 2018 at 10:49 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > Right, we picked UD0 because we _thought_ everybody agreed it being 2 > bytes, just like UD2. This is now not true anymore? Both UD0 and UD1 are documented to have modrm in the latest Intel SDM I can find (Order number 325462-065US, December 2017): https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf for the direct link. Linus ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 19:03 ` Linus Torvalds @ 2018-02-07 19:14 ` Peter Zijlstra 2018-02-07 19:28 ` Borislav Petkov 0 siblings, 1 reply; 35+ messages in thread From: Peter Zijlstra @ 2018-02-07 19:14 UTC (permalink / raw) To: Linus Torvalds Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 07, 2018 at 11:03:42AM -0800, Linus Torvalds wrote: > On Wed, Feb 7, 2018 at 10:49 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > > > Right, we picked UD0 because we _thought_ everybody agreed it being 2 > > bytes, just like UD2. This is now not true anymore? > > Both UD0 and UD1 are documented to have modrm in the latest Intel SDM > I can find (Order number 325462-065US, December 2017): Thanks, and yes :/ They changed it in October17 (which I actually had a copy of so I'm not sure why I was looking at March17). Back when we did that patch the SDM didn't have that extra ModR/M on, and binutils disassembled it as the 2 byte (bad) thing, which was all good. Then someone went and wrecked it. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 19:14 ` Peter Zijlstra @ 2018-02-07 19:28 ` Borislav Petkov 2018-02-07 19:43 ` Linus Torvalds 0 siblings, 1 reply; 35+ messages in thread From: Borislav Petkov @ 2018-02-07 19:28 UTC (permalink / raw) To: Peter Zijlstra Cc: Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 07, 2018 at 08:14:51PM +0100, Peter Zijlstra wrote: > Then someone went and wrecked it. Yeah, note says UD0 didn't eat a ModRM byte on old CPUs. But then that changed too. Fun stuff changing insn encoding underway. So if we opt for adding a ModRM byte, could a 0x90 NOP work so that it doesn't shit itself on those old CPUs? /me goes and checks Well, no: b3: 0f ff eb ud0 %ebx,%ebp decoded with latest objdump turns into: b3: 0f .byte 0xf b4: ff .byte 0xff b5: 90 nop because 0x90, when used as a ModRM means, AFAICT, register-indirect addressing with a 32-bit offset which would need more bytes. :-\ /me adds more bytes... Yap, yuck, that works: b3: 0f ff 90 90 90 90 90 ud0 -0x6f6f6f70(%rax),%edx I guess we need to experiment a bit to find a suitable byte to add... Nasty. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 19:28 ` Borislav Petkov @ 2018-02-07 19:43 ` Linus Torvalds 2018-02-07 20:24 ` Borislav Petkov 2018-02-08 9:13 ` Peter Zijlstra 0 siblings, 2 replies; 35+ messages in thread From: Linus Torvalds @ 2018-02-07 19:43 UTC (permalink / raw) To: Borislav Petkov Cc: Peter Zijlstra, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 7, 2018 at 11:28 AM, Borislav Petkov <bp@alien8.de> wrote: > On Wed, Feb 07, 2018 at 08:14:51PM +0100, Peter Zijlstra wrote: >> Then someone went and wrecked it. > > Yeah, note says UD0 didn't eat a ModRM byte on old CPUs. But then that > changed too. Fun stuff changing insn encoding underway. > > So if we opt for adding a ModRM byte, could a 0x90 NOP work so that it > doesn't shit itself on those old CPUs? We could just also decide that the only thing that the modrm bytes of UD0 actually *affect* is how the CPU might act for a page-crossing instruction. Because I think that's the only semantic difference: if it's a page-crosser, the instruction could take a page fault before raising the #UD. Is there any other decode issue we might want to look out for? Anyway, then we'd make the rule be: - we promise to always pad up the following bytes (our extra warning information etc) so that we never have a missing page afterwards (this is presumably practically speaking already the case). It might still be a page-crossing instruction, but we won't take a page fault in kernel space (due to it being at the end of some text sectoin or whatever that changes the NX bit or due to DEBUG_PAGEALLOC having unmapped the next page). - we special-case the decoder so that we don't get this warning and then we just ignore the issue entirely. Hmm? Linus ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 19:43 ` Linus Torvalds @ 2018-02-07 20:24 ` Borislav Petkov 2018-02-08 9:13 ` Peter Zijlstra 1 sibling, 0 replies; 35+ messages in thread From: Borislav Petkov @ 2018-02-07 20:24 UTC (permalink / raw) To: Linus Torvalds Cc: Peter Zijlstra, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, Rudolf Marek On Wed, Feb 07, 2018 at 11:43:37AM -0800, Linus Torvalds wrote: > We could just also decide that the only thing that the modrm bytes of > UD0 actually *affect* is how the CPU might act for a page-crossing > instruction. > > Because I think that's the only semantic difference: if it's a > page-crosser, the instruction could take a page fault before raising > the #UD. > > Is there any other decode issue we might want to look out for? Yes, AMD doesn't do UD0 with a ModRM: "UD0 0F FF Raise an invalid opcode exception" And I don't know about the other x86 vendors; what fun they've had with the UD*. I guess we should make sure the following bytes are a valid insn. Btw Rudolf had experimented with this, CCed, and he showed that on Intel you get a SIGSEGV for a page crosser with UD1 and SIGILL on AMD. I guess you can get the same with UD0 apparently. > Anyway, then we'd make the rule be: > > - we promise to always pad up the following bytes (our extra warning > information etc) so that we never have a missing page afterwards (this > is presumably practically speaking already the case). It might still > be a page-crossing instruction, but we won't take a page fault in > kernel space (due to it being at the end of some text sectoin or > whatever that changes the NX bit or due to DEBUG_PAGEALLOC having > unmapped the next page). Yeah, I guess we can control the bytes there. > - we special-case the decoder so that we don't get this warning That's easy - the decoder should simply say to upgrade objdump. > and then we just ignore the issue entirely. > > Hmm? Sounds like a plan but with those things the devil's in the detail. Nothing sounds too nasty now, though. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 19:43 ` Linus Torvalds 2018-02-07 20:24 ` Borislav Petkov @ 2018-02-08 9:13 ` Peter Zijlstra 2018-02-08 9:35 ` Peter Zijlstra ` (3 more replies) 1 sibling, 4 replies; 35+ messages in thread From: Peter Zijlstra @ 2018-02-08 9:13 UTC (permalink / raw) To: Linus Torvalds Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 07, 2018 at 11:43:37AM -0800, Linus Torvalds wrote: > On Wed, Feb 7, 2018 at 11:28 AM, Borislav Petkov <bp@alien8.de> wrote: > > On Wed, Feb 07, 2018 at 08:14:51PM +0100, Peter Zijlstra wrote: > >> Then someone went and wrecked it. > > > > Yeah, note says UD0 didn't eat a ModRM byte on old CPUs. But then that > > changed too. Fun stuff changing insn encoding underway. > > > > So if we opt for adding a ModRM byte, could a 0x90 NOP work so that it > > doesn't shit itself on those old CPUs? > > We could just also decide that the only thing that the modrm bytes of > UD0 actually *affect* is how the CPU might act for a page-crossing > instruction. > > Because I think that's the only semantic difference: if it's a > page-crosser, the instruction could take a page fault before raising > the #UD. > > Is there any other decode issue we might want to look out for? _The_ problem is that new binutils cannot sanely decode any function that has a WARN in (this very much includes perf annotate): old: 00000000000016a0 <copy_overflow>: 16a0: 48 89 f2 mov %rsi,%rdx 16a3: 89 fe mov %edi,%esi 16a5: 48 c7 c7 00 00 00 00 mov $0x0,%rdi 16a8: R_X86_64_32S .rodata.str1.8+0x288 16ac: e8 00 00 00 00 callq 16b1 <copy_overflow+0x11> 16ad: R_X86_64_PC32 __warn_printk-0x4 16b1: 0f ff (bad) 16b3: c3 retq 16b4: 66 90 xchg %ax,%ax 16b6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 16bd: 00 00 00 new: 00000000000016a0 <copy_overflow>: 16a0: 48 89 f2 mov %rsi,%rdx 16a3: 89 fe mov %edi,%esi 16a5: 48 c7 c7 00 00 00 00 mov $0x0,%rdi 16a8: R_X86_64_32S .rodata.str1.8+0x288 16ac: e8 00 00 00 00 callq 16b1 <copy_overflow+0x11> 16ad: R_X86_64_PC32 __warn_printk-0x4 16b1: 0f ff c3 ud0 %ebx,%eax 16b4: 66 90 xchg %ax,%ax 16b6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 16bd: 00 00 00 I went through the register opcodes and matched it against the ModR/M encoding, and the best option I've found so far is using 0xd6 as the next byte. That yields: old: 0000000000001690 <copy_overflow>: 1690: 48 89 f2 mov %rsi,%rdx 1693: 89 fe mov %edi,%esi 1695: 48 c7 c7 00 00 00 00 mov $0x0,%rdi 1698: R_X86_64_32S .rodata.str1.8+0x270 169c: e8 00 00 00 00 callq 16a1 <copy_overflow+0x11> 169d: R_X86_64_PC32 __warn_printk-0x4 16a1: 0f ff (bad) 16a3: d6 (bad) 16a4: c3 retq 16a5: 90 nop 16a6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) new: 0000000000001690 <copy_overflow>: 1690: 48 89 f2 mov %rsi,%rdx 1693: 89 fe mov %edi,%esi 1695: 48 c7 c7 00 00 00 00 mov $0x0,%rdi 1698: R_X86_64_32S .rodata.str1.8+0x270 169c: e8 00 00 00 00 callq 16a1 <copy_overflow+0x11> 169d: R_X86_64_PC32 __warn_printk-0x4 16a1: 0f ff d6 ud0 %esi,%edx 16a4: c3 retq 16a5: 90 nop 16a6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 16ad: 00 00 00 And only grows a defconfig image by 91 bytes, purely for the purpose of being able to disassemble it :/ text data bss dec hex filename 17307211 4890808 1052880 23250899 162c7d3 defconfig-build/vmlinux 17307302 4890808 1052880 23250990 162c82e defconfig-build/vmlinux --- arch/x86/include/asm/bug.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index 34d99af43994..f0d5b4a1512d 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -12,16 +12,21 @@ * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas * our kernel decoder thinks it takes a ModRM byte, which seems consistent * with various things like the Intel SDM instruction encoding rules) + * + * And now someone went and taught binutils about UD0 as taking a ModR/M too + * and it generates crap when disassembling the kernel. Stick a pointless 0xD6 + * ModR/M on, which the old binutils decodes as (bad) and the new binutils sees + * as a valid single byte ModR/M. */ -#define ASM_UD0 ".byte 0x0f, 0xff" +#define ASM_UD0 ".byte 0x0f, 0xff, 0xd6" #define ASM_UD1 ".byte 0x0f, 0xb9" /* + ModRM */ #define ASM_UD2 ".byte 0x0f, 0x0b" #define INSN_UD0 0xff0f #define INSN_UD2 0x0b0f -#define LEN_UD0 2 +#define LEN_UD0 3 #ifdef CONFIG_GENERIC_BUG ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 9:13 ` Peter Zijlstra @ 2018-02-08 9:35 ` Peter Zijlstra 2018-02-08 9:46 ` Borislav Petkov ` (2 subsequent siblings) 3 siblings, 0 replies; 35+ messages in thread From: Peter Zijlstra @ 2018-02-08 9:35 UTC (permalink / raw) To: Linus Torvalds Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Thu, Feb 08, 2018 at 10:13:02AM +0100, Peter Zijlstra wrote: > _The_ problem is that new binutils cannot sanely decode any function > that has a WARN in (this very much includes perf annotate): > new: > > 00000000000016a0 <copy_overflow>: > 16a0: 48 89 f2 mov %rsi,%rdx > 16a3: 89 fe mov %edi,%esi > 16a5: 48 c7 c7 00 00 00 00 mov $0x0,%rdi > 16a8: R_X86_64_32S .rodata.str1.8+0x288 > 16ac: e8 00 00 00 00 callq 16b1 <copy_overflow+0x11> > 16ad: R_X86_64_PC32 __warn_printk-0x4 > 16b1: 0f ff c3 ud0 %ebx,%eax > 16b4: 66 90 xchg %ax,%ax > 16b6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) > 16bd: 00 00 00 And note that this example is fairly 'good' as far as things go, it only eats the single byte ret instruction and is on an instruction boundary again, so the rest is good. There are examples where what it eats results in shifting the instruction boundary and the rest is _complete_ crap. Look what it does to sched_cpu_dying: 7199: 48 39 c3 cmp %rax,%rbx 719c: 0f 84 cc 00 00 00 je 726e <sched_cpu_dying+0x1fe> 71a2: 0f ff 41 c6 ud0 -0x3a(%rcx),%eax 71a6: 87 3c 07 xchg %edi,(%rdi,%rax,1) 71a9: 00 00 add %al,(%rax) 71ab: 00 8b 43 04 83 f8 add %cl,-0x77cfbbd(%rbx) 71b1: 01 0f add %ecx,(%rdi) 71b3: 85 4f ff test %ecx,-0x1(%rdi) 71b6: ff (bad) 71b7: ff 48 8b decl -0x75(%rax) 71ba: 44 24 08 rex.R and $0x8,%al 7199: 48 39 c3 cmp %rax,%rbx 719c: 0f 84 cc 00 00 00 je 726e <sched_cpu_dying+0x1fe> 71a2: 0f ff (bad) 71a4: 41 c6 87 3c 07 00 00 movb $0x0,0x73c(%r15) 71ab: 00 71ac: 8b 43 04 mov 0x4(%rbx),%eax 71af: 83 f8 01 cmp $0x1,%eax 71b2: 0f 85 4f ff ff ff jne 7107 <sched_cpu_dying+0x97> 71b8: 48 8b 44 24 08 mov 0x8(%rsp),%rax 71bd: 48 8b 74 24 10 mov 0x10(%rsp),%rsi ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 9:13 ` Peter Zijlstra 2018-02-08 9:35 ` Peter Zijlstra @ 2018-02-08 9:46 ` Borislav Petkov 2018-02-08 9:47 ` David Laight 2018-02-08 17:27 ` Linus Torvalds 3 siblings, 0 replies; 35+ messages in thread From: Borislav Petkov @ 2018-02-08 9:46 UTC (permalink / raw) To: Peter Zijlstra Cc: Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Thu, Feb 08, 2018 at 10:13:02AM +0100, Peter Zijlstra wrote: > diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h > index 34d99af43994..f0d5b4a1512d 100644 > --- a/arch/x86/include/asm/bug.h > +++ b/arch/x86/include/asm/bug.h > @@ -12,16 +12,21 @@ > * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas > * our kernel decoder thinks it takes a ModRM byte, which seems consistent > * with various things like the Intel SDM instruction encoding rules) > + * > + * And now someone went and taught binutils about UD0 as taking a ModR/M too > + * and it generates crap when disassembling the kernel. Stick a pointless 0xD6 > + * ModR/M on, which the old binutils decodes as (bad) and the new binutils sees > + * as a valid single byte ModR/M. > */ > > -#define ASM_UD0 ".byte 0x0f, 0xff" > +#define ASM_UD0 ".byte 0x0f, 0xff, 0xd6" Yeah, we probably should say that it was SALC: http://ref.x86asm.net/geek.html#xD6 but undefined after P4. Not that it matters a whole lot after an UD0. And it still won't matter if it gets redefined to some new fancy prefix. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 35+ messages in thread
* RE: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 9:13 ` Peter Zijlstra 2018-02-08 9:35 ` Peter Zijlstra 2018-02-08 9:46 ` Borislav Petkov @ 2018-02-08 9:47 ` David Laight 2018-02-08 10:13 ` Peter Zijlstra 2018-02-08 17:27 ` Linus Torvalds 3 siblings, 1 reply; 35+ messages in thread From: David Laight @ 2018-02-08 9:47 UTC (permalink / raw) To: 'Peter Zijlstra', Linus Torvalds Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers From: Peter Zijlstra > Sent: 08 February 2018 09:13 ... > > > Yeah, note says UD0 didn't eat a ModRM byte on old CPUs. But then that > > > changed too. Fun stuff changing insn encoding underway. > > > > > > So if we opt for adding a ModRM byte, could a 0x90 NOP work so that it > > > doesn't shit itself on those old CPUs? > > > > We could just also decide that the only thing that the modrm bytes of > > UD0 actually *affect* is how the CPU might act for a page-crossing > > instruction. > > > > Because I think that's the only semantic difference: if it's a > > page-crosser, the instruction could take a page fault before raising > > the #UD. > > > > Is there any other decode issue we might want to look out for? > > _The_ problem is that new binutils cannot sanely decode any function > that has a WARN in (this very much includes perf annotate): > > old: > > 00000000000016a0 <copy_overflow>: > 16a0: 48 89 f2 mov %rsi,%rdx > 16a3: 89 fe mov %edi,%esi > 16a5: 48 c7 c7 00 00 00 00 mov $0x0,%rdi > 16a8: R_X86_64_32S .rodata.str1.8+0x288 > 16ac: e8 00 00 00 00 callq 16b1 <copy_overflow+0x11> > 16ad: R_X86_64_PC32 __warn_printk-0x4 > 16b1: 0f ff (bad) > 16b3: c3 retq > 16b4: 66 90 xchg %ax,%ax > 16b6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) > 16bd: 00 00 00 > > new: > > 00000000000016a0 <copy_overflow>: > 16a0: 48 89 f2 mov %rsi,%rdx > 16a3: 89 fe mov %edi,%esi > 16a5: 48 c7 c7 00 00 00 00 mov $0x0,%rdi > 16a8: R_X86_64_32S .rodata.str1.8+0x288 > 16ac: e8 00 00 00 00 callq 16b1 <copy_overflow+0x11> > 16ad: R_X86_64_PC32 __warn_printk-0x4 > 16b1: 0f ff c3 ud0 %ebx,%eax > 16b4: 66 90 xchg %ax,%ax > 16b6: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) > 16bd: 00 00 00 > > > I went through the register opcodes and matched it against the ModR/M > encoding, and the best option I've found so far is using 0xd6 as the > next byte. Wouldn't 0xc3 work as well. A retq is probably better than an extra (bad). Actually objdump ought to be more explicit than (bad) for the explicit UD0/1 David ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 9:47 ` David Laight @ 2018-02-08 10:13 ` Peter Zijlstra 0 siblings, 0 replies; 35+ messages in thread From: Peter Zijlstra @ 2018-02-08 10:13 UTC (permalink / raw) To: David Laight Cc: Linus Torvalds, Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Thu, Feb 08, 2018 at 09:47:53AM +0000, David Laight wrote: > From: Peter Zijlstra > > I went through the register opcodes and matched it against the ModR/M > > encoding, and the best option I've found so far is using 0xd6 as the > > next byte. > > Wouldn't 0xc3 work as well. > A retq is probably better than an extra (bad). I explicity did not use c3 because that generates spurious "retq" in the code flow which is mighty confusing when you read the stuff. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 9:13 ` Peter Zijlstra ` (2 preceding siblings ...) 2018-02-08 9:47 ` David Laight @ 2018-02-08 17:27 ` Linus Torvalds 2018-02-08 18:03 ` Peter Zijlstra 3 siblings, 1 reply; 35+ messages in thread From: Linus Torvalds @ 2018-02-08 17:27 UTC (permalink / raw) To: Peter Zijlstra Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Thu, Feb 8, 2018 at 1:13 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > _The_ problem is that new binutils cannot sanely decode any function > that has a WARN in (this very much includes perf annotate): Ugh. Is there any reason why we don't just use UD2 and avoid this whole issue? Do we even *do* anything different for UD0 vs UD2? I see that 'is_valid_bugaddr()' seems to accept either, I'm not seeing anything that cares. Yes, yes, we currently use UD0 for warnings, and UD2 for BUG(), but the fact is, we don't actually seem to care, because the *real* differentiator is BUGFLAG_WARNING. No? So all the UD0 games seem a bit pointless. Let's not use it at all because it's so badly documented. Linus ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 17:27 ` Linus Torvalds @ 2018-02-08 18:03 ` Peter Zijlstra 2018-02-08 18:15 ` Linus Torvalds 0 siblings, 1 reply; 35+ messages in thread From: Peter Zijlstra @ 2018-02-08 18:03 UTC (permalink / raw) To: Linus Torvalds Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, Arjan van de Ven On Thu, Feb 08, 2018 at 09:27:56AM -0800, Linus Torvalds wrote: > On Thu, Feb 8, 2018 at 1:13 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > > > _The_ problem is that new binutils cannot sanely decode any function > > that has a WARN in (this very much includes perf annotate): > > Ugh. > > Is there any reason why we don't just use UD2 and avoid this whole issue? That's what I initially had, but then Arjan told me that a number of emulators fatally terminate on UD2 since they assume it to be BUG. Which is when we all went looking at UD1/UD0. But yes, for our purposes UD2 is perfectly fine too, it will just mess up the people doing bringup and the like. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 18:03 ` Peter Zijlstra @ 2018-02-08 18:15 ` Linus Torvalds 2018-02-08 19:44 ` Peter Zijlstra 0 siblings, 1 reply; 35+ messages in thread From: Linus Torvalds @ 2018-02-08 18:15 UTC (permalink / raw) To: Peter Zijlstra Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, Arjan van de Ven On Thu, Feb 8, 2018 at 10:03 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > But yes, for our purposes UD2 is perfectly fine too, it will just mess > up the people doing bringup and the like. Oh, we'll inconvenience people inside Intel? The same people who changed and screwed up the definition of UD0 just a couple of months ago? I'm *so* sorry for the inconvenience to those people /s. F*ck them. They introduced the problem, they get to live with it. Linus ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 18:15 ` Linus Torvalds @ 2018-02-08 19:44 ` Peter Zijlstra 2018-02-08 20:02 ` Linus Torvalds ` (3 more replies) 0 siblings, 4 replies; 35+ messages in thread From: Peter Zijlstra @ 2018-02-08 19:44 UTC (permalink / raw) To: Linus Torvalds Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, Arjan van de Ven On Thu, Feb 08, 2018 at 10:15:30AM -0800, Linus Torvalds wrote: > On Thu, Feb 8, 2018 at 10:03 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > > > But yes, for our purposes UD2 is perfectly fine too, it will just mess > > up the people doing bringup and the like. > > Oh, we'll inconvenience people inside Intel? > > The same people who changed and screwed up the definition of UD0 just > a couple of months ago? > > I'm *so* sorry for the inconvenience to those people /s. > > F*ck them. They introduced the problem, they get to live with it. OK, here's the patch.. It leaves the UD0 in traps.c such that people could recompile their kernel with a simple change. --- Subject: x86: Use UD2 for WARN Since the Intel SDM added an ModR/M byte to UD0 and binutils followed that specification, we now cannot disassemble our kernel anymore. This now mean Intel and AMD disagree on the encoding of UD0. And instead of playing games with additional bytes that are valid ModR/M and single byte instructions (0xd6 for instance), simply use UD2 for both WARN and BUG. Requested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/x86/include/asm/bug.h | 15 ++++++--------- arch/x86/kernel/traps.c | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index 34d99af43994..2ea100e7a60f 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -5,23 +5,20 @@ #include <linux/stringify.h> /* - * Since some emulators terminate on UD2, we cannot use it for WARN. - * Since various instruction decoders disagree on the length of UD1, - * we cannot use it either. So use UD0 for WARN. + * Despite that some emulators terminate on UD2, we use it for WARN. * - * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas - * our kernel decoder thinks it takes a ModRM byte, which seems consistent - * with various things like the Intel SDM instruction encoding rules) + * Since various instruction decoders/specs disagree on the encoding of + * UD0/UD1, */ -#define ASM_UD0 ".byte 0x0f, 0xff" +#define ASM_UD0 ".byte 0x0f, 0xff" /* + ModRM (for Intel) */ #define ASM_UD1 ".byte 0x0f, 0xb9" /* + ModRM */ #define ASM_UD2 ".byte 0x0f, 0x0b" #define INSN_UD0 0xff0f #define INSN_UD2 0x0b0f -#define LEN_UD0 2 +#define LEN_UD2 2 #ifdef CONFIG_GENERIC_BUG @@ -77,7 +74,7 @@ do { \ unreachable(); \ } while (0) -#define __WARN_FLAGS(flags) _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)) +#define __WARN_FLAGS(flags) _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags)) #include <asm-generic/bug.h> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 446c9ef8cfc3..3d9b2308e7fa 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -181,7 +181,7 @@ int fixup_bug(struct pt_regs *regs, int trapnr) break; case BUG_TRAP_TYPE_WARN: - regs->ip += LEN_UD0; + regs->ip += LEN_UD2; return 1; } ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 19:44 ` Peter Zijlstra @ 2018-02-08 20:02 ` Linus Torvalds 2018-02-08 20:31 ` Borislav Petkov ` (2 subsequent siblings) 3 siblings, 0 replies; 35+ messages in thread From: Linus Torvalds @ 2018-02-08 20:02 UTC (permalink / raw) To: Peter Zijlstra Cc: Borislav Petkov, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, Arjan van de Ven On Thu, Feb 8, 2018 at 11:44 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > OK, here's the patch.. It leaves the UD0 in traps.c such that people > could recompile their kernel with a simple change. Ack, that seems about the best we can do. Linus ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 19:44 ` Peter Zijlstra 2018-02-08 20:02 ` Linus Torvalds @ 2018-02-08 20:31 ` Borislav Petkov 2018-02-08 23:09 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Josh Poimboeuf 2018-02-09 8:12 ` [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f Peter Zijlstra 2018-02-13 11:30 ` [tip:x86/pti] x86/debug: Use UD2 for WARN() tip-bot for Peter Zijlstra 2018-02-15 0:27 ` tip-bot for Peter Zijlstra 3 siblings, 2 replies; 35+ messages in thread From: Borislav Petkov @ 2018-02-08 20:31 UTC (permalink / raw) To: Peter Zijlstra Cc: Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, Arjan van de Ven On Thu, Feb 08, 2018 at 08:44:06PM +0100, Peter Zijlstra wrote: > /* > - * Since some emulators terminate on UD2, we cannot use it for WARN. > - * Since various instruction decoders disagree on the length of UD1, > - * we cannot use it either. So use UD0 for WARN. > + * Despite that some emulators terminate on UD2, we use it for WARN. > * > - * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas > - * our kernel decoder thinks it takes a ModRM byte, which seems consistent > - * with various things like the Intel SDM instruction encoding rules) > + * Since various instruction decoders/specs disagree on the encoding of > + * UD0/UD1, ^ Does that comma mean the sentence continues somewhere? > */ > > -#define ASM_UD0 ".byte 0x0f, 0xff" > +#define ASM_UD0 ".byte 0x0f, 0xff" /* + ModRM (for Intel) */ > #define ASM_UD1 ".byte 0x0f, 0xb9" /* + ModRM */ > #define ASM_UD2 ".byte 0x0f, 0x0b" > > #define INSN_UD0 0xff0f > #define INSN_UD2 0x0b0f > > -#define LEN_UD0 2 > +#define LEN_UD2 2 > > #ifdef CONFIG_GENERIC_BUG > > @@ -77,7 +74,7 @@ do { \ > unreachable(); \ > } while (0) > > -#define __WARN_FLAGS(flags) _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)) > +#define __WARN_FLAGS(flags) _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags)) > > #include <asm-generic/bug.h> > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > index 446c9ef8cfc3..3d9b2308e7fa 100644 > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -181,7 +181,7 @@ int fixup_bug(struct pt_regs *regs, int trapnr) > break; > > case BUG_TRAP_TYPE_WARN: > - regs->ip += LEN_UD0; > + regs->ip += LEN_UD2; > return 1; So patch looks ok but it broke objtool: init/main.o: warning: objtool: do_one_initcall()+0x13b: unreachable instruction /bin/sh: line 1: 25800 Segmentation fault ./tools/objtool/objtool orc generate --no-fp "arch/x86/kernel/process_64.o" make[2]: *** [arch/x86/kernel/process_64.o] Error 139 make[1]: *** [arch/x86/kernel] Error 2 make[1]: *** Waiting for unfinished jobs.... arch/x86/entry/common.o: warning: objtool: prepare_exit_to_usermode()+0x7b: unreachable instruction arch/x86/events/core.o: warning: objtool: perf_assign_events()+0x24e: unreachable instruction kernel/fork.o: warning: objtool: free_task()+0x4b: unreachable instruction arch/x86/events/amd/core.o: warning: objtool: amd_pmu_cpu_starting()+0xcc: unreachable instruction arch/x86/events/amd/ibs.o: warning: objtool: perf_ibs_start()+0x10e: unreachable instruction arch/x86/events/amd/iommu.o: warning: objtool: perf_iommu_start()+0x1c0: unreachable instruction arch/x86/events/amd/power.o: warning: objtool: __pmu_event_start()+0x8e: unreachable instruction ... -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch 2018-02-08 20:31 ` Borislav Petkov @ 2018-02-08 23:09 ` Josh Poimboeuf 2018-02-08 23:09 ` [PATCH 1/2] objtool: Fix seg fault in ignore_unreachable_insn() Josh Poimboeuf ` (2 more replies) 2018-02-09 8:12 ` [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f Peter Zijlstra 1 sibling, 3 replies; 35+ messages in thread From: Josh Poimboeuf @ 2018-02-08 23:09 UTC (permalink / raw) To: x86 Cc: Borislav Petkov, Peter Zijlstra, Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, linux-kernel, Arjan van de Ven Josh Poimboeuf (2): objtool: Fix seg fault in ignore_unreachable_insn() x86: Annotate WARN-related UD2 as reachable arch/x86/include/asm/bug.h | 6 +++++- tools/objtool/check.c | 12 +++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) -- 2.14.3 ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/2] objtool: Fix seg fault in ignore_unreachable_insn() 2018-02-08 23:09 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Josh Poimboeuf @ 2018-02-08 23:09 ` Josh Poimboeuf 2018-02-13 11:29 ` [tip:x86/pti] objtool: Fix segfault " tip-bot for Josh Poimboeuf 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 2018-02-08 23:09 ` [PATCH 2/2] x86: Annotate WARN-related UD2 as reachable Josh Poimboeuf 2018-02-09 8:13 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Peter Zijlstra 2 siblings, 2 replies; 35+ messages in thread From: Josh Poimboeuf @ 2018-02-08 23:09 UTC (permalink / raw) To: x86 Cc: Borislav Petkov, Peter Zijlstra, Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, linux-kernel, Arjan van de Ven Peter Zijlstra's patch for converting WARN() to use UD2 triggered a bunch of false "unreachable instruction" warnings, which then triggered a seg fault in ignore_unreachable_insn(). The seg fault happened when it tried to dereference a NULL 'insn->func' pointer. Thanks to static_cpu_has(), some functions can jump to a non-function area in the .altinstr_aux section. That breaks ignore_unreachable_insn()'s assumption that it's always inside the original function. Make sure ignore_unreachable_insn() only follows jumps within the current function. Reported-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- tools/objtool/check.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index b00b1896547e..4ec1a3bf7bf2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1899,13 +1899,19 @@ static bool ignore_unreachable_insn(struct instruction *insn) if (is_kasan_insn(insn) || is_ubsan_insn(insn)) return true; - if (insn->type == INSN_JUMP_UNCONDITIONAL && insn->jump_dest) { - insn = insn->jump_dest; - continue; + if (insn->type == INSN_JUMP_UNCONDITIONAL) { + if (insn->jump_dest && + insn->jump_dest->func == insn->func) { + insn = insn->jump_dest; + continue; + } + + break; } if (insn->offset + insn->len >= insn->func->offset + insn->func->len) break; + insn = list_next_entry(insn, list); } -- 2.14.3 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [tip:x86/pti] objtool: Fix segfault in ignore_unreachable_insn() 2018-02-08 23:09 ` [PATCH 1/2] objtool: Fix seg fault in ignore_unreachable_insn() Josh Poimboeuf @ 2018-02-13 11:29 ` tip-bot for Josh Poimboeuf 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 1 sibling, 0 replies; 35+ messages in thread From: tip-bot for Josh Poimboeuf @ 2018-02-13 11:29 UTC (permalink / raw) To: linux-tip-commits Cc: brgerst, tglx, bp, torvalds, jpoimboe, luto, arjan, fengguang.wu, peterz, mingo, hpa, dvlasenk, linux-kernel Commit-ID: 59d13cd6789e8feb4615bb999d46bbd439a9a396 Gitweb: https://git.kernel.org/tip/59d13cd6789e8feb4615bb999d46bbd439a9a396 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Thu, 8 Feb 2018 17:09:25 -0600 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 13 Feb 2018 11:46:59 +0100 objtool: Fix segfault in ignore_unreachable_insn() Peter Zijlstra's patch for converting WARN() to use UD2 triggered a bunch of false "unreachable instruction" warnings, which then triggered a seg fault in ignore_unreachable_insn(). The seg fault happened when it tried to dereference a NULL 'insn->func' pointer. Thanks to static_cpu_has(), some functions can jump to a non-function area in the .altinstr_aux section. That breaks ignore_unreachable_insn()'s assumption that it's always inside the original function. Make sure ignore_unreachable_insn() only follows jumps within the current function. Reported-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Arjan van de Ven <arjan@linux.intel.com> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: kbuild test robot <fengguang.wu@intel.com> Link: http://lkml.kernel.org/r/bace77a60d5af9b45eddb8f8fb9c776c8de657ef.1518130694.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- tools/objtool/check.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 2e458eb..c7fb5c2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1935,13 +1935,19 @@ static bool ignore_unreachable_insn(struct instruction *insn) if (is_kasan_insn(insn) || is_ubsan_insn(insn)) return true; - if (insn->type == INSN_JUMP_UNCONDITIONAL && insn->jump_dest) { - insn = insn->jump_dest; - continue; + if (insn->type == INSN_JUMP_UNCONDITIONAL) { + if (insn->jump_dest && + insn->jump_dest->func == insn->func) { + insn = insn->jump_dest; + continue; + } + + break; } if (insn->offset + insn->len >= insn->func->offset + insn->func->len) break; + insn = list_next_entry(insn, list); } ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [tip:x86/pti] objtool: Fix segfault in ignore_unreachable_insn() 2018-02-08 23:09 ` [PATCH 1/2] objtool: Fix seg fault in ignore_unreachable_insn() Josh Poimboeuf 2018-02-13 11:29 ` [tip:x86/pti] objtool: Fix segfault " tip-bot for Josh Poimboeuf @ 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 1 sibling, 0 replies; 35+ messages in thread From: tip-bot for Josh Poimboeuf @ 2018-02-15 0:26 UTC (permalink / raw) To: linux-tip-commits Cc: peterz, linux-kernel, torvalds, mingo, luto, dvlasenk, arjan, jpoimboe, brgerst, bp, hpa, fengguang.wu, tglx Commit-ID: fe24e27128252c230a34a6c628da2bf1676781ea Gitweb: https://git.kernel.org/tip/fe24e27128252c230a34a6c628da2bf1676781ea Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Thu, 8 Feb 2018 17:09:25 -0600 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 15 Feb 2018 01:15:49 +0100 objtool: Fix segfault in ignore_unreachable_insn() Peter Zijlstra's patch for converting WARN() to use UD2 triggered a bunch of false "unreachable instruction" warnings, which then triggered a seg fault in ignore_unreachable_insn(). The seg fault happened when it tried to dereference a NULL 'insn->func' pointer. Thanks to static_cpu_has(), some functions can jump to a non-function area in the .altinstr_aux section. That breaks ignore_unreachable_insn()'s assumption that it's always inside the original function. Make sure ignore_unreachable_insn() only follows jumps within the current function. Reported-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Arjan van de Ven <arjan@linux.intel.com> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: kbuild test robot <fengguang.wu@intel.com> Link: http://lkml.kernel.org/r/bace77a60d5af9b45eddb8f8fb9c776c8de657ef.1518130694.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- tools/objtool/check.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 2e458eb..c7fb5c2 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1935,13 +1935,19 @@ static bool ignore_unreachable_insn(struct instruction *insn) if (is_kasan_insn(insn) || is_ubsan_insn(insn)) return true; - if (insn->type == INSN_JUMP_UNCONDITIONAL && insn->jump_dest) { - insn = insn->jump_dest; - continue; + if (insn->type == INSN_JUMP_UNCONDITIONAL) { + if (insn->jump_dest && + insn->jump_dest->func == insn->func) { + insn = insn->jump_dest; + continue; + } + + break; } if (insn->offset + insn->len >= insn->func->offset + insn->func->len) break; + insn = list_next_entry(insn, list); } ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 2/2] x86: Annotate WARN-related UD2 as reachable 2018-02-08 23:09 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Josh Poimboeuf 2018-02-08 23:09 ` [PATCH 1/2] objtool: Fix seg fault in ignore_unreachable_insn() Josh Poimboeuf @ 2018-02-08 23:09 ` Josh Poimboeuf 2018-02-13 11:30 ` [tip:x86/pti] x86/debug, objtool: Annotate WARN()-related " tip-bot for Josh Poimboeuf 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 2018-02-09 8:13 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Peter Zijlstra 2 siblings, 2 replies; 35+ messages in thread From: Josh Poimboeuf @ 2018-02-08 23:09 UTC (permalink / raw) To: x86 Cc: Borislav Petkov, Peter Zijlstra, Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, linux-kernel, Arjan van de Ven By default, objtool assumes that a UD2 is a dead end. This is mainly because GCC 7+ sometimes inserts a UD2 when it detects a divide-by-zero condition. Now that WARN() is moving back to UD2, annotate the code after it as reachable so objtool can follow the code flow. Reported-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- arch/x86/include/asm/bug.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index f2da5fc90e5c..697d89d6fa76 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -74,7 +74,11 @@ do { \ unreachable(); \ } while (0) -#define __WARN_FLAGS(flags) _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags)) +#define __WARN_FLAGS(flags) \ +do { \ + _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags)); \ + annotate_reachable(); \ +} while (0) #include <asm-generic/bug.h> -- 2.14.3 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [tip:x86/pti] x86/debug, objtool: Annotate WARN()-related UD2 as reachable 2018-02-08 23:09 ` [PATCH 2/2] x86: Annotate WARN-related UD2 as reachable Josh Poimboeuf @ 2018-02-13 11:30 ` tip-bot for Josh Poimboeuf 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 1 sibling, 0 replies; 35+ messages in thread From: tip-bot for Josh Poimboeuf @ 2018-02-13 11:30 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, bp, linux-kernel, jpoimboe, peterz, brgerst, dvlasenk, torvalds, tglx, hpa, fengguang.wu, arjan, luto Commit-ID: 30dde5167c432f8e32d3d13fe73a241578a4f259 Gitweb: https://git.kernel.org/tip/30dde5167c432f8e32d3d13fe73a241578a4f259 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Thu, 8 Feb 2018 17:09:26 -0600 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 13 Feb 2018 11:47:00 +0100 x86/debug, objtool: Annotate WARN()-related UD2 as reachable By default, objtool assumes that a UD2 is a dead end. This is mainly because GCC 7+ sometimes inserts a UD2 when it detects a divide-by-zero condition. Now that WARN() is moving back to UD2, annotate the code after it as reachable so objtool can follow the code flow. Reported-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Arjan van de Ven <arjan@linux.intel.com> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: kbuild test robot <fengguang.wu@intel.com> Link: http://lkml.kernel.org/r/0e483379275a42626ba8898117f918e1bf661e40.1518130694.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/bug.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index 34d99af4..71e6f4bf 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -77,7 +77,11 @@ do { \ unreachable(); \ } while (0) -#define __WARN_FLAGS(flags) _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)) +#define __WARN_FLAGS(flags) \ +do { \ + _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)); \ + annotate_reachable(); \ +} while (0) #include <asm-generic/bug.h> ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [tip:x86/pti] x86/debug, objtool: Annotate WARN()-related UD2 as reachable 2018-02-08 23:09 ` [PATCH 2/2] x86: Annotate WARN-related UD2 as reachable Josh Poimboeuf 2018-02-13 11:30 ` [tip:x86/pti] x86/debug, objtool: Annotate WARN()-related " tip-bot for Josh Poimboeuf @ 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 1 sibling, 0 replies; 35+ messages in thread From: tip-bot for Josh Poimboeuf @ 2018-02-15 0:26 UTC (permalink / raw) To: linux-tip-commits Cc: luto, torvalds, mingo, tglx, hpa, dvlasenk, linux-kernel, bp, peterz, arjan, brgerst, fengguang.wu, jpoimboe Commit-ID: 2b5db66862b95532cb6cca8165ae6eb73633cf85 Gitweb: https://git.kernel.org/tip/2b5db66862b95532cb6cca8165ae6eb73633cf85 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Thu, 8 Feb 2018 17:09:26 -0600 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 15 Feb 2018 01:15:49 +0100 x86/debug, objtool: Annotate WARN()-related UD2 as reachable By default, objtool assumes that a UD2 is a dead end. This is mainly because GCC 7+ sometimes inserts a UD2 when it detects a divide-by-zero condition. Now that WARN() is moving back to UD2, annotate the code after it as reachable so objtool can follow the code flow. Reported-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Arjan van de Ven <arjan@linux.intel.com> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: kbuild test robot <fengguang.wu@intel.com> Link: http://lkml.kernel.org/r/0e483379275a42626ba8898117f918e1bf661e40.1518130694.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/bug.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index 34d99af4..71e6f4bf 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -77,7 +77,11 @@ do { \ unreachable(); \ } while (0) -#define __WARN_FLAGS(flags) _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)) +#define __WARN_FLAGS(flags) \ +do { \ + _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)); \ + annotate_reachable(); \ +} while (0) #include <asm-generic/bug.h> ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch 2018-02-08 23:09 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Josh Poimboeuf 2018-02-08 23:09 ` [PATCH 1/2] objtool: Fix seg fault in ignore_unreachable_insn() Josh Poimboeuf 2018-02-08 23:09 ` [PATCH 2/2] x86: Annotate WARN-related UD2 as reachable Josh Poimboeuf @ 2018-02-09 8:13 ` Peter Zijlstra 2 siblings, 0 replies; 35+ messages in thread From: Peter Zijlstra @ 2018-02-09 8:13 UTC (permalink / raw) To: Josh Poimboeuf Cc: x86, Borislav Petkov, Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, linux-kernel, Arjan van de Ven On Thu, Feb 08, 2018 at 05:09:24PM -0600, Josh Poimboeuf wrote: > Josh Poimboeuf (2): > objtool: Fix seg fault in ignore_unreachable_insn() > x86: Annotate WARN-related UD2 as reachable Ah, thanks for saving me the trouble of digging these out of history. I did remember you did them back then, now I don't even have to go find them ;-) ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-08 20:31 ` Borislav Petkov 2018-02-08 23:09 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Josh Poimboeuf @ 2018-02-09 8:12 ` Peter Zijlstra 1 sibling, 0 replies; 35+ messages in thread From: Peter Zijlstra @ 2018-02-09 8:12 UTC (permalink / raw) To: Borislav Petkov Cc: Linus Torvalds, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, Arjan van de Ven On Thu, Feb 08, 2018 at 09:31:46PM +0100, Borislav Petkov wrote: > On Thu, Feb 08, 2018 at 08:44:06PM +0100, Peter Zijlstra wrote: > > + * Since various instruction decoders/specs disagree on the encoding of > > + * UD0/UD1, > ^ > > Does that comma mean the sentence continues somewhere? No that's what doing patches while baking cup-cakes with my oldest daughter gets me :-) ^ permalink raw reply [flat|nested] 35+ messages in thread
* [tip:x86/pti] x86/debug: Use UD2 for WARN() 2018-02-08 19:44 ` Peter Zijlstra 2018-02-08 20:02 ` Linus Torvalds 2018-02-08 20:31 ` Borislav Petkov @ 2018-02-13 11:30 ` tip-bot for Peter Zijlstra 2018-02-15 0:27 ` tip-bot for Peter Zijlstra 3 siblings, 0 replies; 35+ messages in thread From: tip-bot for Peter Zijlstra @ 2018-02-13 11:30 UTC (permalink / raw) To: linux-tip-commits Cc: brgerst, peterz, torvalds, jpoimboe, luto, hpa, bp, tglx, mingo, arjan, dvlasenk, linux-kernel Commit-ID: 0eb55de80e26f87cc26eab139c097eb5a6f2b159 Gitweb: https://git.kernel.org/tip/0eb55de80e26f87cc26eab139c097eb5a6f2b159 Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Fri, 9 Feb 2018 13:16:59 +0100 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 13 Feb 2018 11:47:00 +0100 x86/debug: Use UD2 for WARN() Since the Intel SDM added an ModR/M byte to UD0 and binutils followed that specification, we now cannot disassemble our kernel anymore. This now means Intel and AMD disagree on the encoding of UD0. And instead of playing games with additional bytes that are valid ModR/M and single byte instructions (0xd6 for instance), simply use UD2 for both WARN() and BUG(). Requested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Arjan van de Ven <arjan@linux.intel.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/20180208194406.GD25181@hirez.programming.kicks-ass.net Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/bug.h | 15 ++++++--------- arch/x86/kernel/traps.c | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index 71e6f4bf..6804d66 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -5,23 +5,20 @@ #include <linux/stringify.h> /* - * Since some emulators terminate on UD2, we cannot use it for WARN. - * Since various instruction decoders disagree on the length of UD1, - * we cannot use it either. So use UD0 for WARN. + * Despite that some emulators terminate on UD2, we use it for WARN(). * - * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas - * our kernel decoder thinks it takes a ModRM byte, which seems consistent - * with various things like the Intel SDM instruction encoding rules) + * Since various instruction decoders/specs disagree on the encoding of + * UD0/UD1. */ -#define ASM_UD0 ".byte 0x0f, 0xff" +#define ASM_UD0 ".byte 0x0f, 0xff" /* + ModRM (for Intel) */ #define ASM_UD1 ".byte 0x0f, 0xb9" /* + ModRM */ #define ASM_UD2 ".byte 0x0f, 0x0b" #define INSN_UD0 0xff0f #define INSN_UD2 0x0b0f -#define LEN_UD0 2 +#define LEN_UD2 2 #ifdef CONFIG_GENERIC_BUG @@ -79,7 +76,7 @@ do { \ #define __WARN_FLAGS(flags) \ do { \ - _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)); \ + _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags)); \ annotate_reachable(); \ } while (0) diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 446c9ef..3d9b230 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -181,7 +181,7 @@ int fixup_bug(struct pt_regs *regs, int trapnr) break; case BUG_TRAP_TYPE_WARN: - regs->ip += LEN_UD0; + regs->ip += LEN_UD2; return 1; } ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [tip:x86/pti] x86/debug: Use UD2 for WARN() 2018-02-08 19:44 ` Peter Zijlstra ` (2 preceding siblings ...) 2018-02-13 11:30 ` [tip:x86/pti] x86/debug: Use UD2 for WARN() tip-bot for Peter Zijlstra @ 2018-02-15 0:27 ` tip-bot for Peter Zijlstra 3 siblings, 0 replies; 35+ messages in thread From: tip-bot for Peter Zijlstra @ 2018-02-15 0:27 UTC (permalink / raw) To: linux-tip-commits Cc: mingo, tglx, linux-kernel, brgerst, luto, dvlasenk, arjan, hpa, bp, peterz, jpoimboe, torvalds Commit-ID: 3b3a371cc9bc980429baabe0a8e5f307f3d1f463 Gitweb: https://git.kernel.org/tip/3b3a371cc9bc980429baabe0a8e5f307f3d1f463 Author: Peter Zijlstra <peterz@infradead.org> AuthorDate: Fri, 9 Feb 2018 13:16:59 +0100 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Thu, 15 Feb 2018 01:15:50 +0100 x86/debug: Use UD2 for WARN() Since the Intel SDM added an ModR/M byte to UD0 and binutils followed that specification, we now cannot disassemble our kernel anymore. This now means Intel and AMD disagree on the encoding of UD0. And instead of playing games with additional bytes that are valid ModR/M and single byte instructions (0xd6 for instance), simply use UD2 for both WARN() and BUG(). Requested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Arjan van de Ven <arjan@linux.intel.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/20180208194406.GD25181@hirez.programming.kicks-ass.net Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/bug.h | 15 ++++++--------- arch/x86/kernel/traps.c | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index 71e6f4bf..6804d66 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -5,23 +5,20 @@ #include <linux/stringify.h> /* - * Since some emulators terminate on UD2, we cannot use it for WARN. - * Since various instruction decoders disagree on the length of UD1, - * we cannot use it either. So use UD0 for WARN. + * Despite that some emulators terminate on UD2, we use it for WARN(). * - * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas - * our kernel decoder thinks it takes a ModRM byte, which seems consistent - * with various things like the Intel SDM instruction encoding rules) + * Since various instruction decoders/specs disagree on the encoding of + * UD0/UD1. */ -#define ASM_UD0 ".byte 0x0f, 0xff" +#define ASM_UD0 ".byte 0x0f, 0xff" /* + ModRM (for Intel) */ #define ASM_UD1 ".byte 0x0f, 0xb9" /* + ModRM */ #define ASM_UD2 ".byte 0x0f, 0x0b" #define INSN_UD0 0xff0f #define INSN_UD2 0x0b0f -#define LEN_UD0 2 +#define LEN_UD2 2 #ifdef CONFIG_GENERIC_BUG @@ -79,7 +76,7 @@ do { \ #define __WARN_FLAGS(flags) \ do { \ - _BUG_FLAGS(ASM_UD0, BUGFLAG_WARNING|(flags)); \ + _BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags)); \ annotate_reachable(); \ } while (0) diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 446c9ef..3d9b230 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -181,7 +181,7 @@ int fixup_bug(struct pt_regs *regs, int trapnr) break; case BUG_TRAP_TYPE_WARN: - regs->ip += LEN_UD0; + regs->ip += LEN_UD2; return 1; } ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 18:13 ` Linus Torvalds 2018-02-07 18:35 ` Borislav Petkov @ 2018-02-07 18:38 ` Randy Dunlap 2018-02-07 19:01 ` Linus Torvalds 1 sibling, 1 reply; 35+ messages in thread From: Randy Dunlap @ 2018-02-07 18:38 UTC (permalink / raw) To: Linus Torvalds, kbuild test robot, Peter Zijlstra, Ingo Molnar, Thomas Gleixner Cc: LKML, the arch/x86 maintainers On 02/07/2018 10:13 AM, Linus Torvalds wrote: > Adding more people for this funky warning from the kbuild robot. > > Something is confused. UD0 is 0f ff, the bytes after that shouldn't > matter. But I guess they can be interpreted as modrm bytes, and > somebody started doing that. > > That said, intel only _documents_ UD2 (0f 0b). Intel Order Number: 325383-064US, October 2017, documents UD0, UD1, and UD2. Section A.2.5, Table A-1, says: Use the 0F0B opcode (UD2 instruction), the 0FB9H opcode (UD1 instruction), or the 0FFFH opcode (UD0 instruction) when deliberately trying to generate an invalid opcode exception (#UD). Could this regression be a result of: commit f5b5fab1780c98b74526dbac527574bd02dc16f8 Author: Randy Dunlap <rdunlap@infradead.org> Date: Mon Dec 11 10:38:36 2017 -0800 x86/decoder: Fix and update the opcodes map Update x86-opcode-map.txt based on the October 2017 Intel SDM publication. Fix INVPID to INVVPID. Add UD0 and UD1 instruction opcodes. > Maybe we should avoid using UD0/UD1 entirely. > > Linus > > On Wed, Feb 7, 2018 at 3:44 AM, kbuild test robot > <fengguang.wu@intel.com> wrote: >> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master >> branch HEAD: a2e5790d841658485d642196dbb0927303d6c22f Merge branch 'akpm' (patches from Andrew) >> >> Regressions in current branch: >> >> arch/x86/tools/insn_decoder_test: warning: ffffffff810010d7: 0f ff eb ud0 %ebx,%ebp >> arch/x86/tools/insn_decoder_test: warning: ffffffff81001152: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi >> arch/x86/tools/insn_decoder_test: warning: ffffffff81001275: 0f ff eb ud0 %ebx,%ebp >> arch/x86/tools/insn_decoder_test: warning: ffffffff810013fd: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi >> arch/x86/tools/insn_decoder_test: warning: ffffffff81001499: 0f ff bf 09 00 00 00 ud0 0x9(%rdi),%edi >> arch/x86/tools/insn_decoder_test: warning: ffffffff812296eb: 0f ff e8 ud0 %eax,%ebp -- ~Randy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 18:38 ` [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f Randy Dunlap @ 2018-02-07 19:01 ` Linus Torvalds 2018-02-07 19:06 ` Peter Zijlstra 0 siblings, 1 reply; 35+ messages in thread From: Linus Torvalds @ 2018-02-07 19:01 UTC (permalink / raw) To: Randy Dunlap Cc: kbuild test robot, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 7, 2018 at 10:38 AM, Randy Dunlap <rdunlap@infradead.org> wrote: > On 02/07/2018 10:13 AM, Linus Torvalds wrote: >> >> That said, intel only _documents_ UD2 (0f 0b). > > Intel Order Number: 325383-064US, October 2017, documents UD0, UD1, and UD2. > Section A.2.5, Table A-1, says: Ahh, I had an older version. Looking at the latest one I can find (325462-065US), it does specify that it has a modrm byte: 0F FF /r UD0 1 r32, r/m32 so I think that our opcode maps are wrong, and it's a bit dangerous to put random constants right after the UD0. Maybe we should make our use of UD0 have a third byte: add a harmless modrm byte before the warning constants? But yes, at least my objdump just thinks it's a bad 2-byte sequence, and doesn't look at any modrm bytes at all. Linus ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 19:01 ` Linus Torvalds @ 2018-02-07 19:06 ` Peter Zijlstra 2018-02-07 19:10 ` Peter Zijlstra 0 siblings, 1 reply; 35+ messages in thread From: Peter Zijlstra @ 2018-02-07 19:06 UTC (permalink / raw) To: Linus Torvalds Cc: Randy Dunlap, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers On Wed, Feb 07, 2018 at 11:01:29AM -0800, Linus Torvalds wrote: > On Wed, Feb 7, 2018 at 10:38 AM, Randy Dunlap <rdunlap@infradead.org> wrote: > > On 02/07/2018 10:13 AM, Linus Torvalds wrote: > >> > >> That said, intel only _documents_ UD2 (0f 0b). > > > > Intel Order Number: 325383-064US, October 2017, documents UD0, UD1, and UD2. > > Section A.2.5, Table A-1, says: > > Ahh, I had an older version. > > Looking at the latest one I can find (325462-065US), it does specify > that it has a modrm byte: > > 0F FF /r UD0 1 r32, r/m32 > > so I think that our opcode maps are wrong, and it's a bit dangerous to > put random constants right after the UD0. > > Maybe we should make our use of UD0 have a third byte: add a harmless > modrm byte before the warning constants? > > But yes, at least my objdump just thinks it's a bad 2-byte sequence, > and doesn't look at any modrm bytes at all. Look what my objdump does: $ objdump --version GNU objdump (GNU Binutils for Debian) 2.29.90.20180122 $ objdump -dr defconfig-build/kernel/sched/core.o | grep ud0 183a: 0f ff 65 48 ud0 0x48(%rbp),%esp 1881: 0f ff c3 ud0 %ebx,%eax 18b1: 0f ff 89 c0 48 0f a3 ud0 -0x5cf0b740(%rcx),%ecx 1940: 0f ff 89 c0 48 0f a3 ud0 -0x5cf0b740(%rcx),%ecx 19c2: 0f ff 89 c0 48 0f a3 ud0 -0x5cf0b740(%rcx),%ecx So yeah, we're screwed :-( Adding a harmless modr/m to our UD0 would grow the kernel image. ARGGH ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f 2018-02-07 19:06 ` Peter Zijlstra @ 2018-02-07 19:10 ` Peter Zijlstra 0 siblings, 0 replies; 35+ messages in thread From: Peter Zijlstra @ 2018-02-07 19:10 UTC (permalink / raw) To: Linus Torvalds Cc: Randy Dunlap, kbuild test robot, Ingo Molnar, Thomas Gleixner, LKML, the arch/x86 maintainers, H. Peter Anvin On Wed, Feb 07, 2018 at 08:06:51PM +0100, Peter Zijlstra wrote: > On Wed, Feb 07, 2018 at 11:01:29AM -0800, Linus Torvalds wrote: > > On Wed, Feb 7, 2018 at 10:38 AM, Randy Dunlap <rdunlap@infradead.org> wrote: > > > On 02/07/2018 10:13 AM, Linus Torvalds wrote: > > >> > > >> That said, intel only _documents_ UD2 (0f 0b). > > > > > > Intel Order Number: 325383-064US, October 2017, documents UD0, UD1, and UD2. > > > Section A.2.5, Table A-1, says: > > > > Ahh, I had an older version. > > > > Looking at the latest one I can find (325462-065US), it does specify > > that it has a modrm byte: > > > > 0F FF /r UD0 1 r32, r/m32 > > > > so I think that our opcode maps are wrong, and it's a bit dangerous to > > put random constants right after the UD0. > > > > Maybe we should make our use of UD0 have a third byte: add a harmless > > modrm byte before the warning constants? > > > > But yes, at least my objdump just thinks it's a bad 2-byte sequence, > > and doesn't look at any modrm bytes at all. > > Look what my objdump does: > > $ objdump --version > GNU objdump (GNU Binutils for Debian) 2.29.90.20180122 > > $ objdump -dr defconfig-build/kernel/sched/core.o | grep ud0 > 183a: 0f ff 65 48 ud0 0x48(%rbp),%esp > 1881: 0f ff c3 ud0 %ebx,%eax > 18b1: 0f ff 89 c0 48 0f a3 ud0 -0x5cf0b740(%rcx),%ecx > 1940: 0f ff 89 c0 48 0f a3 ud0 -0x5cf0b740(%rcx),%ecx > 19c2: 0f ff 89 c0 48 0f a3 ud0 -0x5cf0b740(%rcx),%ecx > > So yeah, we're screwed :-( > > Adding a harmless modr/m to our UD0 would grow the kernel image. ARGGH PeterA, at the time there was talk of arranging a single byte UD instruction. Any luck arranging that? ^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2018-02-15 0:39 UTC | newest] Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-07 11:44 [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f kbuild test robot 2018-02-07 18:13 ` Linus Torvalds 2018-02-07 18:35 ` Borislav Petkov 2018-02-07 18:49 ` Peter Zijlstra 2018-02-07 19:03 ` Linus Torvalds 2018-02-07 19:14 ` Peter Zijlstra 2018-02-07 19:28 ` Borislav Petkov 2018-02-07 19:43 ` Linus Torvalds 2018-02-07 20:24 ` Borislav Petkov 2018-02-08 9:13 ` Peter Zijlstra 2018-02-08 9:35 ` Peter Zijlstra 2018-02-08 9:46 ` Borislav Petkov 2018-02-08 9:47 ` David Laight 2018-02-08 10:13 ` Peter Zijlstra 2018-02-08 17:27 ` Linus Torvalds 2018-02-08 18:03 ` Peter Zijlstra 2018-02-08 18:15 ` Linus Torvalds 2018-02-08 19:44 ` Peter Zijlstra 2018-02-08 20:02 ` Linus Torvalds 2018-02-08 20:31 ` Borislav Petkov 2018-02-08 23:09 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Josh Poimboeuf 2018-02-08 23:09 ` [PATCH 1/2] objtool: Fix seg fault in ignore_unreachable_insn() Josh Poimboeuf 2018-02-13 11:29 ` [tip:x86/pti] objtool: Fix segfault " tip-bot for Josh Poimboeuf 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 2018-02-08 23:09 ` [PATCH 2/2] x86: Annotate WARN-related UD2 as reachable Josh Poimboeuf 2018-02-13 11:30 ` [tip:x86/pti] x86/debug, objtool: Annotate WARN()-related " tip-bot for Josh Poimboeuf 2018-02-15 0:26 ` tip-bot for Josh Poimboeuf 2018-02-09 8:13 ` [PATCH 0/2] objtool fixes on top of Peter's WARN UD2 patch Peter Zijlstra 2018-02-09 8:12 ` [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f Peter Zijlstra 2018-02-13 11:30 ` [tip:x86/pti] x86/debug: Use UD2 for WARN() tip-bot for Peter Zijlstra 2018-02-15 0:27 ` tip-bot for Peter Zijlstra 2018-02-07 18:38 ` [linus:master] BUILD REGRESSION a2e5790d841658485d642196dbb0927303d6c22f Randy Dunlap 2018-02-07 19:01 ` Linus Torvalds 2018-02-07 19:06 ` Peter Zijlstra 2018-02-07 19:10 ` Peter Zijlstra
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.