Message-ID: <CAMe9rOpsdpS9rP5wQrJ49zJ+5ir7_bNvp5x6pKhoKMGz8BawFw@mail.gmail.com>
Date:   Fri, 16 Jun 2017 10:44:18 -0700
From:   "H.J. Lu" <hjl.tools@...il.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Dave Hansen <dave.hansen@...el.com>,
        "Robert O'Callahan" <robert@...llahan.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        X86 ML <x86@...nel.org>
Subject: Re: xgetbv nondeterminism

On Fri, Jun 16, 2017 at 9:38 AM, Andy Lutomirski <luto@...nel.org> wrote:
> On Fri, Jun 16, 2017 at 9:17 AM, H.J. Lu <hjl.tools@...il.com> wrote:
>> On Fri, Jun 16, 2017 at 9:01 AM, Andy Lutomirski <luto@...nel.org> wrote:
>>> On Thu, Jun 15, 2017 at 9:34 PM, H.J. Lu <hjl.tools@...il.com> wrote:
>>>> On Thu, Jun 15, 2017 at 8:05 PM, Andy Lutomirski <luto@...nel.org> wrote:
>>>>> On Thu, Jun 15, 2017 at 7:17 PM, H.J. Lu <hjl.tools@...il.com> wrote:
>>>>>> On Thu, Jun 15, 2017 at 4:28 PM, Andy Lutomirski <luto@...nel.org> wrote:
>>>>>>> On Thu, Jun 15, 2017 at 4:11 PM, H.J. Lu <hjl.tools@...il.com> wrote:
>>>>>>>> It is used for lazy binding the first time when an external function is called.
>>>>>>>>
>>>>>>>
>>>>>>> Maybe I'm just being dense, but why?  What does ld.so need to do to
>>>>>>> resolve a symbol and update the GOT that requires using extended
>>>>>>> state?
>>>>>>
>>>>>> Since the first 8 vector registers are used to pass function parameters
>>>>>> and ld.so uses vector registers, _dl_runtime_resolve needs to preserve
>>>>>> the first 8 vector registers when transferring control to ld.so.
>>>>>>
>>>>>
>>>>> Wouldn't it be faster and more future-proof to recompile the relevant
>>>>> parts of ld.so to avoid using extended state?
>>>>>
>>>>
>>>> Are you suggesting not to use vector in ld.so?
>>>
>>> Yes, exactly.
>>>
>>>>  We used to do that
>>>> several years ago, which leads to some subtle bugs, like
>>>>
>>>> https://sourceware.org/bugzilla/show_bug.cgi?id=15128
>>>
>>> I don't think x86_64 has the issue that ARM has there.  The Linux
>>> kernel, for example, has always been compiled to not use vector or
>>> floating point registers on x86 (32 and 64), and it works fine.  Linux
>>> doesn't save extended regs on kernel entry and it doesn't restore them
>>> on exit.
>>>
>>> I would suggest that ld.so be compiled without use of vector
>>> registers, that the normal lazy binding path not try to save any extra
>>> regs, and that ifuncs be called through a thunk that saves whatever
>>> registers need saving, possibly just using XSAVEOPT.  After all, ifunc
>>> is used for only a tiny fraction of symbols.
>>
>> x86-64 was the only target which used FOREIGN_CALL macros
>> in ld.so,  FOREIGN_CALL macros were the cause of race condition
>> in ld.so:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=11214
>>
>> Not to save and restore the first 8 vector registers means that
>> FOREIGN_CALL macros have to be used.  We don't want to
>> do that on x86-64.
>>
>>
>
> You're talking about this, right:
>
> commit f3dcae82d54e5097e18e1d6ef4ff55c2ea4e621e
> Author: H.J. Lu <hjl.tools@...il.com>
> Date:   Tue Aug 25 04:33:54 2015 -0700
>
>     Save and restore vector registers in x86-64 ld.so
>
> It seems to me that the problem wasn't that the save/restore happened
> on some of the time -- it was that the save and restore code used a
> TLS variable to track its own state.  Shouldn't it have been a stack
> variable or even just implicit in the control flow?

No, it can't use stack variable since _dl_runtime_resolve never
returns.

> In any case, glibc is effectively doing a foreign call anyway, right?

No.

> It's doing the foreign call to itself on every lazy binding
> resolution, though, which seems quite expensive.  I'm saying that it
> seems like it would be more sensible to do the complicated foreign
> call logic only when doing the dangerous case, which is when lazy
> binding calls an ifunc.
>
> If I were to rewrite this, I would do it like this:
>
> void *call_runtime_ifunc(void (*ifunc)());  // or whatever the
> signature needs to be

It is unrelated to IFUNC.  This is how external function call works.

> call_runtime_ifunc would be implemented in asm (or maybe even C!) and
> would use XSAVEOPT or similar to save the state to a buffer on the
> stack.  Then it would call the ifunc and restore the state.  No TLS
> needed, so there wouldn't be any races.  In fact, it would work very
> much like your current save/restore code, except that it wouldn't need
> to be as highly optimized because it would be called much less
> frequently.  This should improve performance and could be quite a bit
> simpler.
>
> As an aside, why is saving the first eight registers enough?  I don't
> think there's any particular guarantee that a call through the GOT
> uses the psABI, is there?  Compilers can and do produce custom calling
> conventions, and ISTM that some day a compiler might do that between
> DSOs.  Or those DSOs might not be written in C in the first place.

The result is undefined if psABI isn't followed.

-- 
H.J.

Powered by blists - more mailing lists