Re: [PATCH] fix page leak during core dump

[Hugh Dickins]

On Fri, 30 Mar 2007, Andrew Morton wrote:
> 
> <would anyone be interested in hearing my opinion on the DUMP_SEEK macro
> again?>

Oooh, yes please.

> diff -puN fs/binfmt_elf_fdpic.c~fix-page-leak-during-core-dump fs/binfmt_elf_fdpic.c
> --- a/fs/binfmt_elf_fdpic.c~fix-page-leak-during-core-dump
> +++ a/fs/binfmt_elf_fdpic.c
> @@ -1480,8 +1480,10 @@ static int elf_fdpic_dump_segments(struc
>  				DUMP_SEEK(file->f_pos + PAGE_SIZE);
>  			}
>  			else if (page == ZERO_PAGE(addr)) {
> -				DUMP_SEEK(file->f_pos + PAGE_SIZE);
> -				page_cache_release(page);
> +				if (!dump_seek(file, file->f_pos + PAGE_SIZE)) {
> +					page_cache_release(page);
> +					return 0;
> +				}
>  			}
>  			else {
>  				void *kaddr;
> _

No, I think that's wrong: whereas the binfmt_elf one did its
page_cache_release down below at the bottom of the block, this
version does it in each subblock, so there you're removing the
dump_seek success one.  Can't we preserve that beauteous macro
here and just do...

--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1480,8 +1480,8 @@ static int elf_fdpic_dump_segments(struc
 				DUMP_SEEK(file->f_pos + PAGE_SIZE);
 			}
 			else if (page == ZERO_PAGE(addr)) {
-				DUMP_SEEK(file->f_pos + PAGE_SIZE);
 				page_cache_release(page);
+				DUMP_SEEK(file->f_pos + PAGE_SIZE);
 			}
 			else {
 				void *kaddr;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/