Patchwork [1/9] mm: khugepaged: close use-after-free race during shmem collapsing

login
register
mail settings
Submitter Johannes Weiner
Date Nov. 17, 2016, 7:11 p.m.
Message ID <20161117191138.22769-2-hannes@cmpxchg.org>
Download mbox | patch
Permalink /patch/9435185/
State New
Headers show

Comments

Johannes Weiner - Nov. 17, 2016, 7:11 p.m.
When a radix tree iteration drops the tree lock, another thread might
swoop in and free the node holding the current slot. The iteration
needs to do another tree lookup from the current index to continue.

[kirill.shutemov@linux.intel.com: re-lookup for replacement]
Fixes: f3f0e1d2150b ("khugepaged: add support of collapse for tmpfs/shmem pages")
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/khugepaged.c | 5 +++++
 1 file changed, 5 insertions(+)
Kirill A. Shutemov - Nov. 17, 2016, 11:19 p.m.
On Thu, Nov 17, 2016 at 02:11:30PM -0500, Johannes Weiner wrote:
> When a radix tree iteration drops the tree lock, another thread might
> swoop in and free the node holding the current slot. The iteration
> needs to do another tree lookup from the current index to continue.
> 
> [kirill.shutemov@linux.intel.com: re-lookup for replacement]
> Fixes: f3f0e1d2150b ("khugepaged: add support of collapse for tmpfs/shmem pages")
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Jan Kara - Nov. 18, 2016, 7:29 a.m.
On Thu 17-11-16 14:11:30, Johannes Weiner wrote:
> When a radix tree iteration drops the tree lock, another thread might
> swoop in and free the node holding the current slot. The iteration
> needs to do another tree lookup from the current index to continue.
> 
> [kirill.shutemov@linux.intel.com: re-lookup for replacement]
> Fixes: f3f0e1d2150b ("khugepaged: add support of collapse for tmpfs/shmem pages")
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  mm/khugepaged.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 728d7790dc2d..bdfdab40a813 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1401,6 +1401,9 @@ static void collapse_shmem(struct mm_struct *mm,
>  
>  		spin_lock_irq(&mapping->tree_lock);
>  
> +		slot = radix_tree_lookup_slot(&mapping->page_tree, index);
> +		VM_BUG_ON_PAGE(page != radix_tree_deref_slot_protected(slot,
> +					&mapping->tree_lock), page);
>  		VM_BUG_ON_PAGE(page_mapped(page), page);
>  
>  		/*
> @@ -1424,6 +1427,7 @@ static void collapse_shmem(struct mm_struct *mm,
>  		radix_tree_replace_slot(slot,
>  				new_page + (index % HPAGE_PMD_NR));
>  
> +		slot = radix_tree_iter_next(&iter);
>  		index++;
>  		continue;
>  out_lru:
> @@ -1535,6 +1539,7 @@ static void collapse_shmem(struct mm_struct *mm,
>  			putback_lru_page(page);
>  			unlock_page(page);
>  			spin_lock_irq(&mapping->tree_lock);
> +			slot = radix_tree_iter_next(&iter);
>  		}
>  		VM_BUG_ON(nr_none);
>  		spin_unlock_irq(&mapping->tree_lock);
> -- 
> 2.10.2
>

Patch

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 728d7790dc2d..bdfdab40a813 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1401,6 +1401,9 @@  static void collapse_shmem(struct mm_struct *mm,
 
 		spin_lock_irq(&mapping->tree_lock);
 
+		slot = radix_tree_lookup_slot(&mapping->page_tree, index);
+		VM_BUG_ON_PAGE(page != radix_tree_deref_slot_protected(slot,
+					&mapping->tree_lock), page);
 		VM_BUG_ON_PAGE(page_mapped(page), page);
 
 		/*
@@ -1424,6 +1427,7 @@  static void collapse_shmem(struct mm_struct *mm,
 		radix_tree_replace_slot(slot,
 				new_page + (index % HPAGE_PMD_NR));
 
+		slot = radix_tree_iter_next(&iter);
 		index++;
 		continue;
 out_lru:
@@ -1535,6 +1539,7 @@  static void collapse_shmem(struct mm_struct *mm,
 			putback_lru_page(page);
 			unlock_page(page);
 			spin_lock_irq(&mapping->tree_lock);
+			slot = radix_tree_iter_next(&iter);
 		}
 		VM_BUG_ON(nr_none);
 		spin_unlock_irq(&mapping->tree_lock);