Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753333Ab0AHCN1 (ORCPT ); Thu, 7 Jan 2010 21:13:27 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753295Ab0AHCN0 (ORCPT ); Thu, 7 Jan 2010 21:13:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17055 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753195Ab0AHCNZ (ORCPT ); Thu, 7 Jan 2010 21:13:25 -0500 Message-ID: <4B469511.1030402@redhat.com> Date: Thu, 07 Jan 2010 21:14:41 -0500 From: Masami Hiramatsu User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091209 Fedora/3.0-3.fc11 Thunderbird/3.0 MIME-Version: 1.0 To: Andrew Morton CC: Daisuke HATAYAMA , linux-mm@kvack.org, linux-kernel@vger.kernel.org, xiyou.wangcong@gmail.com, andi@firstfloor.org, jdike@addtoit.com, tony.luck@intel.com Subject: Re: [RESEND][mmotm][PATCH v2, 0/5] elf coredump: Add extended numbering support References: <20100104.100607.189714443.d.hatayama@jp.fujitsu.com> <20100107162928.1d6eba76.akpm@linux-foundation.org> <20100107163259.86165aee.akpm@linux-foundation.org> In-Reply-To: <20100107163259.86165aee.akpm@linux-foundation.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6852 Lines: 180 Andrew Morton wrote: > On Thu, 7 Jan 2010 16:29:28 -0800 > Andrew Morton wrote: > >> On Mon, 04 Jan 2010 10:06:07 +0900 (JST) >> Daisuke HATAYAMA wrote: >> >>> The current ELF dumper can produce broken corefiles if program headers >>> exceed 65535. In particular, the program in 64-bit environment often >>> demands more than 65535 mmaps. If you google max_map_count, then you >>> can find many users facing this problem. >>> >>> Solaris has already dealt with this issue, and other OSes have also >>> adopted the same method as in Solaris. Currently, Sun's document and >>> AMD 64 ABI include the description for the extension, where they call >>> the extension Extended Numbering. See Reference for further information. >>> >>> I believe that linux kernel should adopt the same way as they did, so >>> I've written this patch. >>> >>> I am also preparing for patches of GDB and binutils. >> >> That's a beautifully presented patchset. Thanks for doing all that >> work - it helps. >> >> UML maintenance appears to have ceased in recent times, so if we wish >> to have these changes runtime tested (we should) then I think it would >> be best if you could find someone to do that please. >> >> And no akpm code-review would be complete without: dump_seek() is >> waaaay to large to be inlined. Is there some common .c file to where >> we could move it? >> > > Also, these patches made a bit of a mess of > mm-pass-mm-flags-as-a-coredump-parameter-for-consistency.patch. > > I consider > mm-pass-mm-flags-as-a-coredump-parameter-for-consistency.patch to be > less important (although older) than this patch series so I've fixed up > mm-pass-mm-flags-as-a-coredump-parameter-for-consistency.patch and have > staged it after your patch series. If this causes problems then I'll > drop mm-pass-mm-flags-as-a-coredump-parameter-for-consistency.patch, > sorry. Sure, that's fine to me too. It seems that those patches are not conflict each other directly. Thank you for modifying my patch. And I found just two misses. > From: Masami Hiramatsu > > Pass mm->flags as a coredump parameter for consistency. > > --- > 1787 if (mm->core_state || !get_dumpable(mm)) { <- (1) > 1788 up_write(&mm->mmap_sem); > 1789 put_cred(cred); > 1790 goto fail; > 1791 } > 1792 > [...] > 1798 if (get_dumpable(mm) == 2) { /* Setuid core dump mode */ <-(2) > 1799 flag = O_EXCL; /* Stop rewrite attacks */ > 1800 cred->fsuid = 0; /* Dump root private */ > 1801 } > --- > > Since dumpable bits are not protected by lock, there is a chance to change > these bits between (1) and (2). > > To solve this issue, this patch copies mm->flags to > coredump_params.mm_flags at the beginning of do_coredump() and uses it > instead of get_dumpable() while dumping core. > > This copy is also passed to binfmt->core_dump, since elf*_core_dump() uses > dump_filter bits in mm->flags. > > Signed-off-by: Masami Hiramatsu > Acked-by: Roland McGrath > Cc: Hidehiro Kawai > Cc: Oleg Nesterov > Cc: Ingo Molnar > Reviewed-by: KOSAKI Motohiro > Signed-off-by: Andrew Morton > --- > > fs/binfmt_elf.c | 12 ++---------- > fs/binfmt_elf_fdpic.c | 12 ++---------- > fs/exec.c | 20 ++++++++++++++++---- > include/linux/binfmts.h | 1 + > 4 files changed, 21 insertions(+), 24 deletions(-) > > diff -puN fs/binfmt_elf.c~mm-pass-mm-flags-as-a-coredump-parameter-for-consistency fs/binfmt_elf.c > --- a/fs/binfmt_elf.c~mm-pass-mm-flags-as-a-coredump-parameter-for-consistency > +++ a/fs/binfmt_elf.c > @@ -1905,7 +1905,6 @@ static int elf_core_dump(struct coredump > struct vm_area_struct *vma, *gate_vma; > struct elfhdr *elf = NULL; > loff_t offset = 0, dataoff, foffset; > - unsigned long mm_flags; > struct elf_note_info info; > struct elf_phdr *phdr4note = NULL; > struct elf_shdr *shdr4extnum = NULL; > @@ -1980,13 +1979,6 @@ static int elf_core_dump(struct coredump > > dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE); > > - /* > - * We must use the same mm->flags while dumping core to avoid > - * inconsistency between the program headers and bodies, otherwise an > - * unusable core file can be generated. > - */ > - mm_flags = current->mm->flags; > - > offset += elf_core_vma_data_size(gate_vma, mm_flags); ^^^^^^^^ cprm->mm_flags > offset += elf_core_extra_data_size(); > e_shoff = offset; > @@ -2018,7 +2010,7 @@ static int elf_core_dump(struct coredump > phdr.p_offset = offset; > phdr.p_vaddr = vma->vm_start; > phdr.p_paddr = 0; > - phdr.p_filesz = vma_dump_size(vma, mm_flags); > + phdr.p_filesz = vma_dump_size(vma, cprm->mm_flags); > phdr.p_memsz = vma->vm_end - vma->vm_start; > offset += phdr.p_filesz; > phdr.p_flags = vma->vm_flags & VM_READ ? PF_R : 0; > @@ -2053,7 +2045,7 @@ static int elf_core_dump(struct coredump > unsigned long addr; > unsigned long end; > > - end = vma->vm_start + vma_dump_size(vma, mm_flags); > + end = vma->vm_start + vma_dump_size(vma, cprm->mm_flags); > > for (addr = vma->vm_start; addr < end; addr += PAGE_SIZE) { > struct page *page; > diff -puN fs/binfmt_elf_fdpic.c~mm-pass-mm-flags-as-a-coredump-parameter-for-consistency fs/binfmt_elf_fdpic.c > --- a/fs/binfmt_elf_fdpic.c~mm-pass-mm-flags-as-a-coredump-parameter-for-consistency > +++ a/fs/binfmt_elf_fdpic.c > @@ -1623,7 +1623,6 @@ static int elf_fdpic_core_dump(struct co > #endif > int thread_status_size = 0; > elf_addr_t *auxv; > - unsigned long mm_flags; > struct elf_phdr *phdr4note = NULL; > struct elf_shdr *shdr4extnum = NULL; > Elf_Half e_phnum; > @@ -1766,13 +1765,6 @@ static int elf_fdpic_core_dump(struct co > /* Page-align dumped data */ > dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE); > > - /* > - * We must use the same mm->flags while dumping core to avoid > - * inconsistency between the program headers and bodies, otherwise an > - * unusable core file can be generated. > - */ > - mm_flags = current->mm->flags; > - > offset += elf_core_vma_data_size(mm_flags); ^^^^^^^^ cprm->mm_flags Thanks! -- Masami Hiramatsu Software Engineer Hitachi Computer Products (America), Inc. Software Solutions Division e-mail: mhiramat@redhat.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/