| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160121225641.GC300@pd.tnic>
Date: Thu, 21 Jan 2016 23:56:41 +0100
From: Borislav Petkov <bp@...e.de>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: Andy Lutomirski <luto@...capital.net>,
Brian Gerst <brgerst@...il.com>,
the arch/x86 maintainers <x86@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Denys Vlasenko <dvlasenk@...hat.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after
init
On Thu, Jan 21, 2016 at 02:22:28PM -0800, H. Peter Anvin wrote:
> Yes, having t_no as the fallthrough case ought to move the yes code
> out of line.
Dunno, maybe I'm doing something wrong:
I have this change:
---
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 7f09de998c93..f9833fcb8fcb 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -175,10 +175,10 @@ static __always_inline __pure bool _static_cpu_has(u16 bit)
[bitnum] "i" (1 << (bit & 7)),
[cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
: : t_yes, t_no);
- t_yes:
- return true;
t_no:
return false;
+ t_yes:
+ return true;
}
#define static_cpu_has(bit) \
---
and the resulting code looks even wrong (or my brain is fried for today
- one of the two).
vmlinux:
ffffffff810046ae: e9 cc 0e de 00 jmpq ffffffff81de557f <__alt_instructions_end+0x7aa>
ffffffff810046b3: 48 83 c4 18 add $0x18,%rsp
ffffffff810046b7: 4c 89 e0 mov %r12,%rax
ffffffff810046ba: 5b pop %rbx
ffffffff810046bb: 41 5c pop %r12
ffffffff810046bd: 41 5d pop %r13
ffffffff810046bf: 41 5e pop %r14
ffffffff810046c1: 41 5f pop %r15
ffffffff810046c3: 5d pop %rbp
ffffffff810046c4: c3 retq
dynamic branch:
ffffffff81de557f: f6 05 8f de d1 ff 01 testb $0x1,-0x2e2171(%rip) # ffffffff81b03415 <boot_cpu_data+0x55>
ffffffff81de5586: 0f 85 f6 f1 21 ff jne ffffffff81004782 <__switch_to+0x332>
ffffffff81de558c: e9 22 f1 21 ff jmpq ffffffff810046b3 <__switch_to+0x263>
after X86_FEATURE_ALWAYS patching:
[ 0.288007] apply_alternatives: feat: 3*32+21, old: (ffffffff810046ae, len: 5), repl: (ffffffff81de4dff, len: 5), pad: 0
[ 0.292004] ffffffff810046ae: old_insn: e9 cc 0e de 00
[ 0.300013] ffffffff81de4dff: rpl_insn: e9 af f8 21 ff
[ 0.308006] recompute_jump: target RIP: ffffffff810046b3, new_displ: 0x5
[ 0.312006] recompute_jump: final displ: 0x00000003, JMP 0xffffffff810046b3
[ 0.316006] ffffffff810046ae: final_insn: eb 03 0f 1f 00
ffffffff810046ae: eb 03 0f 1f 00 jmp ffffffff810046b3 ---
ffffffff810046b3: 48 83 c4 18 add $0x18,%rsp <--
ffffffff810046b7: 4c 89 e0 mov %r12,%rax
ffffffff810046ba: 5b pop %rbx
ffffffff810046bb: 41 5c pop %r12
ffffffff810046bd: 41 5d pop %r13
ffffffff810046bf: 41 5e pop %r14
ffffffff810046c1: 41 5f pop %r15
ffffffff810046c3: 5d pop %rbp
ffffffff810046c4: c3 retq
so this is silly: we're basically jumping after the JMP instruction
itself. So that will be the case on !X86_BUG_SYSRET_SS_ATTRS CPUs.
Still a two-byte and now even a useless JMP.
The right thing to do would be to generate a NOP simply.
On X86_BUG_SYSRET_SS_ATTRS CPUs:
[ 0.322014] apply_alternatives: feat: 16*32+8, old: (ffffffff810046ae, len: 5), repl: (ffffffff81de3962, len: 0), pad: 0
[ 0.324005] ffffffff810046ae: old_insn: eb 03 0f 1f 00
[ 0.332006] ffffffff810046ae: final_insn: 0f 1f 44 00 00
ffffffff810046ae: 0f 1f 44 00 00 nop
ffffffff810046b3: 48 83 c4 18 add $0x18,%rsp
ffffffff810046b7: 4c 89 e0 mov %r12,%rax
ffffffff810046ba: 5b pop %rbx
ffffffff810046bb: 41 5c pop %r12
ffffffff810046bd: 41 5d pop %r13
ffffffff810046bf: 41 5e pop %r14
ffffffff810046c1: 41 5f pop %r15
ffffffff810046c3: 5d pop %rbp
ffffffff810046c4: c3 retq
which is actually even wrong!
What it should've done is
jne ffffffff81004782
as the dynamic code did. At that address we have the ss fixup:
ffffffff81004782: 66 8c d0 mov %ss,%ax
ffffffff81004785: 66 83 f8 18 cmp $0x18,%ax
ffffffff81004789: 0f 84 24 ff ff ff je ffffffff810046b3 <__switch_to+0x263>
ffffffff8100478f: b8 18 00 00 00 mov $0x18,%eax
ffffffff81004794: 8e d0 mov %eax,%ss
ffffffff81004796: e9 18 ff ff ff jmpq ffffffff810046b3 <__switch_to+0x263>
with the jump back to the ret code. Which means,
!X86_BUG_SYSRET_SS_ATTRS CPUs get to do a forward and a backward JMP. So
even if it did the right thing, it would be two JMPs.
Meh.
I need to think about something better.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
Powered by blists - more mailing lists