[PATCH] Null pointer dereference at free_vm_area()

[PATCH] Null pointer dereference at free_vm_area()

[Glauber de Oliveira Costa]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

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.

Signed-off-by: Glauber de Oliveira Costa <glommer@br.ibm.com>

-- 
glommer

[-- Attachment #2: free_return_null --]
[-- Type: text/plain, Size: 390 bytes --]

diff -r 829517be689f linux-2.6-xen-sparse/drivers/xen/util.c
--- a/linux-2.6-xen-sparse/drivers/xen/util.c	Fri Dec 23 15:42:46 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/util.c	Mon Dec 26 15:47:50 2005
@@ -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);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Null pointer dereference at free_vm_area()

[Vincent Hanquez]

On Mon, Dec 26, 2005 at 01:58:57PM -0200, Glauber de Oliveira Costa wrote:
> 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.

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

Re: [PATCH] Null pointer dereference at free_vm_area()

[Glauber de Oliveira Costa]

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