Date: Fri, 29 Aug 2008 14:48:25 GMT From: Oleg Dolgov <agile@sunbay.com> To: freebsd-gnats-submit@FreeBSD.org Subject: threads/126950: rtld malloc is thread-unsafe Message-ID: <200808291448.m7TEmPbg020616@www.freebsd.org> Resent-Message-ID: <200808291450.m7TEo80c059864@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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 <kaakun at highway dot ne dot jp>
to see errant behaviour, but first apply patch from kern/95339.
#include <err.h>
#include <dlfcn.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#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 <unistd.h>
#include <sys/param.h>
#include <sys/mman.h>
+#include <machine/atomic.h>
+
#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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808291448.m7TEmPbg020616>