[malloc] Use list_for_each_entry_safe() when we may delete a list entry

free_memblock() currently uses list_for_each_entry() to iterate over
the free list, and may delete an entry over which it iterates.  While
there is no way that the deleted list entry could be overwritten
before we reference it, this does rely upon list_del() leaving the
"next" pointer intact, which is not guaranteed.  Discovered while
tracking down a list-corruption bug (as a result of having modified
list_del() to sanitise the deleted list entry).

Fix by using list_for_each_entry_safe().

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/core/malloc.c b/src/core/malloc.c
index cf33af46ca56cbc6ef5f567dcbf87ffde8c89167..d317ce179c035dbc96c61f73809e74a986b73d89 100644 (file)
--- a/src/core/malloc.c
+++ b/src/core/malloc.c
@@ -196,6 +196,7 @@ void * alloc_memblock ( size_t size, size_t align ) {
 void free_memblock ( void *ptr, size_t size ) {
        struct memory_block *freeing;
        struct memory_block *block;
+       struct memory_block *tmp;
        ssize_t gap_before;
        ssize_t gap_after = -1;
 
@@ -212,7 +213,7 @@ void free_memblock ( void *ptr, size_t size ) {
        DBG ( "Freeing [%p,%p)\n", freeing, ( ( ( void * ) freeing ) + size ));
 
        /* Insert/merge into free list */
-       list_for_each_entry ( block, &free_blocks, list ) {
+       list_for_each_entry_safe ( block, tmp, &free_blocks, list ) {
                /* Calculate gaps before and after the "freeing" block */
                gap_before = ( ( ( void * ) freeing ) - 
                               ( ( ( void * ) block ) + block->size ) );