From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754393AbbCIOGV (ORCPT ); Mon, 9 Mar 2015 10:06:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41899 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753692AbbCIOGT (ORCPT ); Mon, 9 Mar 2015 10:06:19 -0400 From: Denys Vlasenko To: Andy Lutomirski Cc: Denys Vlasenko , Linus Torvalds , Steven Rostedt , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Oleg Nesterov , Frederic Weisbecker , Alexei Starovoitov , Will Drewry , Kees Cook , x86@kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] x86: entry_32.S: change ESPFIX test to not touch PT_OLDSS(%esp) Date: Mon, 9 Mar 2015 15:05:43 +0100 Message-Id: <1425909943-14687-1-git-send-email-dvlasenk@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Old code was trying to avoid having three branch insns, but instead it has a chain of six insns where each insn depends on previos one. And it was touching PT_OLDSS(%esp) unconditionally, even when it may contain bogus data. Elsewhere we have to jump thru hoops just to make sure here PT_OLDSS(%esp) is at least in a valid page. All this just to have one branch instead of three? The new code simply checks each condition. All three checks can run in parallel on an out-of-order CPU. Most of the time, none of branches will be taken. Comparison of object code: Old: 1e6: 8b 44 24 38 mov 0x38(%esp),%eax 1ea: 8a 64 24 40 mov 0x40(%esp),%ah 1ee: 8a 44 24 34 mov 0x34(%esp),%al 1f2: 25 03 04 02 00 and $0x20403,%eax 1f7: 3d 03 04 00 00 cmp $0x403,%eax 1fc: 74 0f je 20d New: 1e6: 0f ba 64 24 38 11 btl $0x11,0x38(%esp) 1ec: 72 0e jb 1fc 1ee: f6 44 24 34 03 testb $0x3,0x34(%esp) 1f3: 74 07 je 1fc 1f5: f6 44 24 40 04 testb $0x4,0x40(%esp) 1fa: 75 0f jne 20b Patch is run-tested. Signed-off-by: Denys Vlasenko CC: Linus Torvalds CC: Steven Rostedt CC: Ingo Molnar CC: Borislav Petkov CC: "H. Peter Anvin" CC: Andy Lutomirski CC: Oleg Nesterov CC: Frederic Weisbecker CC: Alexei Starovoitov CC: Will Drewry CC: Kees Cook CC: x86@kernel.org CC: linux-kernel@vger.kernel.org --- arch/x86/kernel/entry_32.S | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index e33ba51..0a4996b 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -516,16 +516,15 @@ restore_all: TRACE_IRQS_IRET restore_all_notrace: #ifdef CONFIG_X86_ESPFIX32 - movl PT_EFLAGS(%esp), %eax # mix EFLAGS, SS and CS - # Warning: PT_OLDSS(%esp) contains the wrong/random values if we - # are returning to the kernel. - # See comments in process.c:copy_thread() for details. - movb PT_OLDSS(%esp), %ah - movb PT_CS(%esp), %al - andl $(X86_EFLAGS_VM | (SEGMENT_TI_MASK << 8) | SEGMENT_RPL_MASK), %eax - cmpl $((SEGMENT_LDT << 8) | USER_RPL), %eax CFI_REMEMBER_STATE - je ldt_ss # returning to user-space with LDT SS + btl $X86_EFLAGS_VM_BIT,PT_EFLAGS(%esp) + jc restore_nocheck # VM set, not it + testb $3,PT_CS(%esp) + jz restore_nocheck # CPL0, not it + # Note: we access PT_OLDSS only when we know it exists. + # If PT_CS is from CPL0, it does not. + testb $SEGMENT_TI_MASK,PT_OLDSS(%esp) + jnz ldt_ss # returning to user-space with LDT SS #endif restore_nocheck: RESTORE_REGS 4 # skip orig_eax/error_code -- 1.8.1.4