Re: [PATCH] kasan: avoid overflowing quarantine size on low memory systems

From: Alexander Potapenko
Date: Tue Aug 02 2016 - 06:05:36 EST


On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
<aryabinin@xxxxxxxxxxxxx> wrote:
>
>
> On 08/01/2016 05:59 PM, Alexander Potapenko wrote:
>> If the total amount of memory assigned to quarantine is less than the
>> amount of memory assigned to per-cpu quarantines, |new_quarantine_size|
>> may overflow. Instead, set it to zero.
>>
>
> Just curious, how did find this?
> Overflow is possible if system has more than 32 cpus per GB of memory. AFIAK this quite unusual.
We were just reading the quarantine code, and Dmitry spotted the problem.
I agree this is quite unusual, but we'd better prevent this case.

>> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>> implementation")
>> Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx>
>> ---
>> mm/kasan/quarantine.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
>> index 65793f1..416d3b0 100644
>> --- a/mm/kasan/quarantine.c
>> +++ b/mm/kasan/quarantine.c
>> @@ -196,7 +196,7 @@ void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache)
>>
>> void quarantine_reduce(void)
>> {
>> - size_t new_quarantine_size;
>> + size_t new_quarantine_size, percpu_quarantines;
>> unsigned long flags;
>> struct qlist_head to_free = QLIST_INIT;
>> size_t size_to_free = 0;
>> @@ -214,7 +214,15 @@ void quarantine_reduce(void)
>> */
>> new_quarantine_size = (READ_ONCE(totalram_pages) << PAGE_SHIFT) /
>> QUARANTINE_FRACTION;
>> - new_quarantine_size -= QUARANTINE_PERCPU_SIZE * num_online_cpus();
>> + percpu_quarantines = QUARANTINE_PERCPU_SIZE * num_online_cpus();
>> + if (new_quarantine_size < percpu_quarantines) {
>> + WARN_ONCE(1,
>> + "Too little memory, disabling global KASAN quarantine.\n",
>> + );
>
> Why WARN? I'd suggest pr_warn_once();
Agreed. I'll send the updated patch.
(Sorry, Andrew, I'll have to get back to the non-tidy version then, as
pr_warn_once() doesn't return the predicate value)
>> + new_quarantine_size = 0;
>> + } else {
>> + new_quarantine_size -= percpu_quarantines;
>> + }
>> WRITE_ONCE(quarantine_size, new_quarantine_size);
>>
>> last = global_quarantine.head;
>>



--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-StraÃe, 33
80636 MÃnchen

GeschÃftsfÃhrer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg