Browse Source

[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>
tags/v1.20.1
Michael Brown 13 years ago
parent
commit
fc69ab94d9
1 changed files with 2 additions and 1 deletions
  1. 2
    1
      src/core/malloc.c

+ 2
- 1
src/core/malloc.c View File

@@ -196,6 +196,7 @@ void * alloc_memblock ( size_t size, size_t align ) {
196 196
 void free_memblock ( void *ptr, size_t size ) {
197 197
 	struct memory_block *freeing;
198 198
 	struct memory_block *block;
199
+	struct memory_block *tmp;
199 200
 	ssize_t gap_before;
200 201
 	ssize_t gap_after = -1;
201 202
 
@@ -212,7 +213,7 @@ void free_memblock ( void *ptr, size_t size ) {
212 213
 	DBG ( "Freeing [%p,%p)\n", freeing, ( ( ( void * ) freeing ) + size ));
213 214
 
214 215
 	/* Insert/merge into free list */
215
-	list_for_each_entry ( block, &free_blocks, list ) {
216
+	list_for_each_entry_safe ( block, tmp, &free_blocks, list ) {
216 217
 		/* Calculate gaps before and after the "freeing" block */
217 218
 		gap_before = ( ( ( void * ) freeing ) - 
218 219
 			       ( ( ( void * ) block ) + block->size ) );

Loading…
Cancel
Save