Commit 879bca0a2c4f ("mm/vma: fix incorrectly disallowed anonymous VMA merges") introduced the ability to merge previously unavailable VMA merge scenarios. The key piece of logic introduced was the ability to merge a faulted VMA immediately next to an unfaulted VMA, which relies upon dup_anon_vma() to correctly handle anon_vma state. In the case of the merge of an existing VMA (that is changing properties of a VMA and then merging if those properties are shared by adjacent VMAs), dup_anon_vma() is invoked correctly. However in the case of the merge of a new VMA, a corner case peculiar to mremap() was missed. The issue is that vma_expand() only performs dup_anon_vma() if the target (the VMA that will ultimately become the merged VMA): is not the next VMA, i.e. the one that appears after the range in which the new VMA is to be established. A key insight here is that in all other cases other than mremap(), a new VMA merge either expands an existing VMA, meaning that the target VMA will be that VMA, or would have anon_vma be NULL. Specifically: * __mmap_region() - no anon_vma in place, initial mapping. * do_brk_flags() - expanding an existing VMA. * vma_merge_extend() - expanding an existing VMA. * relocate_vma_down() - no anon_vma in place, initial mapping. In addition, we are in the unique situation of needing to duplicate anon_vma state from a VMA that is neither the previous or next VMA being merged with. To account for this, introduce a new field in struct vma_merge_struct specifically for the mremap() case, and update vma_expand() to explicitly check for this case and invoke dup_anon_vma() to ensure anon_vma state is correctly propagated. This issue can be observed most directly by invoked mremap() to move around a VMA and cause this kind of merge with the MREMAP_DONTUNMAP flag specified. This will result in unlink_anon_vmas() being called after failing to duplicate anon_vma state to the target VMA, which results in the anon_vma itself being freed with folios still possessing dangling pointers to the anon_vma and thus a use-after-free bug. This bug was discovered via a syzbot report, which this patch resolves. The following program reproduces the issue (and is fixed by this patch): #define _GNU_SOURCE #include #include #include #include #define RESERVED_PGS (100) #define VMA_A_PGS (10) #define VMA_B_PGS (10) #define NUM_ITERS (1000) static void trigger_bug(void) { unsigned long page_size = sysconf(_SC_PAGE_SIZE); char *reserved, *ptr_a, *ptr_b; /* * The goal here is to achieve: * * mremap() with MREMAP_DONTUNMAP such that A and B merge: * * |-------------------------| * | | * | |-----------| |---------| * v | unfaulted | | faulted | * |-----------| |---------| * B A * * Then unmap VMA A to trigger the bug. */ /* Reserve a region of memory to operate in. */ reserved = mmap(NULL, RESERVED_PGS * page_size, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0); if (reserved == MAP_FAILED) { perror("mmap reserved"); exit(EXIT_FAILURE); } /* Map VMA A into place. */ ptr_a = mmap(&reserved[page_size], VMA_A_PGS * page_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0); if (ptr_a == MAP_FAILED) { perror("mmap VMA A"); exit(EXIT_FAILURE); } /* Fault it in. */ ptr_a[0] = 'x'; /* * Now move it out of the way so we can place VMA B in position, * unfaulted. */ ptr_a = mremap(ptr_a, VMA_A_PGS * page_size, VMA_A_PGS * page_size, MREMAP_FIXED | MREMAP_MAYMOVE, &reserved[50 * page_size]); if (ptr_a == MAP_FAILED) { perror("mremap VMA A out of the way"); exit(EXIT_FAILURE); } /* Map VMA B into place. */ ptr_b = mmap(&reserved[page_size + VMA_A_PGS * page_size], VMA_B_PGS * page_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0); if (ptr_b == MAP_FAILED) { perror("mmap VMA B"); exit(EXIT_FAILURE); } /* Now move VMA A into position w/MREMAP_DONTUNMAP + free anon_vma. */ ptr_a = mremap(ptr_a, VMA_A_PGS * page_size, VMA_A_PGS * page_size, MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP, &reserved[page_size]); if (ptr_a == MAP_FAILED) { perror("mremap VMA A with MREMAP_DONTUNMAP"); exit(EXIT_FAILURE); } /* Finally, unmap VMA A which should trigger the bug. */ munmap(ptr_a, VMA_A_PGS * page_size); /* Cleanup in case bug didn't trigger sufficiently visibly... */ munmap(reserved, RESERVED_PGS * page_size); } int main(void) { int i; for (i = 0; i < NUM_ITERS; i++) trigger_bug(); return EXIT_SUCCESS; } Signed-off-by: Lorenzo Stoakes Fixes: 879bca0a2c4f ("mm/vma: fix incorrectly disallowed anonymous VMA merges") Reported-by: syzbot+b165fc2e11771c66d8ba@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/694a2745.050a0220.19928e.0017.GAE@google.com/ Cc: stable@kernel.org --- mm/vma.c | 58 ++++++++++++++++++++++++++++++++++++++++++-------------- mm/vma.h | 3 +++ 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/mm/vma.c b/mm/vma.c index 6377aa290a27..2268f518a89b 100644 --- a/mm/vma.c +++ b/mm/vma.c @@ -1130,26 +1130,50 @@ int vma_expand(struct vma_merge_struct *vmg) mmap_assert_write_locked(vmg->mm); vma_start_write(target); - if (next && (target != next) && (vmg->end == next->vm_end)) { + if (next && vmg->end == next->vm_end) { + struct vm_area_struct *copied_from = vmg->copied_from; int ret; - sticky_flags |= next->vm_flags & VM_STICKY; - remove_next = true; - /* This should already have been checked by this point. */ - VM_WARN_ON_VMG(!can_merge_remove_vma(next), vmg); - vma_start_write(next); - /* - * In this case we don't report OOM, so vmg->give_up_on_mm is - * safe. - */ - ret = dup_anon_vma(target, next, &anon_dup); - if (ret) - return ret; + if (target != next) { + sticky_flags |= next->vm_flags & VM_STICKY; + remove_next = true; + /* This should already have been checked by this point. */ + VM_WARN_ON_VMG(!can_merge_remove_vma(next), vmg); + vma_start_write(next); + /* + * In this case we don't report OOM, so vmg->give_up_on_mm is + * safe. + */ + ret = dup_anon_vma(target, next, &anon_dup); + if (ret) + return ret; + } else if (copied_from) { + vma_start_write(next); + + /* + * We are copying from a VMA (i.e. mremap()'ing) to + * next, and thus must ensure that either anon_vma's are + * already compatible (in which case this call is a nop) + * or all anon_vma state is propagated to next + */ + ret = dup_anon_vma(next, copied_from, &anon_dup); + if (ret) + return ret; + } else { + /* In no other case may the anon_vma differ. */ + VM_WARN_ON_VMG(target->anon_vma != next->anon_vma, vmg); + } } /* Not merging but overwriting any part of next is not handled. */ VM_WARN_ON_VMG(next && !remove_next && next != target && vmg->end > next->vm_start, vmg); + /* + * We should only see a copy with next as the target on a new merge + * which sets the end to the next of next. + */ + VM_WARN_ON_VMG(target == next && vmg->copied_from && + vmg->end != next->vm_end, vmg); /* Only handles expanding */ VM_WARN_ON_VMG(target->vm_start < vmg->start || target->vm_end > vmg->end, vmg); @@ -1807,6 +1831,13 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, VMA_ITERATOR(vmi, mm, addr); VMG_VMA_STATE(vmg, &vmi, NULL, vma, addr, addr + len); + /* + * VMG_VMA_STATE() installs vma in middle, but this is a new VMA, inform + * merging logic correctly. + */ + vmg.copied_from = vma; + vmg.middle = NULL; + /* * If anonymous vma has not yet been faulted, update new pgoff * to match new location, to increase its chance of merging. @@ -1828,7 +1859,6 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, if (new_vma && new_vma->vm_start < addr + len) return NULL; /* should never get here */ - vmg.middle = NULL; /* New VMA range. */ vmg.pgoff = pgoff; vmg.next = vma_iter_next_rewind(&vmi, NULL); new_vma = vma_merge_new_range(&vmg); diff --git a/mm/vma.h b/mm/vma.h index e4c7bd79de5f..50f0bdb0eb79 100644 --- a/mm/vma.h +++ b/mm/vma.h @@ -106,6 +106,9 @@ struct vma_merge_struct { struct anon_vma_name *anon_name; enum vma_merge_state state; + /* If we are copying a VMA, which VMA are we copying from? */ + struct vm_area_struct *copied_from; + /* Flags which callers can use to modify merge behaviour: */ /* -- 2.52.0