linux-kernel - Re: [PATCH 4/7] x86: Add support for rd/wr fs/gs base

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: <CALCETrVQiZPb1M183Fq61dbFAjnJScuB2y+rdH8BQ+yp-MO+Mw@mail.gmail.com>
Date:	Wed, 30 Apr 2014 16:44:52 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Andi Kleen <andi@...stfloor.org>, X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 4/7] x86: Add support for rd/wr fs/gs base

On Tue, Apr 29, 2014 at 9:52 PM, H. Peter Anvin <hpa@...or.com> wrote:
> On 04/29/2014 04:39 PM, Andi Kleen wrote:
>>> Case 3 is annoying.  If nothing tries to change the user gs base, then
>>> everything is okay because the user gs base and the kernel gs bases are
>>> equal.  But if something does try to change the user gs base, then it
>>> will accidentally change the kernel gs base instead.
>>
>> It doesn't really matter, as they are the same.
>> They would just switch identities.
>>
>> Besides I don't think anyone does that.
>>
>
> It matters -- greatly -- if (and only if) we can enter the kernel with
> usergs == kernelgs and then want to change usergs inside a paranoid
> routine.  At that point we risk being upside down, which basically means
> we're rooted.
>
> However, I believe this patchset also means only IST entries can be
> paranoid, which in turn means we can't sleep inside them.  To the very
> best of my knowledge the only times we change usergs is on context
> switch or inside a system call.  We need to make sure that is actually
> the case, though.
>
> I'm at ELC for a few days, so I'll have limited decent-sized-monitor
> time, but it shouldn't be too hard to convince ourselves of... mostly a
> matter of making sure something like ptrace can't to stupid crap.

The only things that look relevant are the context switch paths and
the kvm stuff.  I don't know what happens if an IST exception happens
while running a guest, though.  TBH I have no idea what the VMX and
SVM interfaces look like.

paranoid_schedule looks scary.  If I'm understanding it correctly, it
expects to be executed with gs == usergs.  I think it's okay, since it
will only be invoked if we trapped from userspace, in which case the
state is well-defined.  But this bit could be wrong:

    testl %ebx,%ebx                /* swapgs needed? */
    jnz paranoid_restore
    testl $3,CS(%rsp)
    jnz   paranoid_userspace

If usergs == kernelgs, then ebx will always be 1 and we'll never end
up in paranoid_userspace.

This could be fixed in two ways.  We could just switch the order of
the tests, since the only way to have ebx == 1 and CS with CPL == 3
should be if we're coming from userspace with usergs==kernelgs.  Or we
could get rid of the paranoid schedule code entirely.  It is actually
needed for anything?  Timer and rescheduling interrupts shouldn't be
paranoid, and if there's any paranoid code that will trigger a
reschedule, couldn't it do it much more sanely by sending an IPI to
self and thus deferring the reschedule until interrupts are enabled?

Alternatively, what if the paranoid entry checked whether we're coming
from userspace at the very beginning and, if so, just jumped to the
non-paranoid entry?

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