bugs in malloc

Freeman Wang xwang at ubicom.com
Tue Nov 24 07:16:47 UTC 2009


Mike

An example may be better to show what we thought.

Say, we already have a mmb list of three blocks M1 --> M2 --> M3. Now a
heap request comes in for an 8k buffer. The heap is extended using mmap
and we iterate through the list and find the new block descriptor,
new_mmb, should be added after M3. *** Now we try to allocate new_mmb
from the mmb_heap and find mmb_heap needs to be extended too *** So a
new mmap syscall is made to extend the mmb_heap, and again the new block
needs a descriptor also from the mmb_heap. Again we iterated through the
existing list, and find this new_mmb_2 should be added after M3 too !!!.
We then try to allocate new_mmb_2 and it should succeed because the mmap
usually gives us at least a page and it has been added to the mmb_heap.
When the allocation of the first new_mmb returns, the list has already
been updated to M1 --> M2 --> M3 --> M4_2, but we do not know, so M4_1
will be still added after M3.

That's just one of the possible ways the current code could mess up.
Depending on where the two blocks are located, things could go wild and
the link list be totally destroyed. However, if we make the allocation
before going through the mmb list, we will be able to make sure the
new_mmb structure is added properly.

Freeman

-----Original Message-----
From: Mike Frysinger [mailto:vapier at gentoo.org] 
Sent: Monday, November 23, 2009 7:42 PM
To: uclibc at uclibc.org
Cc: Freeman Wang
Subject: Re: bugs in malloc

On Monday 23 November 2009 14:55:26 Freeman Wang wrote:
> 1. We found with some special application, the application would get 
> stuck at line 162 of malloc.c and the reason was mem->next points back

> to itself.

please try to reduce the allocation patterns of your 'special'
application.  
it should be easy to enable debugging and capture the malloc/free
sequences and run them again manually.

> It turns out, we believe, to be because new_mmb is allocated after the

> mmb list is iterated throught to find the insertion point. When the 
> mmb_heap also runs out and needs to be extended when the regular heap 
> is just extended, the mmb list could be messed up. We moved the 
> new_mmb allocation up and the problem seems to have been fixed.

i dont see why the current code is a problem.  it's a singly linked list
which means if the list is walked to the end, the new_mmb will be
'inserted' as the last item in the linked list.  prev_mmb points to the
last valid entry in the list and mmb is null.  so the last valid entry
will be updated to point to new_mmb and it will have its next member set
to null.  i dont see any place where the mmb list 'could be messed up'.

if you look a few lines up, the recursive memory-full-issue should
already be handled because a mmap for more memory is done, and that mmap
is put into the heap by the heap free call.

> 2. While trying to fix the above issue, we read the code and found a 
> multi-threading issue with this mmb list handling. Th'is list is 
> halfway protected in free.c and not protected by any lock at all in 
> malloc.c. Is it intentional?

looks like the locking fixes we have in the blackfin tree werent pushed
upstream.  i'll have to rebase them first, but it should at least
partially cover what you see.  if it doesnt, i'll stitch in your pieces.

> 3. In an embedded world without MMU, it is not garanteed that the mmap

> syscall would always get back a valid block, and that's probably why 
> the return value, block, is checked immediately after the syscall. But

> it seems we are not checking the return value of new_mmb which is 
> allocated from the mmb_heap? Is it a potential issue?

you have no guarantee of mmap returning valid memory under a mmu-system
either.  typically an oom situation will have an application crash
quickly, so this particular missing check isnt a big deal, but it should
probably still be added.  i imagine in a threaded situation, one thread
could grab the fresh memory before the original thread got a chance to
use it and thus got null back.
-mike


More information about the uClibc mailing list