coredump: Snapshot the vmas in do_coredump

commit 95c5436a4883841588dae86fb0b9325f47ba5ad3 upstream.

Move the call of dump_vma_snapshot and kvfree(vma_meta) out of the
individual coredump routines into do_coredump itself.  This makes
the code less error prone and easier to maintain.

Make the vma snapshot available to the coredump routines
in struct coredump_params.  This makes it easier to
change and update what is captures in the vma snapshot
and will be needed for fixing fill_file_notes.

Reviewed-by: Jann Horn <jannh@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 3f6a7cac68fd2e9e9f0ab6563e80c96131b1daef..bd6189f55ddb1986c27c6b1d3071f21122d96cfb 100644 (file)
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -2168,8 +2168,7 @@ static void fill_extnum_info(struct elfhdr *elf, struct elf_shdr *shdr4extnum,
 static int elf_core_dump(struct coredump_params *cprm)
 {
        int has_dumped = 0;
-       int vma_count, segs, i;
-       size_t vma_data_size;
+       int segs, i;
        struct elfhdr elf;
        loff_t offset = 0, dataoff;
        struct elf_note_info info = { };
@@ -2177,16 +2176,12 @@ static int elf_core_dump(struct coredump_params *cprm)
        struct elf_shdr *shdr4extnum = NULL;
        Elf_Half e_phnum;
        elf_addr_t e_shoff;
-       struct core_vma_metadata *vma_meta;
-
-       if (dump_vma_snapshot(cprm, &vma_count, &vma_meta, &vma_data_size))
-               return 0;
 
        /*
         * The number of segs are recored into ELF header as 16bit value.
         * Please check DEFAULT_MAX_MAP_COUNT definition when you modify here.
         */
-       segs = vma_count + elf_core_extra_phdrs();
+       segs = cprm->vma_count + elf_core_extra_phdrs();
 
        /* for notes section */
        segs++;
@@ -2225,7 +2220,7 @@ static int elf_core_dump(struct coredump_params *cprm)
 
        dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE);
 
-       offset += vma_data_size;
+       offset += cprm->vma_data_size;
        offset += elf_core_extra_data_size();
        e_shoff = offset;
 
@@ -2245,8 +2240,8 @@ static int elf_core_dump(struct coredump_params *cprm)
                goto end_coredump;
 
        /* Write program headers for segments dump */
-       for (i = 0; i < vma_count; i++) {
-               struct core_vma_metadata *meta = vma_meta + i;
+       for (i = 0; i < cprm->vma_count; i++) {
+               struct core_vma_metadata *meta = cprm->vma_meta + i;
                struct elf_phdr phdr;
 
                phdr.p_type = PT_LOAD;
@@ -2283,8 +2278,8 @@ static int elf_core_dump(struct coredump_params *cprm)
        /* Align to page */
        dump_skip_to(cprm, dataoff);
 
-       for (i = 0; i < vma_count; i++) {
-               struct core_vma_metadata *meta = vma_meta + i;
+       for (i = 0; i < cprm->vma_count; i++) {
+               struct core_vma_metadata *meta = cprm->vma_meta + i;
 
                if (!dump_user_range(cprm, meta->start, meta->dump_size))
                        goto end_coredump;
@@ -2301,7 +2296,6 @@ static int elf_core_dump(struct coredump_params *cprm)
 end_coredump:
        free_note_info(&info);
        kfree(shdr4extnum);
-       kvfree(vma_meta);
        kfree(phdr4note);
        return has_dumped;
 }

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 6d8fd6030cbb53d25689d8acebb88cce9b88cb95..830a6a876ffea519c24de3458fadb49f8b18bb23 100644 (file)
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1465,7 +1465,7 @@ static bool elf_fdpic_dump_segments(struct coredump_params *cprm,
 static int elf_fdpic_core_dump(struct coredump_params *cprm)
 {
        int has_dumped = 0;
-       int vma_count, segs;
+       int segs;
        int i;
        struct elfhdr *elf = NULL;
        loff_t offset = 0, dataoff;
@@ -1480,8 +1480,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
        elf_addr_t e_shoff;
        struct core_thread *ct;
        struct elf_thread_status *tmp;
-       struct core_vma_metadata *vma_meta = NULL;
-       size_t vma_data_size;
 
        /* alloc memory for large data structures: too large to be on stack */
        elf = kmalloc(sizeof(*elf), GFP_KERNEL);
@@ -1491,9 +1489,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
        if (!psinfo)
                goto end_coredump;
 
-       if (dump_vma_snapshot(cprm, &vma_count, &vma_meta, &vma_data_size))
-               goto end_coredump;
-
        for (ct = current->mm->core_state->dumper.next;
                                        ct; ct = ct->next) {
                tmp = elf_dump_thread_status(cprm->siginfo->si_signo,
@@ -1513,7 +1508,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
        tmp->next = thread_list;
        thread_list = tmp;
 
-       segs = vma_count + elf_core_extra_phdrs();
+       segs = cprm->vma_count + elf_core_extra_phdrs();
 
        /* for notes section */
        segs++;
@@ -1558,7 +1553,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
        /* Page-align dumped data */
        dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE);
 
-       offset += vma_data_size;
+       offset += cprm->vma_data_size;
        offset += elf_core_extra_data_size();
        e_shoff = offset;
 
@@ -1578,8 +1573,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
                goto end_coredump;
 
        /* write program headers for segments dump */
-       for (i = 0; i < vma_count; i++) {
-               struct core_vma_metadata *meta = vma_meta + i;
+       for (i = 0; i < cprm->vma_count; i++) {
+               struct core_vma_metadata *meta = cprm->vma_meta + i;
                struct elf_phdr phdr;
                size_t sz;
 
@@ -1628,7 +1623,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 
        dump_skip_to(cprm, dataoff);
 
-       if (!elf_fdpic_dump_segments(cprm, vma_meta, vma_count))
+       if (!elf_fdpic_dump_segments(cprm, cprm->vma_meta, cprm->vma_count))
                goto end_coredump;
 
        if (!elf_core_write_extra_data(cprm))
@@ -1652,7 +1647,6 @@ end_coredump:
                thread_list = thread_list->next;
                kfree(tmp);
        }
-       kvfree(vma_meta);
        kfree(phdr4note);
        kfree(elf);
        kfree(psinfo);

diff --git a/fs/coredump.c b/fs/coredump.c
index bb66a4e3819f16795fb2f24e467fd71b8300d55e..64e2ce8974200a03f53b3aebdebab8dcc9690e7d 100644 (file)
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -53,6 +53,8 @@
 
 #include <trace/events/sched.h>
 
+static bool dump_vma_snapshot(struct coredump_params *cprm);
+
 int core_uses_pid;
 unsigned int core_pipe_limit;
 char core_pattern[CORENAME_MAX_SIZE] = "core";
@@ -601,6 +603,7 @@ void do_coredump(const kernel_siginfo_t *siginfo)
                 * by any locks.
                 */
                .mm_flags = mm->flags,
+               .vma_meta = NULL,
        };
 
        audit_core_dumps(siginfo->si_signo);
@@ -815,6 +818,9 @@ void do_coredump(const kernel_siginfo_t *siginfo)
                        pr_info("Core dump to |%s disabled\n", cn.corename);
                        goto close_fail;
                }
+               if (!dump_vma_snapshot(&cprm))
+                       goto close_fail;
+
                file_start_write(cprm.file);
                core_dumped = binfmt->core_dump(&cprm);
                /*
@@ -828,6 +834,7 @@ void do_coredump(const kernel_siginfo_t *siginfo)
                        dump_emit(&cprm, "", 1);
                }
                file_end_write(cprm.file);
+               kvfree(cprm.vma_meta);
        }
        if (ispipe && core_pipe_limit)
                wait_for_dump_helpers(cprm.file);
@@ -1108,14 +1115,11 @@ static struct vm_area_struct *next_vma(struct vm_area_struct *this_vma,
  * Under the mmap_lock, take a snapshot of relevant information about the task's
  * VMAs.
  */
-int dump_vma_snapshot(struct coredump_params *cprm, int *vma_count,
-                     struct core_vma_metadata **vma_meta,
-                     size_t *vma_data_size_ptr)
+static bool dump_vma_snapshot(struct coredump_params *cprm)
 {
        struct vm_area_struct *vma, *gate_vma;
        struct mm_struct *mm = current->mm;
        int i;
-       size_t vma_data_size = 0;
 
        /*
         * Once the stack expansion code is fixed to not change VMA bounds
@@ -1123,20 +1127,21 @@ int dump_vma_snapshot(struct coredump_params *cprm, int *vma_count,
         * mmap_lock in read mode.
         */
        if (mmap_write_lock_killable(mm))
-               return -EINTR;
+               return false;
 
+       cprm->vma_data_size = 0;
        gate_vma = get_gate_vma(mm);
-       *vma_count = mm->map_count + (gate_vma ? 1 : 0);
+       cprm->vma_count = mm->map_count + (gate_vma ? 1 : 0);
 
-       *vma_meta = kvmalloc_array(*vma_count, sizeof(**vma_meta), GFP_KERNEL);
-       if (!*vma_meta) {
+       cprm->vma_meta = kvmalloc_array(cprm->vma_count, sizeof(*cprm->vma_meta), GFP_KERNEL);
+       if (!cprm->vma_meta) {
                mmap_write_unlock(mm);
-               return -ENOMEM;
+               return false;
        }
 
        for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
                        vma = next_vma(vma, gate_vma), i++) {
-               struct core_vma_metadata *m = (*vma_meta) + i;
+               struct core_vma_metadata *m = cprm->vma_meta + i;
 
                m->start = vma->vm_start;
                m->end = vma->vm_end;
@@ -1146,13 +1151,14 @@ int dump_vma_snapshot(struct coredump_params *cprm, int *vma_count,
 
        mmap_write_unlock(mm);
 
-       if (WARN_ON(i != *vma_count)) {
-               kvfree(*vma_meta);
-               return -EFAULT;
+       if (WARN_ON(i != cprm->vma_count)) {
+               kvfree(cprm->vma_meta);
+               return false;
        }
 
-       for (i = 0; i < *vma_count; i++) {
-               struct core_vma_metadata *m = (*vma_meta) + i;
+
+       for (i = 0; i < cprm->vma_count; i++) {
+               struct core_vma_metadata *m = cprm->vma_meta + i;
 
                if (m->dump_size == DUMP_SIZE_MAYBE_ELFHDR_PLACEHOLDER) {
                        char elfmag[SELFMAG];
@@ -1165,9 +1171,8 @@ int dump_vma_snapshot(struct coredump_params *cprm, int *vma_count,
                        }
                }
 
-               vma_data_size += m->dump_size;
+               cprm->vma_data_size += m->dump_size;
        }
 
-       *vma_data_size_ptr = vma_data_size;
-       return 0;
+       return true;
 }

diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 049cf9421d83141f5cd851ca953f3fcb85932d56..f821b724336135eae391c944e39319cb7b90774a 100644 (file)
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -87,6 +87,9 @@ struct coredump_params {
        loff_t written;
        loff_t pos;
        loff_t to_skip;
+       int vma_count;
+       size_t vma_data_size;
+       struct core_vma_metadata *vma_meta;
 };
 
 /*

diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index 78fcd776b185a853653e2bbd499e7d2c28675439..cc1aee2cb46aed64efa847a740109e3f4b046d48 100644 (file)
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -29,9 +29,6 @@ extern int dump_emit(struct coredump_params *cprm, const void *addr, int nr);
 extern int dump_align(struct coredump_params *cprm, int align);
 int dump_user_range(struct coredump_params *cprm, unsigned long start,
                    unsigned long len);
-int dump_vma_snapshot(struct coredump_params *cprm, int *vma_count,
-                     struct core_vma_metadata **vma_meta,
-                     size_t *vma_data_size_ptr);
 extern void do_coredump(const kernel_siginfo_t *siginfo);
 #else
 static inline void do_coredump(const kernel_siginfo_t *siginfo) {}