| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4B469511.1030402@redhat.com>
Date: Thu, 07 Jan 2010 21:14:41 -0500
From: Masami Hiramatsu <mhiramat@...hat.com>
To: Andrew Morton <akpm@...ux-foundation.org>
CC: Daisuke HATAYAMA <d.hatayama@...fujitsu.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, xiyou.wangcong@...il.com,
andi@...stfloor.org, jdike@...toit.com, tony.luck@...el.com
Subject: Re: [RESEND][mmotm][PATCH v2, 0/5] elf coredump: Add extended numbering
support
Andrew Morton wrote:
> On Thu, 7 Jan 2010 16:29:28 -0800
> Andrew Morton <akpm@...ux-foundation.org> wrote:
>
>> On Mon, 04 Jan 2010 10:06:07 +0900 (JST)
>> Daisuke HATAYAMA <d.hatayama@...fujitsu.com> 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 <mhiramat@...hat.com>
>
> 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 <mhiramat@...hat.com>
> Acked-by: Roland McGrath <roland@...hat.com>
> Cc: Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>
> Cc: Oleg Nesterov <oleg@...hat.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> ---
>
> 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@...hat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists