Skip site navigation (1)Skip section navigation (2)
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
--001485e9a6c647fa15047e180e11
Content-Type: text/plain; charset=ISO-8859-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.

--001485e9a6c647fa15047e180e11
Content-Type: application/octet-stream; name="malloc.diff"
Content-Disposition: attachment; filename="malloc.diff"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_g4x7g2qa0

SW5kZXg6IG1hbGxvYy5jDQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09DQoqKiogbWFsbG9jLmMJKHJldmlzaW9uIDIwMzA0
NykKLS0tIG1hbGxvYy5jCSh3b3JraW5nIGNvcHkpCioqKioqKioqKioqKioqKgoqKiogMzc5Mywz
ODEyICoqKioKLS0tIDM3OTMsMzgxMyAtLS0tCiAgI2lmZGVmIE1BTExPQ19NQUcKICAJCWlmIChf
X2lzdGhyZWFkZWQgJiYgb3B0X21hZykgewogIAkJCW1hZ19yYWNrX3QgKnJhY2sgPSBtYWdfcmFj
azsKICAJCQlpZiAocmFjayA9PSBOVUxMKSB7CiAgCQkJCXJhY2sgPSBtYWdfcmFja19jcmVhdGUo
YXJlbmEpOwogIAkJCQlpZiAocmFjayA9PSBOVUxMKSB7CiAgCQkJCQltYWxsb2Nfc3Bpbl9sb2Nr
KCZhcmVuYS0+bG9jayk7CiAgCQkJCQlhcmVuYV9kYWxsb2Nfc21hbGwoYXJlbmEsIGNodW5rLCBw
dHIsCiAgCQkJCQkgICAgbWFwZWxtKTsKICAJCQkJCW1hbGxvY19zcGluX3VubG9jaygmYXJlbmEt
PmxvY2spOworIAkJCQkJcmV0dXJuOwogIAkJCQl9CiAgCQkJCW1hZ19yYWNrID0gcmFjazsKICAJ
CQl9CiAgCQkJbWFnX3JhY2tfZGFsbG9jKHJhY2ssIHB0cik7CiAgCQl9IGVsc2UgewogICNlbmRp
ZgogIAkJCW1hbGxvY19zcGluX2xvY2soJmFyZW5hLT5sb2NrKTsKICAJCQlhcmVuYV9kYWxsb2Nf
c21hbGwoYXJlbmEsIGNodW5rLCBwdHIsIG1hcGVsbSk7CiAgCQkJbWFsbG9jX3NwaW5fdW5sb2Nr
KCZhcmVuYS0+bG9jayk7CiAgI2lmZGVmIE1BTExPQ19NQUcK
--001485e9a6c647fa15047e180e11--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bc2d971001261334he63ae8fxe203887e63bc5c49>