From 31f2b0ebc01fd332cb0997f7ce9f9cde29af9e20 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Fri, 23 Apr 2010 13:18:01 -0400 Subject: rmap: anon_vma_prepare() can leak anon_vma_chain If find_mergeable_anon_vma() succeeds but another thread installs ->anon_vma before we take ptl, then allocated == NULL but avc should be freed. Change the code to check avc != NULL to detect this case. Also, a couple of whitespace changes to make the critical section more visible. Signed-off-by: Oleg Nesterov Reviewed-by: Rik van Riel Cc: Hugh Dickins Cc: Pete Zaitcev Cc: Borislav Petkov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/rmap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'mm/rmap.c') diff --git a/mm/rmap.c b/mm/rmap.c index 526704e8215d..07fc94758799 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -133,8 +133,8 @@ int anon_vma_prepare(struct vm_area_struct *vma) goto out_enomem_free_avc; allocated = anon_vma; } - spin_lock(&anon_vma->lock); + spin_lock(&anon_vma->lock); /* page_table_lock to protect against threads */ spin_lock(&mm->page_table_lock); if (likely(!vma->anon_vma)) { @@ -144,14 +144,15 @@ int anon_vma_prepare(struct vm_area_struct *vma) list_add(&avc->same_vma, &vma->anon_vma_chain); list_add(&avc->same_anon_vma, &anon_vma->head); allocated = NULL; + avc = NULL; } spin_unlock(&mm->page_table_lock); - spin_unlock(&anon_vma->lock); - if (unlikely(allocated)) { + + if (unlikely(allocated)) anon_vma_free(allocated); + if (unlikely(avc)) anon_vma_chain_free(avc); - } } return 0; -- cgit v1.2.3-70-g09d2 From ab941e0fff3947b6dcc9c578d918d1bba54a6874 Mon Sep 17 00:00:00 2001 From: Naoya Horiguchi Date: Tue, 11 May 2010 14:06:55 -0700 Subject: rmap: remove anon_vma check in page_address_in_vma() Currently page_address_in_vma() compares vma->anon_vma and page_anon_vma(page) for parameter check, but in 2.6.34 a vma can have multiple anon_vmas with anon_vma_chain, so current check does not work. (For anonymous page shared by multiple processes, some verified (page,vma) pairs return -EFAULT wrongly.) We can go to checking all anon_vmas in the "same_vma" chain, but it needs to meet lock requirement. Instead, we can remove anon_vma check safely because page_address_in_vma() assumes that page and vma are already checked to belong to the identical process. Signed-off-by: Naoya Horiguchi Reviewed-by: Rik van Riel Cc: Andi Kleen Cc: Andrea Arcangeli Cc: Mel Gorman Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/rmap.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'mm/rmap.c') diff --git a/mm/rmap.c b/mm/rmap.c index 07fc94758799..0feeef860a8f 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -336,14 +336,13 @@ vma_address(struct page *page, struct vm_area_struct *vma) /* * At what user virtual address is page expected in vma? - * checking that the page matches the vma. + * Caller should check the page is actually part of the vma. */ unsigned long page_address_in_vma(struct page *page, struct vm_area_struct *vma) { - if (PageAnon(page)) { - if (vma->anon_vma != page_anon_vma(page)) - return -EFAULT; - } else if (page->mapping && !(vma->vm_flags & VM_NONLINEAR)) { + if (PageAnon(page)) + ; + else if (page->mapping && !(vma->vm_flags & VM_NONLINEAR)) { if (!vma->vm_file || vma->vm_file->f_mapping != page->mapping) return -EFAULT; -- cgit v1.2.3-70-g09d2