Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH] x86: Remove force_iret()
Date:   Fri, 20 Dec 2019 18:30:19 +0800
Message-Id: <D890A7DA-542B-42A7-8F82-CDBA6EBCA958@amacapital.net>
References: <431a146f6461402da61d09fff155f35b@AcuMS.aculab.com>
Cc:     Brian Gerst <brgerst@gmail.com>, Andy Lutomirski <luto@kernel.org>,
        X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Oleg Nesterov <oleg@redhat.com>
In-Reply-To: <431a146f6461402da61d09fff155f35b@AcuMS.aculab.com>
To:     David Laight <David.Laight@aculab.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Dec 20, 2019, at 6:10 PM, David Laight <David.Laight@aculab.com> wrote:=

>=20
> =EF=BB=BFFrom: Brian Gerst
>> Sent: 20 December 2019 03:48
>>> On Thu, Dec 19, 2019 at 8:50 PM Andy Lutomirski <luto@kernel.org> wrote:=

>>>=20
>>> On Thu, Dec 19, 2019 at 3:58 AM Brian Gerst <brgerst@gmail.com> wrote:
>>>>=20
>>>> force_iret() was originally intended to prevent the return to user mode=
 with
>>>> the SYSRET or SYSEXIT instructions, in cases where the register state c=
ould
>>>> have been changed to be incompatible with those instructions.
>>>=20
>>> It's more than that.  Before the big syscall rework, we didn't restore
>>> the caller-saved regs.  See:
>>>=20
>>> commit 21d375b6b34ff511a507de27bf316b3dde6938d9
>>> Author: Andy Lutomirski <luto@kernel.org>
>>> Date:   Sun Jan 28 10:38:49 2018 -0800
>>>=20
>>>    x86/entry/64: Remove the SYSCALL64 fast path
>>>=20
>>> So if you changed r12, for example, the change would get lost.
>>=20
>> 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.
>=20
> 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, LD=
T 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 kern=
el, due to sigreturn or ptrace.  We have delightful selftests for this.

>=20
> So even though the values were valid when changed, they are
> invalid during the 'return to user' sequence.
>=20
> 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 po=
ssible :)

>=20
> 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?
>=20
>=20

The validation we=E2=80=99re talking about is for SYSRET, not IRET.  It has i=
ts own set of nasty conditions involving EFLAGS, R11, RIP, and RCX.  Fortuna=
tely no segments are involved. The algorithm is, roughly:

if (okay for SYSRET) {
  SYSRET (and assume it can=E2=80=99t 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 i=
n truly awful bugs.

We can=E2=80=99t handle #GP from SYSRET. Thanks, Intel.

(AMD gets this more right. SYSRET is still a turd, but it can=E2=80=99t faul=
t. 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.)=