From owner-freebsd-threads@FreeBSD.ORG Fri Aug 29 22:10:05 2008 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8B7B9106567F for ; Fri, 29 Aug 2008 22:10:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 7CC5C8FC19 for ; Fri, 29 Aug 2008 22:10:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.2/8.14.2) with ESMTP id m7TMA5jL019710 for ; Fri, 29 Aug 2008 22:10:05 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.2/8.14.1/Submit) id m7TMA53f019709; Fri, 29 Aug 2008 22:10:05 GMT (envelope-from gnats) Date: Fri, 29 Aug 2008 22:10:05 GMT Message-Id: <200808292210.m7TMA53f019709@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: "Kip Macy" Cc: Subject: Re: threads/126950: rtld malloc is thread-unsafe X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Kip Macy List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Aug 2008 22:10:05 -0000 The following reply was made to PR threads/126950; it has been noted by GNATS. From: "Kip Macy" To: "Oleg Dolgov" Cc: freebsd-gnats-submit@freebsd.org Subject: Re: threads/126950: rtld malloc is thread-unsafe Date: Fri, 29 Aug 2008 14:41:59 -0700 I think the real solution is to serialize calls to dlopen. Please consider submitting a patch for that. -Kip On Fri, Aug 29, 2008 at 7:48 AM, Oleg Dolgov wrote: > >>Number: 126950 >>Category: threads >>Synopsis: rtld malloc is thread-unsafe >>Confidential: no >>Severity: serious >>Priority: medium >>Responsible: freebsd-threads >>State: open >>Quarter: >>Keywords: >>Date-Required: >>Class: sw-bug >>Submitter-Id: current-users >>Arrival-Date: Fri Aug 29 14:50:08 UTC 2008 >>Closed-Date: >>Last-Modified: >>Originator: Oleg Dolgov >>Release: 7.0-RELEASE >>Organization: > Sunbay >>Environment: >>Description: > rtld internal memory allocator are not thread safe. It use global array 'nextf' of free blocks. Dynamic loader can be easily segfaulted with parallel invocations of rtld operations from multiple threads. Test case and fix for problems exposed by the test are attached. > > precondition: apply patch kern/95339 (fixes for dlopen mt behavior) > >>How-To-Repeat: > Use the following test by Kazuaki Oda > to see errant behaviour, but first apply patch from kern/95339. > > #include > #include > #include > #include > #include > #include > > #define NTHREADS 10 > > void *func(void *dummy); > > int main(void) > { > pthread_t tids[NTHREADS]; > int error; > int i; > > for (i = 0; i < NTHREADS; i++) { > error = pthread_create(&tids[i], NULL, func, NULL); > if (error) > errc(1, error, "pthread_create"); > } > > for (;;) > sleep(1); > > /* NOTREACHED */ > > exit(0); > } > > void *func(void *dummy) > { > void *h; > unsigned long c = 0; > > for (;;) { > if ((h = dlopen("/usr/lib/libm.so", RTLD_NOW)) == NULL) { > fprintf(stderr, "dlopen: %s\n", dlerror()); > continue; > } > if (dlclose(h) == -1) > fprintf(stderr, "dlclose: %s\n", dlerror()); > if (c++ == 10000) { > printf(".\n"); > c = 0; > } > } > > /* NOTREACHED */ > > return (NULL); > } >>Fix: > We need synchronization, but cant use rtld_bind_lock or similar because memory allocation even occurred from _rtld (init_rtld) function (and lock's space also allocated with malloc), in context, where dynamic linker itself has not been relocated yet. > > > Patch attached with submission follows: > > --- rtld-elf/malloc.c 2008-08-29 16:18:17.000000000 +0300 > +++ /usr/src/libexec/rtld-elf/malloc.c 2008-08-29 17:42:38.000000000 +0300 > @@ -58,6 +58,8 @@ > #include > #include > #include > +#include > + > #ifndef BSD > #define MAP_COPY MAP_PRIVATE > #define MAP_FILE 0 > @@ -68,6 +70,10 @@ > #define NEED_DEV_ZERO 1 > #endif > > +static volatile u_int mem_mtx = 0; > +static void mtx_acquire(); > +static void mtx_release(); > + > static void morecore(); > static int findbucket(); > > @@ -152,8 +158,8 @@ > static void xprintf(const char *, ...); > #define TRACE() xprintf("TRACE %s:%d\n", __FILE__, __LINE__) > > -void * > -malloc(nbytes) > +static void * > +__malloc(nbytes) > size_t nbytes; > { > register union overhead *op; > @@ -299,8 +305,8 @@ > } > } > > -void > -free(cp) > +static void > +__free(cp) > void *cp; > { > register int size; > @@ -341,8 +347,8 @@ > */ > int realloc_srchlen = 4; /* 4 should be plenty, -1 =>'s whole list */ > > -void * > -realloc(cp, nbytes) > +static void * > +__realloc(cp, nbytes) > void *cp; > size_t nbytes; > { > @@ -516,3 +522,49 @@ > (void)write(STDOUT_FILENO, buf, strlen(buf)); > va_end(ap); > } > + > +/* > + * Thread safe allocator > + */ > + > +static void mtx_acquire() > +{ > + for ( ; ; ) { > + if (atomic_cmpset_acq_int(&mem_mtx, 0, 1)) > + break; > + } > +} > + > +static void mtx_release() > +{ > + atomic_add_rel_int(&mem_mtx, -1); > +} > + > +void * > +malloc(size_t nbytes) > +{ > + void *p; > + mtx_acquire(); > + p = __malloc(nbytes); > + mtx_release(); > + return p; > +} > + > +void * > +realloc(void *cp, size_t nbytes) > +{ > + void *p; > + mtx_acquire(); > + p = __realloc(cp, nbytes); > + mtx_release(); > + return p; > +} > + > +void > +free(void *cp) > +{ > + mtx_acquire(); > + __free(cp); > + mtx_release(); > +} > + > > >>Release-Note: >>Audit-Trail: >>Unformatted: > _______________________________________________ > freebsd-threads@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-threads > To unsubscribe, send any mail to "freebsd-threads-unsubscribe@freebsd.org" >