linux-kernel - Re: [PATCH 2/2] x86/ldt: allow to disable modify

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 3 Aug 2015 12:06:12 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Willy Tarreau <w@....eu>
Cc:	Andy Lutomirski <luto@...nel.org>,
	Kees Cook <keescook@...omium.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	"security@...nel.org" <security@...nel.org>,
	X86 ML <x86@...nel.org>, Borislav Petkov <bp@...en8.de>,
	Sasha Levin <sasha.levin@...cle.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Boris Ostrovsky <boris.ostrovsky@...cle.com>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	Jan Beulich <jbeulich@...e.com>,
	xen-devel <xen-devel@...ts.xen.org>
Subject: Re: [PATCH 2/2] x86/ldt: allow to disable modify_ldt at runtime

On Mon, Aug 3, 2015 at 12:01 PM, Willy Tarreau <w@....eu> wrote:
> 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.

Sounds good to me.

>
>> 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.

You sure?  I'm reasonably confident that Athlon 64 and newer support
SYSRET in legacy and long mode.  Of course, I think that SYSCALL is
totally worthless in legacy mode (SYSCALL_MASK isn't available, so I
suspect that the lack of sensible TF handling would be a
show-stopper).

>
>> 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.
>

How about a dedicated "NMI masked" flag in EFLAGS?  That would be
straightforward and dead simple to handle.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/