linux-kernel - Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER

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]

Message-ID: <CALCETrU4S6gD+wwBaTzUC7D7NQMSvd=3N5x6MpHR58tw-p-UZQ@mail.gmail.com>
Date:	Thu, 25 Feb 2016 10:20:17 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	Brian Gerst <brgerst@...il.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-tip-commits@...r.kernel.org" 
	<linux-tip-commits@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Denys Vlasenko <dvlasenk@...hat.com>
Subject: Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32

On Thu, Feb 25, 2016 at 7:42 AM, Brian Gerst <brgerst@...il.com> wrote:
> On Thu, Feb 25, 2016 at 8:47 AM, Brian Gerst <brgerst@...il.com> wrote:
>> On Thu, Feb 25, 2016 at 3:03 AM, Andy Lutomirski <luto@...capital.net> wrote:
>>>
>>> On Feb 24, 2016 10:01 PM, "H. Peter Anvin" <hpa@...or.com> wrote:
>>>>
>>>> On 02/24/16 21:53, tip-bot for Andy Lutomirski wrote:
>>>> > Commit-ID:  04d1d281dcfe683a53cddfab8371fc8bb302b069
>>>> > Gitweb:
>>>> > http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069
>>>> > Author:     Andy Lutomirski <luto@...nel.org>
>>>> > AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800
>>>> > Committer:  Ingo Molnar <mingo@...nel.org>
>>>> > CommitDate: Wed, 24 Feb 2016 08:43:04 +0100
>>>> >
>>>> > x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32
>>>> >
>>>> > Both before and after 5f310f739b4c ("x86/entry/32: Re-implement
>>>> > SYSENTER using the new C path"), we relied on a uaccess very early
>>>> > in the SYSENTER path to clear AC.  After that change, though, we can
>>>> > potentially make it all the way into C code with AC set, which
>>>> > enlarges the attack surface for SMAP bypass by doing SYSENTER with
>>>> > AC set.
>>>> >
>>>> > Strengthen the SMAP protection by addding the missing ASM_CLAC right
>>>> > at the beginning.
>>>> >
>>>>
>>>> Hmmm... this potentially adds a *lot* of unnecessary cycles to this
>>>> path.  Could we reinstate the early uaccess?
>>>
>>> I think that's more trouble than it's worth, and it'll undo a bunch of the
>>> context tracking cleanups that deferring it made possible, especially since
>>> this only matters in a configuration (32-bit SMAP) that no one uses. [1]
>>>
>>> *However*, I just realized that I have no idea why the 32-bit sysenter path
>>> is safe against NT being set.  I fixed it on compat, and now I'm confused as
>>> to the status on 32-bit.  If we need to fix up NT, I think we can fold AC
>>> into that.
>>
>> 32-bit still saves eflags in switch_to(), so NT can't leak to other
>> tasks.  But for consistency it should get the same treatment as 64-bit
>> (clear NT in sysenter entry and drop saving eflags in switch_to).
>
> Looking deeper, the sysexit path doesn't restore eflags, so we'd need
> to manually restore AC and IOPL.

Ick.

First things first, though -- there's no immediate reason to stop
saving flags on 32-bit context switches.  So we don't need to worry
about IOPL except for sanity and/or consistency.

Ideally we'd fix this up and restore flags on sysexit.  At least
failing to restore arithmetic flags isn't an info leak because the
exit code clobbers them with entirely predictable data.  I doubt
anyone cares all that much if we clobber AC.

I wrote a test for NT and the test fails for a different reason: our
TF handling appears broken as well.  (Our sysenter TF handling is
*crap*, but it seems to work on 64-bit kernels at least.)

My personal preference would be to add the missing popf.

--Andy