Null pointer dereference at free_vm_area()

Hi folks,

The free_vm_area() function may crash if it gets a NULL pointer as a parameter. I do think that the right behaviour should be returning in this case. This is, for example, the same behaviour of kfree(), and as alloc_vm_area() may also return NULL, it may lead to a more elegant alloc/free sequence in case of a fail.

In case you agree with that, a patch follows.

glommer

diff -r 829517be689f linux-2.6-xen-sparse/drivers/xen/util.c
@@ -35,6 +35,8 @@
 void free_vm_area(struct vm_struct *area)
 {
 	struct vm_struct *ret;
+	if (!area)
+		return;
 	ret = remove_vm_area(area->addr);
 	BUG_ON(ret != area);
 	kfree(area);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Hi,

I quickly look around where free_vm_area is called, and I cannot see any codepath that could benefit such a code cleanup nor find any that could lead to a NULL pointer pass to it.

I think you should provide a use for this patch if you want it to be applied.

Thanks,

Vincent Hanquez

I think it's more stylish than functional nowadays. In a situations in which we call alloc_vm_area() more than once, and test for the return value of them all in one shot. It can maybe lead to a cleaner code, as shown in the pseudocode bellow.

a1 = alloc_vm_area()
a2 = alloc_vm_area()

if (!a1 || !a2){
	free_vm_area(a1);
	free_vm_area(a2);
	return;
}

Instead of:

a1 = alloc_vm_area();
if (!a1)

	return;
a2 = alloc_vm_area()
if (!a2){
	free_vm_area(a1);
	return;
}

But of course, it's mainly a matter of opinion.

glommer