From owner-freebsd-threads@FreeBSD.ORG Fri Aug 29 14:50:15 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 0AD681065758 for ; Fri, 29 Aug 2008 14:50:14 +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 15B548FC19 for ; Fri, 29 Aug 2008 14:50:09 +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 m7TEo8IJ059865 for ; Fri, 29 Aug 2008 14:50:08 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.2/8.14.1/Submit) id m7TEo80c059864; Fri, 29 Aug 2008 14:50:08 GMT (envelope-from gnats) Resent-Date: Fri, 29 Aug 2008 14:50:08 GMT Resent-Message-Id: <200808291450.m7TEo80c059864@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-threads@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Oleg Dolgov Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CFF2E1065672 for ; Fri, 29 Aug 2008 14:48:26 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id BAF038FC1D for ; Fri, 29 Aug 2008 14:48:26 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.2/8.14.2) with ESMTP id m7TEmP8m020627 for ; Fri, 29 Aug 2008 14:48:25 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.2/8.14.1/Submit) id m7TEmPbg020616; Fri, 29 Aug 2008 14:48:25 GMT (envelope-from nobody) Message-Id: <200808291448.m7TEmPbg020616@www.freebsd.org> Date: Fri, 29 Aug 2008 14:48:25 GMT From: Oleg Dolgov To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: threads/126950: rtld malloc is thread-unsafe X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Aug 2008 14:50:15 -0000 >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: