coredump: Use the vma snapshot in fill_files_note

commit 390031c942116d4733310f0684beb8db19885fe6 upstream.

Matthew Wilcox reported that there is a missing mmap_lock in
file_files_note that could possibly lead to a user after free.

Solve this by using the existing vma snapshot for consistency
and to avoid the need to take the mmap_lock anywhere in the
coredump code except for dump_vma_snapshot.

Update the dump_vma_snapshot to capture vm_pgoff and vm_file
that are neeeded by fill_files_note.

Add free_vma_snapshot to free the captured values of vm_file.

Reported-by: Matthew Wilcox <willy@infradead.org>
Link: https://lkml.kernel.org/r/20220131153740.2396974-1-willy@infradead.org
Cc: stable@vger.kernel.org
Fixes: a07279c9a8cd ("binfmt_elf, binfmt_elf_fdpic: use a VMA list snapshot")
Fixes: 2aa362c49c31 ("coredump: extend core dump note section to contain file names of mapped files")
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 1ae3fe3..c93150f 100644 (file)
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1618,17 +1618,16 @@ static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
  *   long file_ofs
  * followed by COUNT filenames in ASCII: "FILE1" NUL "FILE2" NUL...
  */
-static int fill_files_note(struct memelfnote *note)
+static int fill_files_note(struct memelfnote *note, struct coredump_params *cprm)
 {
-       struct mm_struct *mm = current->mm;
-       struct vm_area_struct *vma;
        unsigned count, size, names_ofs, remaining, n;
        user_long_t *data;
        user_long_t *start_end_ofs;
        char *name_base, *name_curpos;
+       int i;
 
        /* *Estimated* file count and total data size needed */
-       count = mm->map_count;
+       count = cprm->vma_count;
        if (count > UINT_MAX / 64)
                return -EINVAL;
        size = count * 64;
@@ -1650,11 +1649,12 @@ static int fill_files_note(struct memelfnote *note)
        name_base = name_curpos = ((char *)data) + names_ofs;
        remaining = size - names_ofs;
        count = 0;
-       for (vma = mm->mmap; vma != NULL; vma = vma->vm_next) {
+       for (i = 0; i < cprm->vma_count; i++) {
+               struct core_vma_metadata *m = &cprm->vma_meta[i];
                struct file *file;
                const char *filename;
 
-               file = vma->vm_file;
+               file = m->file;
                if (!file)
                        continue;
                filename = file_path(file, name_curpos, remaining);
@@ -1674,9 +1674,9 @@ static int fill_files_note(struct memelfnote *note)
                memmove(name_curpos, filename, n);
                name_curpos += n;
 
-               *start_end_ofs++ = vma->vm_start;
-               *start_end_ofs++ = vma->vm_end;
-               *start_end_ofs++ = vma->vm_pgoff;
+               *start_end_ofs++ = m->start;
+               *start_end_ofs++ = m->end;
+               *start_end_ofs++ = m->pgoff;
                count++;
        }
 
@@ -1687,7 +1687,7 @@ static int fill_files_note(struct memelfnote *note)
         * Count usually is less than mm->map_count,
         * we need to move filenames down.
         */
-       n = mm->map_count - count;
+       n = cprm->vma_count - count;
        if (n != 0) {
                unsigned shift_bytes = n * 3 * sizeof(data[0]);
                memmove(name_base - shift_bytes, name_base,
@@ -1886,7 +1886,7 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
        fill_auxv_note(&info->auxv, current->mm);
        info->size += notesize(&info->auxv);
 
-       if (fill_files_note(&info->files) == 0)
+       if (fill_files_note(&info->files, cprm) == 0)
                info->size += notesize(&info->files);
 
        return 1;
@@ -2075,7 +2075,7 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
        fill_auxv_note(info->notes + 3, current->mm);
        info->numnote = 4;
 
-       if (fill_files_note(info->notes + info->numnote) == 0) {
+       if (fill_files_note(info->notes + info->numnote, cprm) == 0) {
                info->notes_files = info->notes + info->numnote;
                info->numnote++;
        }

diff --git a/fs/coredump.c b/fs/coredump.c
index e95fea3..26eb5a0 100644 (file)
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -54,6 +54,7 @@
 #include <trace/events/sched.h>
 
 static bool dump_vma_snapshot(struct coredump_params *cprm);
+static void free_vma_snapshot(struct coredump_params *cprm);
 
 int core_uses_pid;
 unsigned int core_pipe_limit;
@@ -834,7 +835,7 @@ void do_coredump(const kernel_siginfo_t *siginfo)
                        dump_emit(&cprm, "", 1);
                }
                file_end_write(cprm.file);
-               kvfree(cprm.vma_meta);
+               free_vma_snapshot(&cprm);
        }
        if (ispipe && core_pipe_limit)
                wait_for_dump_helpers(cprm.file);
@@ -1111,6 +1112,20 @@ static struct vm_area_struct *next_vma(struct vm_area_struct *this_vma,
        return gate_vma;
 }
 
+static void free_vma_snapshot(struct coredump_params *cprm)
+{
+       if (cprm->vma_meta) {
+               int i;
+               for (i = 0; i < cprm->vma_count; i++) {
+                       struct file *file = cprm->vma_meta[i].file;
+                       if (file)
+                               fput(file);
+               }
+               kvfree(cprm->vma_meta);
+               cprm->vma_meta = NULL;
+       }
+}
+
 /*
  * Under the mmap_lock, take a snapshot of relevant information about the task's
  * VMAs.
@@ -1147,6 +1162,11 @@ static bool dump_vma_snapshot(struct coredump_params *cprm)
                m->end = vma->vm_end;
                m->flags = vma->vm_flags;
                m->dump_size = vma_dump_size(vma, cprm->mm_flags);
+               m->pgoff = vma->vm_pgoff;
+
+               m->file = vma->vm_file;
+               if (m->file)
+                       get_file(m->file);
        }
 
        mmap_write_unlock(mm);

diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index cc1aee2..4b95e46 100644 (file)
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -12,6 +12,8 @@ struct core_vma_metadata {
        unsigned long start, end;
        unsigned long flags;
        unsigned long dump_size;
+       unsigned long pgoff;
+       struct file   *file;
 };
 
 extern int core_uses_pid;