[Devel,RHEL7,COMMIT] mm/tcache: close race between attach and invalidate page

Submitted by Konstantin Khorenko on April 7, 2017, 12:23 p.m.

Details

Message ID 201704071223.v37CN7Fu026169@finist_cl7.x64_64.work.ct
State New
Series "mm/tcache: close race between attach and invalidate page"
Headers show

Commit Message

Konstantin Khorenko April 7, 2017, 12:23 p.m.
The commit is pushed to "branch-rh7-3.10.0-514.10.2.vz7.29.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-514.10.2.vz7.29.13
------>
commit 7551b5c4ae61988369226c24b83717f0d3ef5aef
Author: Andrey Ryabinin <aryabinin@virtuozzo.com>
Date:   Fri Apr 7 16:23:07 2017 +0400

    mm/tcache: close race between attach and invalidate page
    
    Currently the following race between page attach and invalidate page
    could result in crashes:
    
                CPU1                                      CPU2
     tcache_cleancache_put_page();
        tcache_attach_page();
           tcache_page_tree_replace();
                                                  tcache_invalidate_node_pages();
    						page = radix_tree_deref_slot_protected();
                                                    __tcache_page_tree_delete(page);
                                                    tcache_lru_del(page);
                                                    tcache_put_page(page);  <-- this frees the page
            ...
            tcache_hold_page(page); // page may freed at this point
            tcache_lru_add(node->pool, page);
    
    To prevent this, protect with node->tree_lock not only insertion to radix
    tree, but also tcache_hold_page() and addition to the lru list.
    So if we extract the page from radix_tree now we can be sure that
    it is already on lru list and page refcounter increased.
    
    https://jira.sw.ru/browse/PSBM-63197
    
    Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 mm/tcache.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/mm/tcache.c b/mm/tcache.c
index c7613b2..e15d212 100644
--- a/mm/tcache.c
+++ b/mm/tcache.c
@@ -724,7 +724,6 @@  static int tcache_page_tree_replace(struct tcache_node *node, pgoff_t index,
 
 	*old_page = NULL;
 
-	spin_lock(&node->tree_lock);
 	/*
 	 * If the node was invalidated after we looked it up, abort in order to
 	 * avoid clashes with tcache_invalidate_node_pages.
@@ -752,7 +751,6 @@  static int tcache_page_tree_replace(struct tcache_node *node, pgoff_t index,
 		}
 	}
 out:
-	spin_unlock(&node->tree_lock);
 	return err;
 }
 
@@ -792,7 +790,7 @@  tcache_attach_page(struct tcache_node *node, pgoff_t index, struct page *page)
 
 	tcache_init_page(page, node, index);
 
-	local_irq_save(flags);
+	spin_lock_irqsave(&node->tree_lock, flags);
 	err = tcache_page_tree_replace(node, index, page, &old_page);
 	if (err)
 		goto out;
@@ -804,7 +802,7 @@  tcache_attach_page(struct tcache_node *node, pgoff_t index, struct page *page)
 	tcache_hold_page(page);
 	tcache_lru_add(node->pool, page);
 out:
-	local_irq_restore(flags);
+	spin_unlock_irqrestore(&node->tree_lock, flags);
 	return err;
 }
 

Comments

Konstantin Khorenko April 7, 2017, 12:26 p.m.
Please consider to create a ReadyKernel patch for it.

https://readykernel.com/

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 04/07/2017 03:23 PM, Konstantin Khorenko wrote:
> The commit is pushed to "branch-rh7-3.10.0-514.10.2.vz7.29.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
> after rh7-3.10.0-514.10.2.vz7.29.13
> ------>
> commit 7551b5c4ae61988369226c24b83717f0d3ef5aef
> Author: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Date:   Fri Apr 7 16:23:07 2017 +0400
>
>     mm/tcache: close race between attach and invalidate page
>
>     Currently the following race between page attach and invalidate page
>     could result in crashes:
>
>                 CPU1                                      CPU2
>      tcache_cleancache_put_page();
>         tcache_attach_page();
>            tcache_page_tree_replace();
>                                                   tcache_invalidate_node_pages();
>     						page = radix_tree_deref_slot_protected();
>                                                     __tcache_page_tree_delete(page);
>                                                     tcache_lru_del(page);
>                                                     tcache_put_page(page);  <-- this frees the page
>             ...
>             tcache_hold_page(page); // page may freed at this point
>             tcache_lru_add(node->pool, page);
>
>     To prevent this, protect with node->tree_lock not only insertion to radix
>     tree, but also tcache_hold_page() and addition to the lru list.
>     So if we extract the page from radix_tree now we can be sure that
>     it is already on lru list and page refcounter increased.
>
>     https://jira.sw.ru/browse/PSBM-63197
>
>     Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  mm/tcache.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/mm/tcache.c b/mm/tcache.c
> index c7613b2..e15d212 100644
> --- a/mm/tcache.c
> +++ b/mm/tcache.c
> @@ -724,7 +724,6 @@ static int tcache_page_tree_replace(struct tcache_node *node, pgoff_t index,
>
>  	*old_page = NULL;
>
> -	spin_lock(&node->tree_lock);
>  	/*
>  	 * If the node was invalidated after we looked it up, abort in order to
>  	 * avoid clashes with tcache_invalidate_node_pages.
> @@ -752,7 +751,6 @@ static int tcache_page_tree_replace(struct tcache_node *node, pgoff_t index,
>  		}
>  	}
>  out:
> -	spin_unlock(&node->tree_lock);
>  	return err;
>  }
>
> @@ -792,7 +790,7 @@ tcache_attach_page(struct tcache_node *node, pgoff_t index, struct page *page)
>
>  	tcache_init_page(page, node, index);
>
> -	local_irq_save(flags);
> +	spin_lock_irqsave(&node->tree_lock, flags);
>  	err = tcache_page_tree_replace(node, index, page, &old_page);
>  	if (err)
>  		goto out;
> @@ -804,7 +802,7 @@ tcache_attach_page(struct tcache_node *node, pgoff_t index, struct page *page)
>  	tcache_hold_page(page);
>  	tcache_lru_add(node->pool, page);
>  out:
> -	local_irq_restore(flags);
> +	spin_unlock_irqrestore(&node->tree_lock, flags);
>  	return err;
>  }
>
> .
>