[PATCH] fix memory leak while coredumping

[PATCH] fix memory leak while coredumping

[IWAMOTO Toshihiro]

Hi,

with some help of coworker, I found a bug in binfmt_elf.c.
The bug exists in linux-2.6.1 and linux-2.6.2-rc2-mm2.

This patch fixes a memory leak that happens when a core file hits the
process's resource limit.
I've tested the DUMP_WRITE case only a little, and the DUMP_SEEK case
is only compile tested.

--- old/fs/binfmt_elf.c	Fri Jan 16 12:12:24 2004
+++ new/fs/binfmt_elf.c	Mon Feb  2 19:31:42 2004
@@ -1441,12 +1441,22 @@
 				DUMP_SEEK (file->f_pos + PAGE_SIZE);
 			} else {
 				if (page == ZERO_PAGE(addr)) {
-					DUMP_SEEK (file->f_pos + PAGE_SIZE);
+					if (!dump_seek(file,
+					    file->f_pos + PAGE_SIZE)) {
+						page_cache_release(page);
+						goto end_coredump;
+					}
 				} else {
 					void *kaddr;
 					flush_cache_page(vma, addr);
 					kaddr = kmap(page);
-					DUMP_WRITE(kaddr, PAGE_SIZE);
+					if ((size += PAGE_SIZE) > limit ||
+					    !dump_write(file, kaddr,
+					    PAGE_SIZE)) {
+						kunmap(page);
+						page_cache_release(page);
+						goto end_coredump;
+					}
 					kunmap(page);
 				}
 				page_cache_release(page);

Re: [PATCH] fix memory leak while coredumping

[Andrew Morton]

IWAMOTO Toshihiro <iwamoto@valinux.co.jp> wrote:
>
> Hi,
> 
> with some help of coworker, I found a bug in binfmt_elf.c.
> The bug exists in linux-2.6.1 and linux-2.6.2-rc2-mm2.
> 
> This patch fixes a memory leak that happens when a core file hits the
> process's resource limit.
> I've tested the DUMP_WRITE case only a little, and the DUMP_SEEK case
> is only compile tested.
> 
> --- old/fs/binfmt_elf.c	Fri Jan 16 12:12:24 2004
> +++ new/fs/binfmt_elf.c	Mon Feb  2 19:31:42 2004
> @@ -1441,12 +1441,22 @@
>  				DUMP_SEEK (file->f_pos + PAGE_SIZE);
>  			} else {
>  				if (page == ZERO_PAGE(addr)) {
> -					DUMP_SEEK (file->f_pos + PAGE_SIZE);
> +					if (!dump_seek(file,
> +					    file->f_pos + PAGE_SIZE)) {
> +						page_cache_release(page);
> +						goto end_coredump;
> +					}

There's no point in doing page_cache_release() of the zero page.

>  				} else {
>  					void *kaddr;
>  					flush_cache_page(vma, addr);
>  					kaddr = kmap(page);
> -					DUMP_WRITE(kaddr, PAGE_SIZE);
> +					if ((size += PAGE_SIZE) > limit ||
> +					    !dump_write(file, kaddr,
> +					    PAGE_SIZE)) {
> +						kunmap(page);
> +						page_cache_release(page);
> +						goto end_coredump;
> +					}
>  					kunmap(page);
>  				}
>  				page_cache_release(page);

This chunk is good, thanks.

Those macros are foul.