hugetlb: acquire the i_mmap_lock before walking the prio_tree to unmap a page

When the owner of a mapping fails COW because a child process is holding a reference, the children VMAs are walked and the page is unmapped. The i_mmap_lock is taken for the unmapping of the page but not the walking of the prio_tree. In theory, that tree could be changing if the lock is not held. This patch takes the i_mmap_lock properly for the duration of the prio_tree walk. [hugh.dickins@tiscali.co.uk: Spotted the problem in the first place] Signed-off-by: Mel Gorman <mel@csn.ul.ie> Acked-by: Hugh Dickins <hugh.dickins@tiscali.co.uk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

hugetlb: acquire the i_mmap_lock before walking the prio_tree to unmap a page
When the owner of a mapping fails COW because a child process is holding a reference, the children VMAs are walked and the page is unmapped. The i_mmap_lock is taken for the unmapping of the page but not the walking of the prio_tree. In theory, that tree could be changing if the lock is not held. This patch takes the i_mmap_lock properly for the duration of the prio_tree walk. [hugh.dickins@tiscali.co.uk: Spotted the problem in the first place] Signed-off-by: Mel Gorman <mel@csn.ul.ie> Acked-by: Hugh Dickins <hugh.dickins@tiscali.co.uk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Mel Gorman · Linus Torvalds
1 parent 70da2340fb
Showing 1 changed file with 8 additions and 1 deletions Side-by-side Diff
mm/hugetlb.c
@@ -2237,6 +2237,12 @@
 		+ (vma->vm_pgoff >> PAGE_SHIFT);
 	mapping = (struct address_space *)page_private(page);
  
+	/*
+	 * Take the mapping lock for the duration of the table walk. As
+	 * this mapping should be shared between all the VMAs,
+	 * __unmap_hugepage_range() is called as the lock is already held
+	 */
+	spin_lock(&mapping->i_mmap_lock);
 	vma_prio_tree_foreach(iter_vma, &iter, &mapping->i_mmap, pgoff, pgoff) {
 		/* Do not unmap the current VMA */
 		if (iter_vma == vma)
  
@@ -2250,10 +2256,11 @@
 		 * from the time of fork. This would look like data corruption
 		 */
 		if (!is_vma_resv_set(iter_vma, HPAGE_RESV_OWNER))
-			unmap_hugepage_range(iter_vma,
+			__unmap_hugepage_range(iter_vma,
 				address, address + huge_page_size(h),
 				page);
 	}
+	spin_unlock(&mapping->i_mmap_lock);
  
 	return 1;
 }
...	...	@@ -2237,6 +2237,12 @@
2237	2237	+ (vma->vm_pgoff >> PAGE_SHIFT);
2238	2238	mapping = (struct address_space *)page_private(page);
2239	2239
	2240	+ /*
	2241	+ * Take the mapping lock for the duration of the table walk. As
	2242	+ * this mapping should be shared between all the VMAs,
	2243	+ * __unmap_hugepage_range() is called as the lock is already held
	2244	+ */
	2245	+ spin_lock(&mapping->i_mmap_lock);
2240	2246	vma_prio_tree_foreach(iter_vma, &iter, &mapping->i_mmap, pgoff, pgoff) {
2241	2247	/* Do not unmap the current VMA */
2242	2248	if (iter_vma == vma)
2243	2249
...	...	@@ -2250,10 +2256,11 @@
2250	2256	* from the time of fork. This would look like data corruption
2251	2257	*/
2252	2258	if (!is_vma_resv_set(iter_vma, HPAGE_RESV_OWNER))
2253		- unmap_hugepage_range(iter_vma,
	2259	+ __unmap_hugepage_range(iter_vma,
2254	2260	address, address + huge_page_size(h),
2255	2261	page);
2256	2262	}
	2263	+ spin_unlock(&mapping->i_mmap_lock);
2257	2264
2258	2265	return 1;
2259	2266	}