From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753029AbdFQMvn (ORCPT ); Sat, 17 Jun 2017 08:51:43 -0400 Received: from mail-ot0-f174.google.com ([74.125.82.174]:36468 "EHLO mail-ot0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751957AbdFQMvl (ORCPT ); Sat, 17 Jun 2017 08:51:41 -0400 MIME-Version: 1.0 In-Reply-To: References: <1eb55c22-6c90-e1b3-19d4-cb7b2c6fc0dc@intel.com> From: "H.J. Lu" Date: Sat, 17 Jun 2017 05:51:39 -0700 Message-ID: Subject: Re: xgetbv nondeterminism To: Andy Lutomirski Cc: Dave Hansen , "Robert O'Callahan" , "linux-kernel@vger.kernel.org" , X86 ML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 16, 2017 at 11:21 PM, Andy Lutomirski 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 > 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.