Commit 390031c9 authored by Eric W. Biederman's avatar Eric W. Biederman
Browse files

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: default avatarMatthew Wilcox <willy@infradead.org>
Link: https://lkml.kernel.org/r/20220131153740.2396974-1-willy@infradead.org


Cc: stable@vger.kernel.org
Fixes: a07279c9 ("binfmt_elf, binfmt_elf_fdpic: use a VMA list snapshot")
Fixes: 2aa362c4 ("coredump: extend core dump note section to contain file names of mapped files")
Reviewed-by: default avatarKees Cook <keescook@chromium.org>
Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
parent 9ec7d323

fs/binfmt_elf.c

+12 −12
Original line number Diff line number Diff line
@@ -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++;

	}

fs/coredump.c

+21 −1
Original line number Diff line number Diff line
@@ -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);

include/linux/coredump.h

+2 −0
Original line number Diff line number Diff line
@@ -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 {