Re: [PATCH] fix page leak during core dump

Hugh Dickins
2007-03-30T23:07:08Z

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;