From patchwork Thu Nov 27 15:35:53 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 415572 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 5609E14012A for ; Fri, 28 Nov 2014 02:36:06 +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:date:from:to:subject:message-id:references :mime-version:content-type:in-reply-to; q=dns; s=default; b=UVJU cN442utVeEdFfAuHKHOmDYWBQL2pnw7PIAQIa7cEv/cGecKP1ncw05B82BmTQIoo PwBoGQNFGQAnju+yimi0GkG9trDTmG3hMi0v6R/0W3X/VTKVrKApUtcwSjuEyCBC Gy8oa3RbS5pSqGZJSLy3D9yZhsSsfzc1dMF/r8E= 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:date:from:to:subject:message-id:references :mime-version:content-type:in-reply-to; s=default; bh=HR4tbfQanZ WJlvkszYWfQtKdEc0=; b=W/horKZc+mncUUKfVzLoy++hzyFN5wkJrpibn+I1XV fzmUsM8a1tL6l1cABp5VoyOebpcMcXSoPpIOCjAn9UUaQiUfcqiWmdWcJYb5Pu3Y tETxN56tPAi9iylkRhT1I3hiKY91zsSUDRFOlz8HBXzatS/ChukddaRsAXnNQOg/ I= Received: (qmail 18014 invoked by alias); 27 Nov 2014 15:36:00 -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 18005 invoked by uid 89); 27 Nov 2014 15:36:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yh0-f44.google.com X-Received: by 10.236.199.102 with SMTP id w66mr4223623yhn.10.1417102556128; Thu, 27 Nov 2014 07:35:56 -0800 (PST) Date: Thu, 27 Nov 2014 07:35:53 -0800 From: "H.J. Lu" To: Joseph Myers , Paul Archard , Ond??ej B??lka , MyungJoo Ham , GNU C Library Subject: Re: [PATCH][BZ #13862] Reuse of cached stack can cause bounds overrun of thread DTV Message-ID: <20141127153553.GA8492@gmail.com> References: <20140204121120.GB22572@domone.podge> <00c301cf21d3$ca7253e0$5f56fba0$@proceranetworks.com> <20141127140654.GA31381@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20141127140654.GA31381@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) On Thu, Nov 27, 2014 at 06:06:54AM -0800, H.J. Lu wrote: > On Wed, Nov 26, 2014 at 09:06:32PM -0800, H.J. Lu wrote: > > On Wed, Nov 26, 2014 at 9:39 AM, H.J. Lu wrote: > > > On Wed, Nov 26, 2014 at 9:07 AM, Joseph Myers wrote: > > >> On Wed, 26 Nov 2014, H.J. Lu wrote: > > >> > > >>> On Wed, Nov 26, 2014 at 8:23 AM, Joseph Myers wrote: > > >>> > On Wed, 26 Nov 2014, H.J. Lu wrote: > > >>> > > > >>> >> Is Paul's paperwork ready now? > > >>> > > > >>> > There is an assignment for past and future changes (dated 2014-5-13) and a > > >>> > disclaimer from Procera Networks for past changes (dated 2014-1-21). > > >>> > > > >>> > > >>> Can we move forward with Paul's fix? > > >> > > >> Someone needs to retest and resubmit it. It's so old it's not in > > >> patchwork (which only tracks patch submissions back to this March). > > >> > > > > > > I will rebase and submit a new patch with a testcase. > > > > Paul's fix is based on _dl_update_slotinfo in the same file. However, > > his patch failed to properly convert INSTALL_NEW_DTV to INSTALL_DTV. > > I believe this is the correct fix. I will submit the complete patch with a > > testcase later. > > > > > > Here is the promised patch. OK for trunk? > > Thanks. > > > H.J. > ---- > From b811f571b5604f84c84d5b96394e6b892e05d14f Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" > Date: Thu, 27 Nov 2014 05:42:23 -0800 > Subject: [PATCH] Reallocate DTV if the current DTV isn't big enough > > This patch reallocates DTV if the current DTV isn't big enough, using > the similar approach in _dl_update_slotinfo. Tested on X86-64, x32 > and ia32. > > [BZ #13862] > * elf/dl-tls.c (oom): Remove #ifdef SHARED/#endif. > (_dl_static_dtv, _dl_initial_dtv): Moved before ... > (_dl_allocate_tls_init): This. Reallocate DTV if the current > DTV isn't big enough. > * nptl/Makefile (tests): Add tst-stack4. > (modules-names): Add tst-stack4mod. > ($(objpfx)tst-stack4): New. > (tst-stack4mod.sos): Likewise. > ($(objpfx)tst-stack4.out): Likewise. > ($(tst-stack4mod.sos)): Likewise. > (clean): Likewise. > * nptl/tst-stack4.c: New file. > * nptl/tst-stack4mod.c: Likewise. Here is the updated patch to use test-skeleton.c in nptl/tst-stack4.c. H.J. --- From 220d769f14eccf1ef36862b294cffee5ae615a74 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 27 Nov 2014 05:42:23 -0800 Subject: [PATCH] Reallocate DTV if the current DTV isn't big enough This patch reallocates DTV if the current DTV isn't big enough, using the similar approach in _dl_update_slotinfo. Tested on X86-64, x32 and ia32. [BZ #13862] * elf/dl-tls.c (oom): Remove #ifdef SHARED/#endif. (_dl_static_dtv, _dl_initial_dtv): Moved before ... (_dl_allocate_tls_init): This. Reallocate DTV if the current DTV isn't big enough. * nptl/Makefile (tests): Add tst-stack4. (modules-names): Add tst-stack4mod. ($(objpfx)tst-stack4): New. (tst-stack4mod.sos): Likewise. ($(objpfx)tst-stack4.out): Likewise. ($(tst-stack4mod.sos)): Likewise. (clean): Likewise. * nptl/tst-stack4.c: New file. * nptl/tst-stack4mod.c: Likewise. --- ChangeLog | 17 +++++++ elf/dl-tls.c | 53 +++++++++++++++++--- nptl/Makefile | 17 ++++++- nptl/tst-stack4.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++ nptl/tst-stack4mod.c | 9 ++++ 5 files changed, 220 insertions(+), 9 deletions(-) create mode 100644 nptl/tst-stack4.c create mode 100644 nptl/tst-stack4mod.c diff --git a/ChangeLog b/ChangeLog index 92ffe16..e6474be 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2014-11-27 H.J. Lu + + [BZ #13862] + * elf/dl-tls.c (oom): Remove #ifdef SHARED/#endif. + (_dl_static_dtv, _dl_initial_dtv): Moved before ... + (_dl_allocate_tls_init): This. Reallocate DTV if the current + DTV isn't big enough. + * nptl/Makefile (tests): Add tst-stack4. + (modules-names): Add tst-stack4mod. + ($(objpfx)tst-stack4): New. + (tst-stack4mod.sos): Likewise. + ($(objpfx)tst-stack4.out): Likewise. + ($(tst-stack4mod.sos)): Likewise. + (clean): Likewise. + * nptl/tst-stack4.c: New file. + * nptl/tst-stack4mod.c: Likewise. + 2014-11-27 Stefan Liebler * nscd/connections.c: Include libc-internal.h because of macro diff --git a/elf/dl-tls.c b/elf/dl-tls.c index 5204fda..3142b16 100644 --- a/elf/dl-tls.c +++ b/elf/dl-tls.c @@ -34,14 +34,12 @@ /* Out-of-memory handler. */ -#ifdef SHARED static void __attribute__ ((__noreturn__)) oom (void) { _dl_fatal_printf ("cannot allocate memory for thread-local data: ABORT\n"); } -#endif size_t @@ -397,6 +395,11 @@ _dl_allocate_tls_storage (void) } +#ifndef SHARED +extern dtv_t _dl_static_dtv[]; +# define _dl_initial_dtv (&_dl_static_dtv[1]) +#endif + void * internal_function _dl_allocate_tls_init (void *result) @@ -410,6 +413,47 @@ _dl_allocate_tls_init (void *result) size_t total = 0; size_t maxgen = 0; + /* Check if the current dtv is big enough. */ + if (dtv[-1].counter < GL(dl_tls_max_dtv_idx)) + { + /* Reallocate the dtv. */ + dtv_t *newp; + size_t newsize = GL(dl_tls_max_dtv_idx) + DTV_SURPLUS; + size_t oldsize = dtv[-1].counter; + + if (dtv == GL(dl_initial_dtv)) + { + /* This is the initial dtv that was allocated + during rtld startup using the dl-minimal.c + malloc instead of the real malloc. We can't + free it, we have to abandon the old storage. */ + + newp = malloc ((2 + newsize) * sizeof (dtv_t)); + if (newp == NULL) + oom (); + memcpy (newp, &dtv[-1], (2 + oldsize) * sizeof (dtv_t)); + } + else + { + newp = realloc (&dtv[-1], + (2 + newsize) * sizeof (dtv_t)); + if (newp == NULL) + oom (); + } + + newp[0].counter = newsize; + + /* Clear the newly allocated part. */ + memset (newp + 2 + oldsize, '\0', + (newsize - oldsize) * sizeof (dtv_t)); + + /* Install this new dtv in the thread data structures. */ + INSTALL_DTV (result, newp); + + /* Point dtv to the generation counter. */ + dtv = &newp[1]; + } + /* We have to prepare the dtv for all currently loaded modules using TLS. For those which are dynamically loaded we add the values indicating deferred allocation. */ @@ -492,11 +536,6 @@ _dl_allocate_tls (void *mem) rtld_hidden_def (_dl_allocate_tls) -#ifndef SHARED -extern dtv_t _dl_static_dtv[]; -# define _dl_initial_dtv (&_dl_static_dtv[1]) -#endif - void internal_function _dl_deallocate_tls (void *tcb, bool dealloc_tcb) diff --git a/nptl/Makefile b/nptl/Makefile index 86a1b0c..ac76596 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -254,7 +254,7 @@ tests = tst-typesizes \ tst-exec1 tst-exec2 tst-exec3 tst-exec4 \ tst-exit1 tst-exit2 tst-exit3 \ tst-stdio1 tst-stdio2 \ - tst-stack1 tst-stack2 tst-stack3 tst-pthread-getattr \ + tst-stack1 tst-stack2 tst-stack3 tst-stack4 tst-pthread-getattr \ tst-pthread-attr-affinity \ tst-unload \ tst-dlsym1 \ @@ -311,7 +311,7 @@ endif 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-tls5modd tst-tls5mode tst-tls5modf tst-stack4mod \ tst-_res1mod1 tst-_res1mod2 tst-execstack-mod tst-fini1mod extra-test-objs += $(addsuffix .os,$(strip $(modules-names))) tst-cleanup4aux.o test-extras += $(modules-names) tst-cleanup4aux @@ -479,6 +479,19 @@ $(objpfx)tst-stack3-mem.out: $(objpfx)tst-stack3.out $(evaluate-test) generated += tst-stack3-mem.out tst-stack3.mtrace +$(objpfx)tst-stack4: $(libdl) $(shared-thread-library) +tst-stack4mod.sos=$(shell for i in 0 1 2 3 4 5 6 7 8 9 10 \ + 11 12 13 14 15 16 17 18 19; do \ + for j in 0 1 2 3 4 5 6 7 8 9 10 \ + 11 12 13 14 15 16 17 18 19; do \ + echo $(objpfx)tst-stack4mod-$$i-$$j.so; \ + done; done) +$(objpfx)tst-stack4.out: $(tst-stack4mod.sos) +$(tst-stack4mod.sos): $(objpfx)tst-stack4mod.so + cp -f $< $@ +clean: + rm -f $(tst-stack4mod.sos) + $(objpfx)tst-cleanup4: $(objpfx)tst-cleanup4aux.o $(shared-thread-library) $(objpfx)tst-cleanupx4: $(objpfx)tst-cleanup4aux.o $(shared-thread-library) diff --git a/nptl/tst-stack4.c b/nptl/tst-stack4.c new file mode 100644 index 0000000..7eda4d0 --- /dev/null +++ b/nptl/tst-stack4.c @@ -0,0 +1,133 @@ +#include +#include +#include +#include +#include + +#define DSO_SHARED_FILES 20 +#define DSO_OPEN_THREADS 20 +#define DSO_EXEC_THREADS 2 + +pthread_mutex_t g_lock; + +typedef void (*function) (void); + +void * +dso_invoke(void *dso_fun) +{ + function *fun_vec = (function *) dso_fun; + int dso; + + for (dso = 0; dso < DSO_SHARED_FILES; dso++) + (*fun_vec[dso]) (); + + pthread_exit (NULL); +} + +void * +dso_process (void * p) +{ + void *handle[DSO_SHARED_FILES]; + function fun_vec[DSO_SHARED_FILES]; + char dso_path[DSO_SHARED_FILES][100]; + int dso; + uintptr_t t = (uintptr_t) p; + + // Open dso's and get a function. + for (dso = 0; dso < DSO_SHARED_FILES; dso++) + { + sprintf (dso_path[dso], "tst-stack4mod-%i-%i.so", t, dso); + + pthread_mutex_lock (&g_lock); + + handle[dso] = dlopen (dso_path[dso], RTLD_NOW); + assert (handle[dso]); + + fun_vec[dso] = (function) dlsym (handle[dso], "function"); + assert (fun_vec[dso]); + + pthread_mutex_unlock (&g_lock); + } + + // Spawn workers + pthread_t thread[DSO_EXEC_THREADS]; + int i, ret; + uintptr_t result = 0; + for (i = 0; i < DSO_EXEC_THREADS; i++) + { + pthread_mutex_lock (&g_lock); + ret = pthread_create (&thread[i], NULL, dso_invoke, (void *) fun_vec); + if (ret != 0) + { + printf ("pthread_create failed: %d\n", ret); + result = 1; + } + pthread_mutex_unlock (&g_lock); + } + + if (!result) + for (i = 0; i < DSO_EXEC_THREADS; i++) + { + ret = pthread_join (thread[i], NULL); + if (ret != 0) + { + printf ("pthread_join failed: %d\n", ret); + result = 1; + } + } + + // Close all dso's + for (dso = 0; dso < DSO_SHARED_FILES; dso++) + { + pthread_mutex_lock (&g_lock); + dlclose (handle[dso]); + pthread_mutex_unlock (&g_lock); + } + + // Exit + pthread_exit ((void *) result); +} + +static int +do_test (void) +{ + pthread_t thread[DSO_OPEN_THREADS]; + int i,j; + int ret; + int result = 0; + + pthread_mutex_init (&g_lock, NULL); + + for (j = 0; j < 100; j++) + { + for (i = 0; i < DSO_OPEN_THREADS; i++) + { + ret = pthread_create (&thread[i], NULL, dso_process, + (void *) (uintptr_t) i); + if (ret != 0) + { + printf ("pthread_create failed: %d\n", ret); + result = 1; + } + } + + if (result) + break; + + for (i = 0; i < DSO_OPEN_THREADS; i++) + { + ret = pthread_join (thread[i], NULL); + if (ret != 0) + { + printf ("pthread_join failed: %d\n", ret); + result = 1; + } + } + } + + return result; +} + +#define TEST_FUNCTION do_test () +#define TIMEOUT 100 +#include "../test-skeleton.c" diff --git a/nptl/tst-stack4mod.c b/nptl/tst-stack4mod.c new file mode 100644 index 0000000..91d0ae3 --- /dev/null +++ b/nptl/tst-stack4mod.c @@ -0,0 +1,9 @@ +__thread int var[256] attribute_hidden = {0}; + +void +function (void) +{ + int i; + for (i = 0; i < 256; i++) + var[i] = i; +}