Skip to content

Commit 39fd0cc

Browse files
ebiedermgregkh
authored andcommitted
coredump: Use the vma snapshot in fill_files_note
commit 390031c 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: a07279c ("binfmt_elf, binfmt_elf_fdpic: use a VMA list snapshot") Fixes: 2aa362c ("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>
1 parent cabd696 commit 39fd0cc

File tree

3 files changed

+35
-13
lines changed

3 files changed

+35
-13
lines changed

fs/binfmt_elf.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,17 +1618,16 @@ static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
16181618
* long file_ofs
16191619
* followed by COUNT filenames in ASCII: "FILE1" NUL "FILE2" NUL...
16201620
*/
1621-
static int fill_files_note(struct memelfnote *note)
1621+
static int fill_files_note(struct memelfnote *note, struct coredump_params *cprm)
16221622
{
1623-
struct mm_struct *mm = current->mm;
1624-
struct vm_area_struct *vma;
16251623
unsigned count, size, names_ofs, remaining, n;
16261624
user_long_t *data;
16271625
user_long_t *start_end_ofs;
16281626
char *name_base, *name_curpos;
1627+
int i;
16291628

16301629
/* *Estimated* file count and total data size needed */
1631-
count = mm->map_count;
1630+
count = cprm->vma_count;
16321631
if (count > UINT_MAX / 64)
16331632
return -EINVAL;
16341633
size = count * 64;
@@ -1650,11 +1649,12 @@ static int fill_files_note(struct memelfnote *note)
16501649
name_base = name_curpos = ((char *)data) + names_ofs;
16511650
remaining = size - names_ofs;
16521651
count = 0;
1653-
for (vma = mm->mmap; vma != NULL; vma = vma->vm_next) {
1652+
for (i = 0; i < cprm->vma_count; i++) {
1653+
struct core_vma_metadata *m = &cprm->vma_meta[i];
16541654
struct file *file;
16551655
const char *filename;
16561656

1657-
file = vma->vm_file;
1657+
file = m->file;
16581658
if (!file)
16591659
continue;
16601660
filename = file_path(file, name_curpos, remaining);
@@ -1674,9 +1674,9 @@ static int fill_files_note(struct memelfnote *note)
16741674
memmove(name_curpos, filename, n);
16751675
name_curpos += n;
16761676

1677-
*start_end_ofs++ = vma->vm_start;
1678-
*start_end_ofs++ = vma->vm_end;
1679-
*start_end_ofs++ = vma->vm_pgoff;
1677+
*start_end_ofs++ = m->start;
1678+
*start_end_ofs++ = m->end;
1679+
*start_end_ofs++ = m->pgoff;
16801680
count++;
16811681
}
16821682

@@ -1687,7 +1687,7 @@ static int fill_files_note(struct memelfnote *note)
16871687
* Count usually is less than mm->map_count,
16881688
* we need to move filenames down.
16891689
*/
1690-
n = mm->map_count - count;
1690+
n = cprm->vma_count - count;
16911691
if (n != 0) {
16921692
unsigned shift_bytes = n * 3 * sizeof(data[0]);
16931693
memmove(name_base - shift_bytes, name_base,
@@ -1886,7 +1886,7 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
18861886
fill_auxv_note(&info->auxv, current->mm);
18871887
info->size += notesize(&info->auxv);
18881888

1889-
if (fill_files_note(&info->files) == 0)
1889+
if (fill_files_note(&info->files, cprm) == 0)
18901890
info->size += notesize(&info->files);
18911891

18921892
return 1;
@@ -2075,7 +2075,7 @@ static int fill_note_info(struct elfhdr *elf, int phdrs,
20752075
fill_auxv_note(info->notes + 3, current->mm);
20762076
info->numnote = 4;
20772077

2078-
if (fill_files_note(info->notes + info->numnote) == 0) {
2078+
if (fill_files_note(info->notes + info->numnote, cprm) == 0) {
20792079
info->notes_files = info->notes + info->numnote;
20802080
info->numnote++;
20812081
}

fs/coredump.c

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
#include <trace/events/sched.h>
5555

5656
static bool dump_vma_snapshot(struct coredump_params *cprm);
57+
static void free_vma_snapshot(struct coredump_params *cprm);
5758

5859
int core_uses_pid;
5960
unsigned int core_pipe_limit;
@@ -834,7 +835,7 @@ void do_coredump(const kernel_siginfo_t *siginfo)
834835
dump_emit(&cprm, "", 1);
835836
}
836837
file_end_write(cprm.file);
837-
kvfree(cprm.vma_meta);
838+
free_vma_snapshot(&cprm);
838839
}
839840
if (ispipe && core_pipe_limit)
840841
wait_for_dump_helpers(cprm.file);
@@ -1111,6 +1112,20 @@ static struct vm_area_struct *next_vma(struct vm_area_struct *this_vma,
11111112
return gate_vma;
11121113
}
11131114

1115+
static void free_vma_snapshot(struct coredump_params *cprm)
1116+
{
1117+
if (cprm->vma_meta) {
1118+
int i;
1119+
for (i = 0; i < cprm->vma_count; i++) {
1120+
struct file *file = cprm->vma_meta[i].file;
1121+
if (file)
1122+
fput(file);
1123+
}
1124+
kvfree(cprm->vma_meta);
1125+
cprm->vma_meta = NULL;
1126+
}
1127+
}
1128+
11141129
/*
11151130
* Under the mmap_lock, take a snapshot of relevant information about the task's
11161131
* VMAs.
@@ -1147,6 +1162,11 @@ static bool dump_vma_snapshot(struct coredump_params *cprm)
11471162
m->end = vma->vm_end;
11481163
m->flags = vma->vm_flags;
11491164
m->dump_size = vma_dump_size(vma, cprm->mm_flags);
1165+
m->pgoff = vma->vm_pgoff;
1166+
1167+
m->file = vma->vm_file;
1168+
if (m->file)
1169+
get_file(m->file);
11501170
}
11511171

11521172
mmap_write_unlock(mm);

include/linux/coredump.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ struct core_vma_metadata {
1212
unsigned long start, end;
1313
unsigned long flags;
1414
unsigned long dump_size;
15+
unsigned long pgoff;
16+
struct file *file;
1517
};
1618

1719
extern int core_uses_pid;

0 commit comments

Comments
 (0)