From patchwork Mon Oct 8 19:15:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tulio Magno Quites Machado Filho X-Patchwork-Id: 980762 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-96287-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="FkhIaYTc"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42TVSp38yvzB4MN for ; Tue, 9 Oct 2018 06:15:54 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:in-reply-to:references :message-id; q=dns; s=default; b=iwJBv03yA8CMizi0w1CSJxIQ+A7VjUj 4cdve6XE5QFYXEIqOUbOvaXf1LOwLHfihN3HQZurrgz+Cp5o73tdOxndzi1VEgfo MPtbPEQod97Lv6gOytYGzsvK/anwGaykdE019SaGg+sx4cS7d6KkX4m5tDWac19W jNPjv+HKn58o= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:in-reply-to:references :message-id; s=default; bh=Fibsb1Xvg9N8vU6o9cYeDQeIC2Y=; b=FkhIa YTcEwItRQgi9Dje9HIzLPk6w2M+kU4SdDUhjur6179ixfN9Buq7mIO2QVPz7dKTD Mxq070NpJog84rPtzaopNKHGBkQlwQjROVjqMG+NiNMbhefvnx4WPCvQqMD1BRkE 1noGSZfFHZAJKoH4CLhlxVyVW9osJajvKXIxeM= Received: (qmail 73526 invoked by alias); 8 Oct 2018 19:15:47 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 73342 invoked by uid 89); 8 Oct 2018 19:15:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=string.h, UD:string.h, stringh, lazy X-HELO: mx0a-001b2d01.pphosted.com From: Tulio Magno Quites Machado Filho To: "Carlos O'Donell" , libc-alpha@sourceware.org, John David Anglin , Adhemerval Zanella , Joseph Myers , Florian Weimer Subject: [PATCHv2] Protect _dl_profile_fixup data-dependency order [BZ #23690] Date: Mon, 8 Oct 2018 16:15:02 -0300 In-Reply-To: <2009c13b-169f-e476-6380-ffe4ceede9ac@redhat.com> References: <2009c13b-169f-e476-6380-ffe4ceede9ac@redhat.com> x-cbid: 18100819-0016-0000-0000-0000093DC1AC X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00009843; HX=3.00000242; KW=3.00000007; PH=3.00000004; SC=3.00000267; SDB=6.01099786; UDB=6.00568962; IPR=6.00879835; MB=3.00023667; MTD=3.00000008; XFM=3.00000015; UTC=2018-10-08 19:15:14 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18100819-0017-0000-0000-000040A1D14F Message-Id: <20181008191502.26499-1-tuliom@linux.ibm.com> I suspect this patch doesn't address all the comments from v1. However, I believe some of the open questions/comments may not be necessary anymore after the latest changes. I've decided to not add the new test to xtests, because it executes in less than 3s in most of my tests. There is just a single case that takes up to 30s. Changes since v1: - Fixed the coding style issues. - Replaced atomic loads/store with memory fences. - Added a test. ---- 8< ---- The field reloc_result->addr is used to indicate if the rest of the fields of reloc_result have already been written, creating a data-dependency order. Reading reloc_result->addr to the variable value requires to complete before reading the rest of the fields of reloc_result. Likewise, the writes to the other fields of the reloc_result must complete before reloc_result-addr is updated. 2018-10-08 Tulio Magno Quites Machado Filho [BZ #23690] * elf/dl-runtime.c (_dl_profile_fixup): Guarantee memory modification order when accessing reloc_result->addr. * nptl/Makefile (tests): Add tst-audit-threads. (modules-names): Add tst-audit-threads-mod1 and tst-audit-threads-mod2. Add rules to build tst-audit-threads. * nptl/tst-audit-threads-mod1.c: New file. * nptl/tst-audit-threads-mod2.c: Likewise. * nptl/tst-audit-threads.c: Likewise. * nptl/tst-audit-threads.h: Likewise. Signed-off-by: Tulio Magno Quites Machado Filho --- elf/dl-runtime.c | 19 ++++++++- nptl/Makefile | 14 ++++++- nptl/tst-audit-threads-mod1.c | 38 ++++++++++++++++++ nptl/tst-audit-threads-mod2.c | 22 +++++++++++ nptl/tst-audit-threads.c | 92 +++++++++++++++++++++++++++++++++++++++++++ nptl/tst-audit-threads.h | 86 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 267 insertions(+), 4 deletions(-) create mode 100644 nptl/tst-audit-threads-mod1.c create mode 100644 nptl/tst-audit-threads-mod2.c create mode 100644 nptl/tst-audit-threads.c create mode 100644 nptl/tst-audit-threads.h diff --git a/elf/dl-runtime.c b/elf/dl-runtime.c index 63bbc89776..c1ba372bd7 100644 --- a/elf/dl-runtime.c +++ b/elf/dl-runtime.c @@ -183,9 +183,18 @@ _dl_profile_fixup ( /* This is the address in the array where we store the result of previous relocations. */ struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index]; - DL_FIXUP_VALUE_TYPE *resultp = &reloc_result->addr; + /* CONCURRENCY NOTES: + + The following code uses DL_FIXUP_VALUE_CODE_ADDR to access a potential + member of reloc_result->addr to indicate if it is the first time this + object is being relocated. + Reading/Writing from/to reloc_result->addr must not happen before previous + writes to reloc_result complete as they could end-up with an incomplete + struct. */ + DL_FIXUP_VALUE_TYPE *resultp = &reloc_result->addr; DL_FIXUP_VALUE_TYPE value = *resultp; + atomic_thread_fence_acquire (); if (DL_FIXUP_VALUE_CODE_ADDR (value) == 0) { /* This is the first time we have to relocate this object. */ @@ -346,7 +355,13 @@ _dl_profile_fixup ( /* Store the result for later runs. */ if (__glibc_likely (! GLRO(dl_bind_not))) - *resultp = value; + { + /* Guarantee all previous writes complete before + resultp (aka. reloc_result->addr) is updated. See CONCURRENCY + NOTES earlier */ + atomic_thread_fence_release (); + *resultp = value; + } } /* By default we do not call the pltexit function. */ diff --git a/nptl/Makefile b/nptl/Makefile index be8066524c..5aed247a9d 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -382,7 +382,8 @@ tests += tst-cancelx2 tst-cancelx3 tst-cancelx4 tst-cancelx5 \ tst-cleanupx0 tst-cleanupx1 tst-cleanupx2 tst-cleanupx3 tst-cleanupx4 \ tst-oncex3 tst-oncex4 ifeq ($(build-shared),yes) -tests += tst-atfork2 tst-tls4 tst-_res1 tst-fini1 tst-compat-forwarder +tests += tst-atfork2 tst-tls4 tst-_res1 tst-fini1 tst-compat-forwarder \ + tst-audit-threads tests-internal += tst-tls3 tst-tls3-malloc tst-tls5 tst-stackguard1 tests-nolibpthread += tst-fini1 ifeq ($(have-z-execstack),yes) @@ -394,7 +395,8 @@ modules-names = tst-atfork2mod tst-tls3mod tst-tls4moda tst-tls4modb \ tst-tls5mod tst-tls5moda tst-tls5modb tst-tls5modc \ tst-tls5modd tst-tls5mode tst-tls5modf tst-stack4mod \ tst-_res1mod1 tst-_res1mod2 tst-execstack-mod tst-fini1mod \ - tst-join7mod tst-compat-forwarder-mod + tst-join7mod tst-compat-forwarder-mod tst-audit-threads-mod1 \ + tst-audit-threads-mod2 extra-test-objs += $(addsuffix .os,$(strip $(modules-names))) \ tst-cleanup4aux.o tst-cleanupx4aux.o test-extras += tst-cleanup4aux tst-cleanupx4aux @@ -709,6 +711,14 @@ endif $(objpfx)tst-compat-forwarder: $(objpfx)tst-compat-forwarder-mod.so +ifeq ($(run-built-tests),yes) +ifeq (yes,$(build-shared)) +$(objpfx)tst-audit-threads.out: $(objpfx)tst-audit-threads-mod1.so +$(objpfx)tst-audit-threads: $(objpfx)tst-audit-threads-mod2.so +tst-audit-threads-ENV = LD_AUDIT=$(objpfx)tst-audit-threads-mod1.so +endif +endif + # The tests here better do not run in parallel ifneq ($(filter %tests,$(MAKECMDGOALS)),) .NOTPARALLEL: diff --git a/nptl/tst-audit-threads-mod1.c b/nptl/tst-audit-threads-mod1.c new file mode 100644 index 0000000000..e2d3f78bae --- /dev/null +++ b/nptl/tst-audit-threads-mod1.c @@ -0,0 +1,38 @@ +/* Dummy audit library for test-audit-threads. + + Copyright (C) 2018 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include + +volatile int count = 0; + +unsigned int +la_version(unsigned int ver) +{ + return 1; +} + +unsigned int +la_objopen(struct link_map *map, Lmid_t lmid, uintptr_t *cookie) +{ + return LA_FLG_BINDTO | LA_FLG_BINDFROM; +} diff --git a/nptl/tst-audit-threads-mod2.c b/nptl/tst-audit-threads-mod2.c new file mode 100644 index 0000000000..6ceedb0196 --- /dev/null +++ b/nptl/tst-audit-threads-mod2.c @@ -0,0 +1,22 @@ +/* Shared object with a huge number of functions for test-audit-threads. + + Copyright (C) 2018 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* Define all the retNumN functions. */ +#define definenum +#include "tst-audit-threads.h" diff --git a/nptl/tst-audit-threads.c b/nptl/tst-audit-threads.c new file mode 100644 index 0000000000..93ddebaecb --- /dev/null +++ b/nptl/tst-audit-threads.c @@ -0,0 +1,92 @@ +/* Test multi-threading using LD_AUDIT. + + Copyright (C) 2018 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* This test uses a dummy LD_AUDIT library (test-audit-threads-mod1) and a + library with a huge number of functions in order to validate lazy symbol + binding with an audit library. */ + +#include +#include +#include +#include + +static int do_test (void); + +/* This test usually takes less than 3s to run. However, there are cases that + take up to 30s. */ +#define TIMEOUT 60 +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" + +#define externnum +#include "tst-audit-threads.h" +#undef externnum + +int num_threads; +pthread_barrier_t barrier; + +void +sync_all (int num) +{ + pthread_barrier_wait (&barrier); +} + +void +call_all_ret_nums (void) +{ +#define callnum +#include "tst-audit-threads.h" +#undef callnum +} + +void * +thread_main (void *unused) +{ + call_all_ret_nums (); + return NULL; +} + +#define STR2(X) #X +#define STR(X) STR2(X) + +static int +do_test (void) +{ + int i; + pthread_t *threads; + + num_threads = get_nprocs (); + if (num_threads <= 1) + num_threads = 2; + + /* Used to synchronize all the threads after calling each retNumN. */ + pthread_barrier_init (&barrier, NULL, num_threads); + + threads = (pthread_t *) malloc (num_threads * sizeof(pthread_t)); + bzero (threads, num_threads * sizeof(pthread_t)); + for (i = 0; i < num_threads; i++) + pthread_create(threads + i, NULL, thread_main, NULL); + + for (i = 0; i < num_threads; i++) + pthread_join(threads[i], NULL); + + free (threads); + + return 0; +} diff --git a/nptl/tst-audit-threads.h b/nptl/tst-audit-threads.h new file mode 100644 index 0000000000..c2b4d1d589 --- /dev/null +++ b/nptl/tst-audit-threads.h @@ -0,0 +1,86 @@ +/* Helper header for test-audit-threads. + + Copyright (C) 2018 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#define CONCAT(a, b) a ## b +#define NUM(x, y) CONCAT (x, y) + +#define FUNC10(x) \ + FUNC (NUM (x, 0)); \ + FUNC (NUM (x, 1)); \ + FUNC (NUM (x, 2)); \ + FUNC (NUM (x, 3)); \ + FUNC (NUM (x, 4)); \ + FUNC (NUM (x, 5)); \ + FUNC (NUM (x, 6)); \ + FUNC (NUM (x, 7)); \ + FUNC (NUM (x, 8)); \ + FUNC (NUM (x, 9)) + +#define FUNC100(x) \ + FUNC10 (NUM (x, 0)); \ + FUNC10 (NUM (x, 1)); \ + FUNC10 (NUM (x, 2)); \ + FUNC10 (NUM (x, 3)); \ + FUNC10 (NUM (x, 4)); \ + FUNC10 (NUM (x, 5)); \ + FUNC10 (NUM (x, 6)); \ + FUNC10 (NUM (x, 7)); \ + FUNC10 (NUM (x, 8)); \ + FUNC10 (NUM (x, 9)) + +#define FUNC1000(x) \ + FUNC100 (NUM (x, 0)); \ + FUNC100 (NUM (x, 1)); \ + FUNC100 (NUM (x, 2)); \ + FUNC100 (NUM (x, 3)); \ + FUNC100 (NUM (x, 4)); \ + FUNC100 (NUM (x, 5)); \ + FUNC100 (NUM (x, 6)); \ + FUNC100 (NUM (x, 7)); \ + FUNC100 (NUM (x, 8)); \ + FUNC100 (NUM (x, 9)) + +#define FUNC10000() \ + FUNC1000 (1); \ + FUNC1000 (2); \ + FUNC1000 (3); \ + FUNC1000 (4); \ + FUNC1000 (5); \ + FUNC1000 (6); \ + FUNC1000 (7); \ + FUNC1000 (8); \ + FUNC1000 (9) + +#ifdef FUNC +# undef FUNC +#endif + +#ifdef externnum +# define FUNC(x) extern int CONCAT (retNum, x) (void) +#endif + +#ifdef definenum +# define FUNC(x) int CONCAT (retNum, x) (void) { return x; } +#endif + +#ifdef callnum +# define FUNC(x) CONCAT (retNum, x) (); sync_all (x) +#endif + +FUNC10000 ();