<gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> The patch below does not apply to the 5.17-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@xxxxxxxxxxxxxxx>.
I believe it requires backporting these first.
commit 84158b7f6a06 ("coredump: Also dump first pages of non-executable ELF libraries")
commit 95c5436a4883 ("coredump: Snapshot the vmas in do_coredump")
commit 49c1866348f3 ("coredump: Remove the WARN_ON in dump_vma_snapshot")
The first is a more interesting bug fix from Jann Horn.
The other two are prerequisite cleanup-patches.
I will let other folks judge how concerned they are about missing
locking that was detected by code review.
Eric
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From 390031c942116d4733310f0684beb8db19885fe6 Mon Sep 17 00:00:00 2001
> From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> Date: Tue, 8 Mar 2022 13:04:19 -0600
> Subject: [PATCH] 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@xxxxxxxxxxxxx>
> Link: https://lkml.kernel.org/r/20220131153740.2396974-1-willy@xxxxxxxxxxxxx
> Cc: stable@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxx>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 7f0c391832cf..ca5296cae979 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1641,17 +1641,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;
> @@ -1673,11 +1672,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);
> @@ -1697,9 +1697,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++;
> }
>
> @@ -1710,7 +1710,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,
> @@ -1909,7 +1909,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;
> @@ -2098,7 +2098,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 7f100a637264..7ed7d601e5e0 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -55,6 +55,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;
> @@ -765,7 +766,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);
> @@ -1099,6 +1100,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.
> @@ -1135,6 +1150,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 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 {
