linux-kernel - Re: [RFC, PATCH 4/4] elf_core_dump(): Add extended numbering support

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 15 Dec 2009 16:27:00 +0800
From:	Américo Wang <xiyou.wangcong@...il.com>
To:	Daisuke HATAYAMA <d.hatayama@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	jdike@...toit.com, tony.luck@...el.com, mhiramat@...hat.com
Subject: Re: [RFC, PATCH 4/4] elf_core_dump(): Add extended numbering support

On Tue, Dec 15, 2009 at 10:41 AM, Daisuke HATAYAMA
<d.hatayama@...fujitsu.com> wrote:
> The current ELF dumper implementation can produce broken corefiles
> if program headers exceed 65535. This number is determined by the
> number of vmas which the process have. In particular, some extreme
> programs may use more than 65535 vmas. (If you google max_map_count,
> you can find some users facing this problem.) This kind of program
> never be able to generate correct coredumps.
>
> This patch implements ``extended numbering'' that uses sh_info
> field of the first section header instead of e_phnum field in order
> to represent upto 4294967295 vmas.
>
> This is supported by AMD64-ABI(http://www.x86-64.org/documentation.html)
> and Solaris(http://docs.sun.com/app/docs/doc/817-1984/). Of course,
> we are preparing patches for gdb and binutils.
>
> Signed-off-by: Daisuke HATAYAMA <d.hatayama@...fujitsu.com>


Some comments below.

> ---
>  arch/ia64/kernel/elfcore.c |   16 ++++++++
>  arch/um/sys-i386/elfcore.c |   18 +++++++++
>  fs/binfmt_elf.c            |   88 +++++++++++++++++++++++++++++++++++++++-----
>  include/linux/elf.h        |   26 ++++++++++++-
>  4 files changed, 137 insertions(+), 11 deletions(-)
>
> diff --git a/arch/ia64/kernel/elfcore.c b/arch/ia64/kernel/elfcore.c
> index 9c0dd8b..a15d8d4 100644
> --- a/arch/ia64/kernel/elfcore.c
> +++ b/arch/ia64/kernel/elfcore.c
> @@ -73,3 +73,19 @@ int elf_core_write_extra_data(struct file *file, size_t *size,
>        }
>        return 1;
>  }
> +
> +size_t elf_core_extra_data_size(void)
> +{
> +       const struct elf_phdr *const gate_phdrs =
> +               (const struct elf_phdr *) (GATE_ADDR + GATE_EHDR->e_phoff);
> +       int i;
> +       size_t size = 0;
> +
> +       for (i = 0; i < GATE_EHDR->e_phnum; ++i) {
> +               if (gate_phdrs[i].p_type == PT_LOAD) {
> +                       size += PAGE_ALIGN(gate_phdrs[i].p_memsz);
> +                       break;
> +               }
> +       }
> +       return size;
> +}
> diff --git a/arch/um/sys-i386/elfcore.c b/arch/um/sys-i386/elfcore.c
> index 4e320f0..4e34e47 100644
> --- a/arch/um/sys-i386/elfcore.c
> +++ b/arch/um/sys-i386/elfcore.c
> @@ -76,3 +76,21 @@ int elf_core_write_extra_data(struct file *file, size_t *size,
>        }
>        return 1;
>  }
> +
> +size_t elf_core_extra_data_size(void)
> +{
> +       if ( vsyscall_ehdr ) {
> +               const struct elfhdr *const ehdrp =
> +                       (struct elfhdr *)vsyscall_ehdr;
> +               const struct elf_phdr *const phdrp =
> +                       (const struct elf_phdr *) (vsyscall_ehdr + ehdrp->e_phoff);
> +               int i;
> +
> +               for (i = 0; i < ehdrp->e_phnum; ++i) {
> +                       if (phdrp[i].p_type == PT_LOAD) {
> +                               return (size_t) phdrp[i].p_filesz;
> +                       }
> +               }

Unnecessary braces.

> +       }
> +       return 0;
> +}
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index cded1ba..ad2ad5f 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1895,6 +1895,38 @@ static struct vm_area_struct *next_vma(struct vm_area_struct *this_vma,
>        return gate_vma;
>  }
>
> +static void fill_extnum_info(struct elfhdr *elf, struct elf_shdr *shdr4extnum,
> +                            elf_addr_t e_shoff, int segs)
> +{
> +       elf->e_shoff = e_shoff;
> +       elf->e_shentsize = sizeof(*shdr4extnum);
> +       elf->e_shnum = 1;
> +       elf->e_shstrndx = SHN_UNDEF;
> +
> +       shdr4extnum->sh_name = 0;
> +       shdr4extnum->sh_addr = 0;
> +       shdr4extnum->sh_offset = 0;
> +       shdr4extnum->sh_type = SHT_NULL;
> +       shdr4extnum->sh_flags = 0;
> +       shdr4extnum->sh_size = elf->e_shnum;
> +       shdr4extnum->sh_link = elf->e_shstrndx;
> +       shdr4extnum->sh_info = segs;
> +       shdr4extnum->sh_addralign = 0;
> +       shdr4extnum->sh_entsize = 0;


Why not kzalloc() the struct and just fill the non-zero values?

<snip>

> @@ -2079,11 +2139,19 @@ static int elf_core_dump(long signr, struct pt_regs *regs, struct file *file, un
>        if (!elf_core_write_extra_data(file, &size, limit))
>                goto end_coredump;
>
> +       if (e_phnum == PN_XNUM) {
> +               size += sizeof(*shdr4extnum);
> +               if (size > limit
> +                   || !dump_write(file, shdr4extnum, sizeof(*shdr4extnum)))
> +                       goto end_coredump;
> +       }
> +
>  end_coredump:

Even without the last 'goto', it will also come here finally.
So, that is unnecessary.
--
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/