Skip to content

Commit 734537c

Browse files
kiryltorvalds
authored andcommitted
mm: fix use-after-free if memory allocation failed in vma_adjust()
There's one case when vma_adjust() expands the vma, overlapping with *two* next vma. See case 6 of mprotect, described in the comment to vma_merge(). To handle this (and only this) situation we iterate twice over main part of the function. See "goto again". Vegard reported[1] that he sees out-of-bounds access complain from KASAN, if anon_vma_clone() on the *second* iteration fails. This happens because we free 'next' vma by the end of first iteration and don't have a way to undo this if anon_vma_clone() fails on the second iteration. The solution is to do all required allocations upfront, before we touch vmas. The allocation on the second iteration is only required if first two vmas don't have anon_vma, but third does. So we need, in total, one anon_vma_clone() call. It's easy to adjust 'exporter' to the third vma for such case. [1] http://lkml.kernel.org/r/[email protected] Link: http://lkml.kernel.org/r/1469625255-126641-1-git-send-email-kirill.shutemov@linux.intel.com Signed-off-by: Kirill A. Shutemov <[email protected]> Reported-by: Vegard Nossum <[email protected]> Cc: Rik van Riel <[email protected]> Cc: Vlastimil Babka <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent c3491ec commit 734537c

File tree

1 file changed

+15
-5
lines changed

1 file changed

+15
-5
lines changed

mm/mmap.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,6 @@ int vma_adjust(struct vm_area_struct *vma, unsigned long start,
621621
{
622622
struct mm_struct *mm = vma->vm_mm;
623623
struct vm_area_struct *next = vma->vm_next;
624-
struct vm_area_struct *importer = NULL;
625624
struct address_space *mapping = NULL;
626625
struct rb_root *root = NULL;
627626
struct anon_vma *anon_vma = NULL;
@@ -631,17 +630,25 @@ int vma_adjust(struct vm_area_struct *vma, unsigned long start,
631630
int remove_next = 0;
632631

633632
if (next && !insert) {
634-
struct vm_area_struct *exporter = NULL;
633+
struct vm_area_struct *exporter = NULL, *importer = NULL;
635634

636635
if (end >= next->vm_end) {
637636
/*
638637
* vma expands, overlapping all the next, and
639638
* perhaps the one after too (mprotect case 6).
640639
*/
641-
again: remove_next = 1 + (end > next->vm_end);
640+
remove_next = 1 + (end > next->vm_end);
642641
end = next->vm_end;
643642
exporter = next;
644643
importer = vma;
644+
645+
/*
646+
* If next doesn't have anon_vma, import from vma after
647+
* next, if the vma overlaps with it.
648+
*/
649+
if (remove_next == 2 && next && !next->anon_vma)
650+
exporter = next->vm_next;
651+
645652
} else if (end > next->vm_start) {
646653
/*
647654
* vma expands, overlapping part of the next:
@@ -675,7 +682,7 @@ again: remove_next = 1 + (end > next->vm_end);
675682
return error;
676683
}
677684
}
678-
685+
again:
679686
vma_adjust_trans_huge(vma, start, end, adjust_next);
680687

681688
if (file) {
@@ -796,8 +803,11 @@ again: remove_next = 1 + (end > next->vm_end);
796803
* up the code too much to do both in one go.
797804
*/
798805
next = vma->vm_next;
799-
if (remove_next == 2)
806+
if (remove_next == 2) {
807+
remove_next = 1;
808+
end = next->vm_end;
800809
goto again;
810+
}
801811
else if (next)
802812
vma_gap_update(next);
803813
else

0 commit comments

Comments
 (0)