linux-kernel - Re: xgetbv nondeterminism

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: <CAMe9rOqvdMpe7eV6npsmFwvzkB9iD+n_0HoPj8wW6BEFa5-y4A@mail.gmail.com>
Date:   Sat, 17 Jun 2017 05:51:39 -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 11:21 PM, Andy Lutomirski <luto@...nel.org> wrote:
>>>
>>> In any event, I still don't understand the issue.  The code does this,
>>> effectively:
>>>
>>> PLT -> GOT
>>> GOT points to a stub that transfers control to ld.so
>>> ld.so resolves the symbol (_dl_fixup, I think)
>>> ld.so patches the GOT
>>> ld.so jumps to the resolved function
>>>
>>> As far as I can tell, the only part of the whole process that might
>>> touch vector registers at all is elf_ifunc_invoke().  Couldn't all the
>>> register saving and restoring be moved to elf_ifunc_invoke()?
>>
>> Please grep for  FOREIGN_CALL the elf directory.
>
> I grepped FOREIGN_CALL.  It has no explanation whatsoever and appears
> to unconditionally do nothing in the current glibc version.
>
> In f3dcae82d54e5097e18e1d6ef4ff55c2ea4e621e^, in pseudocode, it does:
>
> __thread bool must_save;
>
> RTLD_CHECK_FOREIGN_CALL: return must_save;
>
> RTLD_ENABLE_FOREIGN_CALL: old_must_save = must_save; must_save = true;
>
> RTLD_PREPARE_FOREIGN_CALL: save_state(); must_save = false;
>
> RTLD_FINALIZE_FOREIGN_CALL: if (must_save) restore(); must_save = old_must_save;
>
> save_state() and restore_state() operate on TLS buffers.
>
> In summary: this is not async-signal-safe.  It's also really messy --
> there are macros that declare local variables, and the logic isn't
> apparent without really digging in to all the code.
>
> I still don't see why this couldn't be:
>
> static void elf_do_foreign_stuff(args here)
> {
>   void *buf = alloca(state_size);
>   xsaveopt(buf);  /* or open-code it if you prefer */
>   call_the_ifunc();
>   xrstor(buf);
> }

As you have found out that it doesn't work this way since

RTLD_PREPARE_FOREIGN_CALL

and

RTLD_FINALIZE_FOREIGN_CALL

are used in 2 DIFFERENT files.

> If there's more than just the iifunc (malloc?  profiling?  printf?)
> then all of that could be wrapped as well.

It has nothing to do with ifunc.

> All this stuff comes from:
>
> commit b48a267b8fbb885191a04cffdb4050a4d4c8a20b
> Author: Ulrich Drepper <drepper@...hat.com>
> Date:   Wed Jul 29 08:33:03 2009 -0700
>
>     Preserve SSE registers in runtime relocations on x86-64.
>
>     SSE registers are used for passing parameters and must be preserved
>     in runtime relocations.  This is inside ld.so enforced through the
>     tests in tst-xmmymm.sh.  But the malloc routines used after startup
>     come from libc.so and can be arbitrarily complex.  It's overkill
>     to save the SSE registers all the time because of that.  These calls
>     are rare.  Instead we save them on demand.  The new infrastructure
>     put in place in this patch makes this possible and efficient.
>
> While I think that the control flow is a giant mess and the use of TLS

Yes.

> was a mistake, I think Uli had the right idea: explicitly save the

Yes.

> extended state only when needed.
>

Only its implementation lead to race condition.

-- 
H.J.