Date: Tue, 26 Jan 2010 16:34:50 -0500 From: Ryan Stone <rysto32@gmail.com> To: freebsd-current@freebsd.org Cc: jasone@freebsd.org Subject: Missing return in malloc.c:arena_dalloc? Message-ID: <bc2d971001261334he63ae8fxe203887e63bc5c49@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
I think that there is a bug in the magazine allocator in jemalloc that
can cause a segfault in certain rare conditions. It looks to me like
a return is missing:
*** malloc.c (revision 203047)
--- malloc.c (working copy)
***************
*** 3793,3812 ****
--- 3793,3813 ----
#ifdef MALLOC_MAG
if (__isthreaded && opt_mag) {
mag_rack_t *rack = mag_rack;
if (rack == NULL) {
rack = mag_rack_create(arena);
if (rack == NULL) {
malloc_spin_lock(&arena->lock);
arena_dalloc_small(arena, chunk, ptr,
mapelm);
malloc_spin_unlock(&arena->lock);
+ return;
}
mag_rack = rack;
}
mag_rack_dalloc(rack, ptr);
} else {
#endif
malloc_spin_lock(&arena->lock);
arena_dalloc_small(arena, chunk, ptr, mapelm);
malloc_spin_unlock(&arena->lock);
#ifdef MALLOC_MAG
The logic here seems to be, "if the magazine option is enabled, try to
get the magazine rack out of the TLS variable mag_rack. If we haven't
yet allocated a rack for this thread, try to create one with
mag_rack_create. If we fail to allocate the rack, then free the
memory directly to the arena. Currently, after freeing to the arena
the code falls-through and tries to free the memory to a NULL rack,
which can't possibly be right. mag_rack_dalloc definitely can't
handle a NULL rack. It seems to me that the intent here is to return
after calling arena_dalloc_small? There's a similar case in
mag_rack_dalloc where it tries to allocate a magazine, and if it
fails, it calls arena_dalloc_small and then returns.
Note that gmail has mangled the tabs in the diff I pasted above, so
I'm attaching a correct version as a patch.
[-- Attachment #2 --]
Index: malloc.c
===================================================================
*** malloc.c (revision 203047)
--- malloc.c (working copy)
***************
*** 3793,3812 ****
--- 3793,3813 ----
#ifdef MALLOC_MAG
if (__isthreaded && opt_mag) {
mag_rack_t *rack = mag_rack;
if (rack == NULL) {
rack = mag_rack_create(arena);
if (rack == NULL) {
malloc_spin_lock(&arena->lock);
arena_dalloc_small(arena, chunk, ptr,
mapelm);
malloc_spin_unlock(&arena->lock);
+ return;
}
mag_rack = rack;
}
mag_rack_dalloc(rack, ptr);
} else {
#endif
malloc_spin_lock(&arena->lock);
arena_dalloc_small(arena, chunk, ptr, mapelm);
malloc_spin_unlock(&arena->lock);
#ifdef MALLOC_MAG
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bc2d971001261334he63ae8fxe203887e63bc5c49>