2002-11-18 23:27:37

by Nathan Scott

Subject: Memory leak in 2.4 vmalloc.c get_vm_area

hi Alan,

I noticed you recently merged this patch with Marcelo in the
2.4 BK tree (lists you as author, and annotation says it came
from DaveM originally)...

--- 1.10/mm/vmalloc.c Tue Feb 5 06:10:20 2002
+++ 1.11/mm/vmalloc.c Thu Sep 5 05:22:42 2002
@@ -177,6 +177,8 @@
if (!area)
return NULL;
size += PAGE_SIZE;
+ if(!size)
+ return NULL;
addr = VMALLOC_START;
write_lock(&vmlist_lock);
for (p = &vmlist; (tmp = *p) ; p = &tmp->next) {


This looks to me like it introduces a memory leak in the new !size
case - either the "size" bump and test needs to be moved before the
"area" kmalloc, or we need to kfree(area) before returning NULL.

If you like, I'll make a (trivial) patch to do one of these?

cheers.

--
Nathan

2002-11-19 12:17:08

by Dave Jones

Subject: Re: Memory leak in 2.4 vmalloc.c get_vm_area

On Tue, Nov 19, 2002 at 10:32:02AM +1100, Nathan Scott wrote:
> hi Alan,
>
> I noticed you recently merged this patch with Marcelo in the
> 2.4 BK tree (lists you as author, and annotation says it came
> from DaveM originally)...
>
> --- 1.10/mm/vmalloc.c Tue Feb 5 06:10:20 2002
> +++ 1.11/mm/vmalloc.c Thu Sep 5 05:22:42 2002
> @@ -177,6 +177,8 @@
> if (!area)
> return NULL;
> size += PAGE_SIZE;
> + if(!size)
> + return NULL;
> addr = VMALLOC_START;
> write_lock(&vmlist_lock);
> for (p = &vmlist; (tmp = *p) ; p = &tmp->next) {
>
>
> This looks to me like it introduces a memory leak in the new !size
> case - either the "size" bump and test needs to be moved before the
> "area" kmalloc, or we need to kfree(area) before returning NULL.
>
> If you like, I'll make a (trivial) patch to do one of these?

Correct diagnosis. Patch went to Marcelo a while back.
(Which I thought he took). Alan already picked it up iirc.

Will retransmit, as this is -rc material IMO.

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs