Skip to content

Commit 8f26e0b

Browse files
aagittorvalds
authored andcommitted
mm: vma_merge: correct false positive from __vma_unlink->validate_mm_rb
The old code was always doing: vma->vm_end = next->vm_end vma_rb_erase(next) // in __vma_unlink vma->vm_next = next->vm_next // in __vma_unlink next = vma->vm_next vma_gap_update(next) The new code still does the above for remove_next == 1 and 2, but for remove_next == 3 it has been changed and it does: next->vm_start = vma->vm_start vma_rb_erase(vma) // in __vma_unlink vma_gap_update(next) In the latter case, while unlinking "vma", validate_mm_rb() is told to ignore "vma" that is being removed, but next->vm_start was reduced instead. So for the new case, to avoid the false positive from validate_mm_rb, it should be "next" that is ignored when "vma" is being unlinked. "vma" and "next" in the above comment, considered pre-swap(). Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Andrea Arcangeli <[email protected]> Tested-by: Shaun Tancheff <[email protected]> Cc: Rik van Riel <[email protected]> Cc: Hugh Dickins <[email protected]> Cc: Mel Gorman <[email protected]> Cc: Jan Vorlicek <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 86d12e4 commit 8f26e0b

File tree

1 file changed

+41
-18
lines changed

1 file changed

+41
-18
lines changed

mm/mmap.c

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -402,14 +402,8 @@ static inline void vma_rb_insert(struct vm_area_struct *vma,
402402
rb_insert_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
403403
}
404404

405-
static void vma_rb_erase(struct vm_area_struct *vma, struct rb_root *root)
405+
static void __vma_rb_erase(struct vm_area_struct *vma, struct rb_root *root)
406406
{
407-
/*
408-
* All rb_subtree_gap values must be consistent prior to erase,
409-
* with the possible exception of the vma being erased.
410-
*/
411-
validate_mm_rb(root, vma);
412-
413407
/*
414408
* Note rb_erase_augmented is a fairly large inline function,
415409
* so make sure we instantiate it only once with our desired
@@ -418,6 +412,32 @@ static void vma_rb_erase(struct vm_area_struct *vma, struct rb_root *root)
418412
rb_erase_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
419413
}
420414

415+
static __always_inline void vma_rb_erase_ignore(struct vm_area_struct *vma,
416+
struct rb_root *root,
417+
struct vm_area_struct *ignore)
418+
{
419+
/*
420+
* All rb_subtree_gap values must be consistent prior to erase,
421+
* with the possible exception of the "next" vma being erased if
422+
* next->vm_start was reduced.
423+
*/
424+
validate_mm_rb(root, ignore);
425+
426+
__vma_rb_erase(vma, root);
427+
}
428+
429+
static __always_inline void vma_rb_erase(struct vm_area_struct *vma,
430+
struct rb_root *root)
431+
{
432+
/*
433+
* All rb_subtree_gap values must be consistent prior to erase,
434+
* with the possible exception of the vma being erased.
435+
*/
436+
validate_mm_rb(root, vma);
437+
438+
__vma_rb_erase(vma, root);
439+
}
440+
421441
/*
422442
* vma has some anon_vma assigned, and is already inserted on that
423443
* anon_vma's interval trees.
@@ -604,11 +624,12 @@ static void __insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
604624
static __always_inline void __vma_unlink_common(struct mm_struct *mm,
605625
struct vm_area_struct *vma,
606626
struct vm_area_struct *prev,
607-
bool has_prev)
627+
bool has_prev,
628+
struct vm_area_struct *ignore)
608629
{
609630
struct vm_area_struct *next;
610631

611-
vma_rb_erase(vma, &mm->mm_rb);
632+
vma_rb_erase_ignore(vma, &mm->mm_rb, ignore);
612633
next = vma->vm_next;
613634
if (has_prev)
614635
prev->vm_next = next;
@@ -630,13 +651,7 @@ static inline void __vma_unlink_prev(struct mm_struct *mm,
630651
struct vm_area_struct *vma,
631652
struct vm_area_struct *prev)
632653
{
633-
__vma_unlink_common(mm, vma, prev, true);
634-
}
635-
636-
static inline void __vma_unlink(struct mm_struct *mm,
637-
struct vm_area_struct *vma)
638-
{
639-
__vma_unlink_common(mm, vma, NULL, false);
654+
__vma_unlink_common(mm, vma, prev, true, vma);
640655
}
641656

642657
/*
@@ -815,8 +830,16 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
815830
if (remove_next != 3)
816831
__vma_unlink_prev(mm, next, vma);
817832
else
818-
/* vma is not before next if they've been swapped */
819-
__vma_unlink(mm, next);
833+
/*
834+
* vma is not before next if they've been
835+
* swapped.
836+
*
837+
* pre-swap() next->vm_start was reduced so
838+
* tell validate_mm_rb to ignore pre-swap()
839+
* "next" (which is stored in post-swap()
840+
* "vma").
841+
*/
842+
__vma_unlink_common(mm, next, NULL, false, vma);
820843
if (file)
821844
__remove_shared_vm_struct(next, file, mapping);
822845
} else if (insert) {

0 commit comments

Comments
 (0)