lkml.org 
[lkml]   [2022]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 5/5] coredump: Use the vma snapshot in fill_files_note

    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")
    Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
    ---
    fs/binfmt_elf.c | 19 ++++++++++---------
    fs/coredump.c | 22 +++++++++++++++++++++-
    include/linux/coredump.h | 2 ++
    3 files changed, 33 insertions(+), 10 deletions(-)

    diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
    index 272032b1f9a2..5fcaa01d211e 100644
    --- a/fs/binfmt_elf.c
    +++ b/fs/binfmt_elf.c
    @@ -1619,14 +1619,14 @@ 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;
    @@ -1651,11 +1651,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);
    @@ -1675,9 +1676,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++;
    }

    @@ -1887,7 +1888,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;
    @@ -2076,7 +2077,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 c5e7d63525c6..6a97a8ea7295 100644
    --- 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);

    static int core_uses_pid;
    static unsigned int core_pipe_limit;
    @@ -764,7 +765,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);
    @@ -1085,6 +1086,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.
    @@ -1121,6 +1136,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);

    cprm->vma_data_size += m->dump_size;
    }
    diff --git a/include/linux/coredump.h b/include/linux/coredump.h
    index 7d05370e555e..08a1d3e7e46d 100644
    --- 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;
    };

    struct coredump_params {
    --
    2.29.2
    \
     
     \ /
      Last update: 2022-01-31 19:49    [W:2.588 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site