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

[Dmitry Vyukov <dvyukov@google.com>]

On Thu, Mar 9, 2017 at 10:25 AM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> 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.

Doh, we really need clang-format. This is not productive.
Will send v2.


>>  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.

I've explained it in the comment above.




>>       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>