mm: vmalloc: Check for page allocation failure before vmlist insertion

PATCH In review Mel Gorman 3 participants 3 messages Dec 5, 2011 – Dec 6, 2011

Subsystems (2)

Memory management vmalloc

lore

Share & tools

lore

Mel Gorman

author

2011-12-05 14:07 UTC #0 lore raw

PATCH 1/1 mm: vmalloc: Check for page allocation failure before vmlist insertion

Commit [f5252e00: mm: avoid null pointer access in vm_struct via /proc/vmallocinfo] adds newly allocated vm_structs to the vmlist after it is fully initialised. Unfortunately, it did not check that __vmalloc_area_node() successfully populated the area. In the event of allocation failure, the vmalloc area is freed but the pointer to freed memory is inserted into the vmlist leading to a a crash later in get_vmalloc_info().

This patch adds a check for ____vmalloc_area_node() failure within __vmalloc_node_range. It does not use "goto fail" as in the previous error path as a warning was already displayed by __vmalloc_area_node() before it called vfree in its failure path.

Credit goes to Luciano Chavez for doing all the real work of identifying exactly where the problem was.

If accepted, this should be considered a -stable candidate.

Reported-and-tested-by Luciano Chavez <[email protected]>

Signed-off-by Mel Gorman <[email protected]>

---
 mm/vmalloc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

mm/vmalloc.c +2 -0

index 3231bf3..1d8b32f 100644
@@ -1633,6 +1633,8 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align,
 		goto fail;
 
 	addr = __vmalloc_area_node(area, gfp_mask, prot, node, caller);
+	if (!addr)
+		return NULL;
 
 	/*
 	 * In this function, newly allocated vm_struct is not added

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Rik van Riel

Reviewed-by on v1 1/1

2011-12-06 06:48 UTC #1 lore raw

Quoted 13 lines · Mel Gorman

Commit [f5252e00: mm: avoid null pointer access in vm_struct via /proc/vmallocinfo] adds newly allocated vm_structs to the vmlist after it is fully initialised. Unfortunately, it did not check that __vmalloc_area_node() successfully populated the area. In the event of allocation failure, the vmalloc area is freed but the pointer to freed memory is inserted into the vmlist leading to a a crash later in get_vmalloc_info().
This patch adds a check for ____vmalloc_area_node() failure within __vmalloc_node_range. It does not use "goto fail" as in the previous error path as a warning was already displayed by __vmalloc_area_node() before it called vfree in its failure path.

Quoted 7 lines

Credit goes to Luciano Chavez for doing all the real work of identifying exactly where the problem was.
If accepted, this should be considered a -stable candidate.
Reported-and-tested-by Luciano Chavez<[email protected]>
Signed-off-by Mel Gorman<[email protected]>

Reviewed-by Rik van Riel <[email protected]>

David Rientjes

Acked-by on v1 1/1

2011-12-06 20:26 UTC #2 lore raw

In reply to Mel Gorman

Quoted 13 lines · Mel Gorman

Commit [f5252e00: mm: avoid null pointer access in vm_struct via /proc/vmallocinfo] adds newly allocated vm_structs to the vmlist after it is fully initialised. Unfortunately, it did not check that __vmalloc_area_node() successfully populated the area. In the event of allocation failure, the vmalloc area is freed but the pointer to freed memory is inserted into the vmlist leading to a a crash later in get_vmalloc_info().
This patch adds a check for ____vmalloc_area_node() failure within __vmalloc_node_range. It does not use "goto fail" as in the previous error path as a warning was already displayed by __vmalloc_area_node() before it called vfree in its failure path.

Quoted 5 lines

Credit goes to Luciano Chavez for doing all the real work of identifying exactly where the problem was.
If accepted, this should be considered a -stable candidate.

Right, for 3.1.x.

Quoted 2 lines

Reported-and-tested-by Luciano Chavez <[email protected]>
Signed-off-by Mel Gorman <[email protected]>

Acked-by David Rientjes <[email protected]>