From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932234AbbHCTCT (ORCPT ); Mon, 3 Aug 2015 15:02:19 -0400 Received: from wtarreau.pck.nerim.net ([62.212.114.60]:35582 "EHLO 1wt.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932085AbbHCTCR (ORCPT ); Mon, 3 Aug 2015 15:02:17 -0400 Date: Mon, 3 Aug 2015 21:01:49 +0200 From: Willy Tarreau To: Andy Lutomirski Cc: Andy Lutomirski , Kees Cook , Steven Rostedt , "security@kernel.org" , X86 ML , Borislav Petkov , Sasha Levin , LKML , Konrad Rzeszutek Wilk , Boris Ostrovsky , Andrew Cooper , Jan Beulich , xen-devel Subject: Re: [PATCH 2/2] x86/ldt: allow to disable modify_ldt at runtime Message-ID: <20150803190148.GB24014@1wt.eu> References: <1438626217-23970-1-git-send-email-w@1wt.eu> <1438626217-23970-3-git-send-email-w@1wt.eu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Aug 03, 2015 at 11:45:24AM -0700, Andy Lutomirski wrote: > I'm not entirely convinced that the lock bit should work this way. At > some point, we might want a setting for "32-bit only" or even "32-bit, > present, not non-conforming only" (like we do unconditionally for > set_thread_area). When we do that, having -1 act like 0 might be > confusing. > > I'd actually favor rigging it up to support enumerated values and/or > the word "locked" somewhere in the text. So we could have "0", "1 > locked", "1" or even "enabled" "enabled locked", "disabled", "disabled > locked", "safe 32-bit", "safe 32-bit locked", etc. Got it, that makes sense indeed. I asked myself whether we'd use more than these 3 values, and estimated that "locked on" didn't make much sense here, and I thought that nobody would like to manipulate such things using bitmaps. But with words like this it can indeed make sense. I feel like it's probably part of a larger project then. Do you think we should step back and only support 0/1 for now ? I also have the patch available. > I'll add an explicit 16-bit check to my infinite todo list for the asm > part. Now that the synchronous modify_ldt code is merged, it won't be > racy, and it would make a 32-bit only mode actually be useful (except > maybe on AMD -- someone needs to test just how badly broken IRET is on > AMD systems -- I know that AMD has IRET-to-16-bit differently broken > from Intel, and that causes test-cast failures. Grump.) > > P.S. Hey CPU vendors: please consider stopping your utter suckage when > it comes to critical system instructions. Intel and AMD both > terminally screwed up IRET in multiple ways that clearly took actual > effort. Intel screwed up SYSRET pretty badly (AFAIK every single > 64-bit OS has had at least one root hole as a result), and AMD screwed > SYSRET up differently (userspace crash bug that requires a performance > hit to mitigate because no one at AMD realized that one might preempt > a process during a syscall). Well the good thing is that SYSRET reused the LOADALL opcode so at least this one cannot be screwed on 64-bit :-) It would have helped us to emulate IRET though. > P.P.S. You know what would be *way* better than allowing IRET to > fault? Just allow IRET to continue executing the next instruction on > failure (I'm talking about #GP, #NP, and #SS here, not page faults). > > P.P.P.S. Who thought that IRET faults unmasking NMIs made any sense > whatsoever when NMIs run on an IST stack? Seriously, people? A dedicated flag "don't clear NMI yet" would have been nice in EFLAGS so that the software stack could set it in fault handlers. It would be one-shot and always cleared by IRET. That would have been very handy. Willy