Message-ID: <CALCETrW6LLmFR5Y6tmH=nPThCHefysf_nNwxHOFp4tAY4Spunw@mail.gmail.com>
Date:   Fri, 17 Apr 2020 08:52:06 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Sasha Levin <sashal@...nel.org>
Cc:     Andi Kleen <ak@...ux.intel.com>,
        "Bae, Chang Seok" <chang.seok.bae@...el.com>,
        Andy Lutomirski <luto@...nel.org>,
        "Metzger, Markus T" <markus.t.metzger@...el.com>,
        "hpa@...or.com" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "bp@...en8.de" <bp@...en8.de>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        Pedro Alves <palves@...hat.com>,
        Simon Marchi <simark@...ark.ca>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 00/17] Enable FSGSBASE instructions

tip-On Fri, Apr 17, 2020 at 6:30 AM Sasha Levin <sashal@...nel.org> wrote:
>
> On Mon, Apr 13, 2020 at 05:32:05PM -0700, Andi Kleen wrote:
> >> Is my attempt at understanding the current situation correct?
> >
> >Yes.
> >
> >Nothing breaks, and it's a nice improvement for context switch
> >performance, in NMI/PMU performance, and also gives user space two free
> >registers to play around with.
>
> Thomas, Andy,
>
> Could you list your outstanding objections to this patchset? I know it
> might be rehashing stuff you've already mentioned in this thread but I
> think that there's a disconnect between folks and it'll help with
> restarting everything.
>

My outstanding objections are:

1. The previous submission was broken.  This should obviously be fixed.

2. The issues documented here need to be addressed:

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=56f2ab41b652251f336a0f471b1033afeaedd161

3. Adding FSGSBASE fundamentally changes the user ABI, and the changes
will be observable.  This means that something could break, especially
in the case where ptrace is in use and the tracee uses the new
instructions.  The old behavior cannot sanely be preserved with
FSGSBASE enabled.  This isn't a showstopper, but whoever resubmits
this thing needs to document what changes and what use cases might
break.  I'm hopeful that the only thing that will break is actual
human beings using a tool like gdb to manually poke at the registers.
This is fine -- the behavior of the registers is different and human
beings debugging need to be aware of this.  But the existing automated
stuff that gdb, lldb, etc do needs to continue working.  This
especially includes using gdb to force the tracee to call a function,
e.g. 'p function()'.

4. The exising ptrace API does not provide a sane way to ask what the
base value associated with a selector would be.  This means that,
under the natural way to make FSGSBASE and ptrace work together (e.g.
as implemented in the previous submission), the tracer has no good way
to emulate 'MOV [whatever], %gs' in the tracee.

Now maybe no one cares about #4.  I certainly have the impression that
the *gdb developers* don't care.  But gdb isn't exactly a good example
of a piece of software that tries to work correctly when dealing with
unusual software.  Maybe other things like rr will care more.  It
might be nice to avoid a situation where a piece of careful software
(like rr?) can support kernel 5.y, but breaks in 5.y+1 because of
FSGSBASE, and only starts working again in 5.y+3 because we added the
ptrace API that's needed.

So maybe the first version should have a PTRACE_LOAD_SEGMENT that
sticks a selector in FS or GS and changes the base accordingly, even
if no current userspace has spoken up and said they need it.  And a
selftest.

--Andy

Powered by blists - more mailing lists