linux-kernel - Re: [PATCH] x86: Remove force

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: <D890A7DA-542B-42A7-8F82-CDBA6EBCA958@amacapital.net>
Date:   Fri, 20 Dec 2019 18:30:19 +0800
From:   Andy Lutomirski <luto@...capital.net>
To:     David Laight <David.Laight@...lab.com>
Cc:     Brian Gerst <brgerst@...il.com>, Andy Lutomirski <luto@...nel.org>,
        X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        "H . Peter Anvin" <hpa@...or.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH] x86: Remove force_iret()



> On Dec 20, 2019, at 6:10 PM, David Laight <David.Laight@...lab.com> wrote:
> 
> From: Brian Gerst
>> Sent: 20 December 2019 03:48
>>> On Thu, Dec 19, 2019 at 8:50 PM Andy Lutomirski <luto@...nel.org> wrote:
>>> 
>>> On Thu, Dec 19, 2019 at 3:58 AM Brian Gerst <brgerst@...il.com> wrote:
>>>> 
>>>> force_iret() was originally intended to prevent the return to user mode with
>>>> the SYSRET or SYSEXIT instructions, in cases where the register state could
>>>> have been changed to be incompatible with those instructions.
>>> 
>>> It's more than that.  Before the big syscall rework, we didn't restore
>>> the caller-saved regs.  See:
>>> 
>>> commit 21d375b6b34ff511a507de27bf316b3dde6938d9
>>> Author: Andy Lutomirski <luto@...nel.org>
>>> Date:   Sun Jan 28 10:38:49 2018 -0800
>>> 
>>>    x86/entry/64: Remove the SYSCALL64 fast path
>>> 
>>> So if you changed r12, for example, the change would get lost.
>> 
>> force_iret() specifically dealt with changes to CS, SS and EFLAGS.
>> Saving and restoring the extra registers was a different problem
>> although it affected the same functions like ptrace, signals, and
>> exec.
> 
> Is it ever possible for any of the segment registers to refer to the LDT
> and for another thread to invalidate the entries 'very late' ?

Not in newer kernels, because the actual LDT is never modified.  Instead, LDT changes create a whole new LDT and propagate it with an IPI.

But the IRET path can fail due to changes to the selectors while in the kernel, due to sigreturn or ptrace.  We have delightful selftests for this.

> 
> So even though the values were valid when changed, they are
> invalid during the 'return to user' sequence.
> 
> I remember writing a signal handler that 'corrupted' all the
> segment registers (etc) and fixing the NetBSD kernel to handle
> all the faults restoring the segment registers and IRET faulting
> in kernel (IIRC invalid user %SS or %CS).
> (IRET can also fault in user space, but that is a normal fault.)

Did you remember to test the #NP case?  Many kernels forgot that this was possible :)

> 
> Is it actually cheaper to properly validate the segment registers,
> or take the 'hit' of the slightly slower IRET path and get the cpu
> to do it for you?
> 
> 

The validation we’re talking about is for SYSRET, not IRET.  It has its own set of nasty conditions involving EFLAGS, R11, RIP, and RCX.  Fortunately no segments are involved. The algorithm is, roughly:

if (okay for SYSRET) {
  SYSRET (and assume it can’t fail)
} else {
  if (need ESPFIX)
   Horrible hacks;
  IRET;
}

And we handle #GP, #SS, #NP and #DF from IRET. And we have selftests for all of this. And no one runs the bloody selftests on 32-bit kernels, resulting in truly awful bugs.

We can’t handle #GP from SYSRET. Thanks, Intel.

(AMD gets this more right. SYSRET is still a turd, but it can’t fault. Intel handles RIP canonical checks differently from AMD, and SYSRET will #GP if RCX is noncanonical.  The result was privilege escalation on basically every OS when this was noticed.)