[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>

1 files changed, 2 insertions, 1 deletions

diff --git a/src/core/malloc.c b/src/core/malloc.c
index cf33af46..d317ce17 100644
--- 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 ) );