All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Gerst <brgerst@gmail.com>
To: David Laight <David.Laight@aculab.com>
Cc: 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>
Subject: Re: [PATCH] x86: Remove force_iret()
Date: Fri, 20 Dec 2019 07:18:02 -0500	[thread overview]
Message-ID: <CAMzpN2i+DrKkzDyiS6Cj61LmCu+--e5puQpKrNxYVMDRPMvvBw@mail.gmail.com> (raw)
In-Reply-To: <431a146f6461402da61d09fff155f35b@AcuMS.aculab.com>

On Fri, Dec 20, 2019 at 5:10 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Brian Gerst
> > Sent: 20 December 2019 03:48
> > On Thu, Dec 19, 2019 at 8:50 PM Andy Lutomirski <luto@kernel.org> wrote:
> > >
> > > On Thu, Dec 19, 2019 at 3:58 AM Brian Gerst <brgerst@gmail.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@kernel.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' ?
> So even though the values were valid when changed, they are
> invalid during the 'return to user' sequence.

Not in the SYSRET case, where the kernel requires that CS and SS are
static segments in the GDT.  Any userspace context that uses LDT
segments for CS/SS must return with IRET.  There is fault handling for
IRET (fixup_bad_iret()) for this case.

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

SYSRET is faster because it avoids segment table lookups and
permission checks for CS and SS.  It simply sets the selectors to
values set in an MSR and the attributes (base, limit, etc.) to fixed
values.  It is up to the OS to make sure the actual segment
descriptors in memory match those default attributes.

--
Brian Gerst

  parent reply	other threads:[~2019-12-20 12:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-19 11:58 [PATCH] x86: Remove force_iret() Brian Gerst
2019-12-20  1:49 ` Andy Lutomirski
2019-12-20  3:48   ` Brian Gerst
2019-12-20 10:10     ` David Laight
2019-12-20 10:30       ` Andy Lutomirski
2019-12-20 10:59         ` David Laight
2019-12-20 21:20           ` Andy Lutomirski
2019-12-20 12:18       ` Brian Gerst [this message]
2019-12-20 12:35         ` David Laight
2019-12-20 19:35 ` Oleg Nesterov
2020-01-08 20:30 ` [tip: x86/asm] " tip-bot2 for Brian Gerst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMzpN2i+DrKkzDyiS6Cj61LmCu+--e5puQpKrNxYVMDRPMvvBw@mail.gmail.com \
    --to=brgerst@gmail.com \
    --cc=David.Laight@aculab.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.