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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to