Re: [PATCH] kasan: fix races in quarantine_remove_cache()

[Andrey Ryabinin <aryabinin@virtuozzo.com>]

On 03/08/2017 06:15 PM, Dmitry Vyukov wrote:

> diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
> index 6f1ed1630873..075422c3cee3 100644
> --- a/mm/kasan/quarantine.c
> +++ b/mm/kasan/quarantine.c
> @@ -27,6 +27,7 @@
>  #include <linux/slab.h>
>  #include <linux/string.h>
>  #include <linux/types.h>
> +#include <linux/srcu.h>
>  

Nit: keep alphabetical order please.


>  
>  void quarantine_reduce(void)
>  {
>  	size_t total_size, new_quarantine_size, percpu_quarantines;
>  	unsigned long flags;
> +	int srcu_idx;
>  	struct qlist_head to_free = QLIST_INIT;
>  
>  	if (likely(READ_ONCE(quarantine_size) <=
>  		   READ_ONCE(quarantine_max_size)))
>  		return;
>  
> +	/*
> +	 * srcu critical section ensures that quarantine_remove_cache()
> +	 * will not miss objects belonging to the cache while they are in our
> +	 * local to_free list. srcu is chosen because (1) it gives us private
> +	 * grace period domain that does not interfere with anything else,
> +	 * and (2) it allows synchronize_srcu() to return without waiting
> +	 * if there are no pending read critical sections (which is the
> +	 * expected case).
> +	 */
> +	srcu_idx = srcu_read_lock(&remove_cache_srcu);

I'm puzzled why is SRCU, why not RCU? Given that we take spin_lock in the next line
we certainly don't need ability to sleep in read-side critical section.

>  	spin_lock_irqsave(&quarantine_lock, flags);
>  
>  	/*
> @@ -237,6 +257,7 @@ void quarantine_reduce(void)
>  	spin_unlock_irqrestore(&quarantine_lock, flags);
>  
>  	qlist_free_all(&to_free, NULL);
> +	srcu_read_unlock(&remove_cache_srcu, srcu_idx);
>  }
>  

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>