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

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

[Alexander Potapenko]

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.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
implementation")
Signed-off-by: Alexander Potapenko <glider@google.com>
---
 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",
+		);
+		new_quarantine_size = 0;
+	} else {
+		new_quarantine_size -= percpu_quarantines;
+	}
 	WRITE_ONCE(quarantine_size, new_quarantine_size);
 
 	last = global_quarantine.head;
-- 
2.8.0.rc3.226.g39d4020

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

[Alexander Potapenko]

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.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
implementation")
Signed-off-by: Alexander Potapenko <glider@google.com>
---
 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",
+		);
+		new_quarantine_size = 0;
+	} else {
+		new_quarantine_size -= percpu_quarantines;
+	}
 	WRITE_ONCE(quarantine_size, new_quarantine_size);
 
 	last = global_quarantine.head;
-- 
2.8.0.rc3.226.g39d4020

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

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

[Andrew Morton]

On Mon,  1 Aug 2016 16:59:23 +0200 Alexander Potapenko <glider@google.com> 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.
> 
> --- a/mm/kasan/quarantine.c
> +++ b/mm/kasan/quarantine.c
> @@ -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",
> +		);
> +		new_quarantine_size = 0;
> +	} else {
> +		new_quarantine_size -= percpu_quarantines;
> +	}
>  	WRITE_ONCE(quarantine_size, new_quarantine_size);
>  
>  	last = global_quarantine.head;

This is a little tidier:

--- a/mm/kasan/quarantine.c~kasan-avoid-overflowing-quarantine-size-on-low-memory-systems-fix
+++ a/mm/kasan/quarantine.c
@@ -217,14 +217,11 @@ void quarantine_reduce(void)
 	new_quarantine_size = (READ_ONCE(totalram_pages) << PAGE_SHIFT) /
 		QUARANTINE_FRACTION;
 	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",
-		);
+	if (WARN_ONCE(new_quarantine_size < percpu_quarantines,
+		"Too little memory, disabling global KASAN quarantine.\n"))
 		new_quarantine_size = 0;
-	} else {
+	else
 		new_quarantine_size -= percpu_quarantines;
-	}
 	WRITE_ONCE(quarantine_size, new_quarantine_size);
 
 	last = global_quarantine.head;
_

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

[Andrew Morton]

On Mon,  1 Aug 2016 16:59:23 +0200 Alexander Potapenko <glider@google.com> 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.
> 
> --- a/mm/kasan/quarantine.c
> +++ b/mm/kasan/quarantine.c
> @@ -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",
> +		);
> +		new_quarantine_size = 0;
> +	} else {
> +		new_quarantine_size -= percpu_quarantines;
> +	}
>  	WRITE_ONCE(quarantine_size, new_quarantine_size);
>  
>  	last = global_quarantine.head;

This is a little tidier:

--- a/mm/kasan/quarantine.c~kasan-avoid-overflowing-quarantine-size-on-low-memory-systems-fix
+++ a/mm/kasan/quarantine.c
@@ -217,14 +217,11 @@ void quarantine_reduce(void)
 	new_quarantine_size = (READ_ONCE(totalram_pages) << PAGE_SHIFT) /
 		QUARANTINE_FRACTION;
 	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",
-		);
+	if (WARN_ONCE(new_quarantine_size < percpu_quarantines,
+		"Too little memory, disabling global KASAN quarantine.\n"))
 		new_quarantine_size = 0;
-	} else {
+	else
 		new_quarantine_size -= percpu_quarantines;
-	}
 	WRITE_ONCE(quarantine_size, new_quarantine_size);
 
 	last = global_quarantine.head;
_

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

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

[Andrey Ryabinin]

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. 

> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
> implementation")
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
>  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();

> +		new_quarantine_size = 0;
> +	} else {
> +		new_quarantine_size -= percpu_quarantines;
> +	}
>  	WRITE_ONCE(quarantine_size, new_quarantine_size);
>  
>  	last = global_quarantine.head;
> 

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

[Andrey Ryabinin]

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. 

> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
> implementation")
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
>  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();

> +		new_quarantine_size = 0;
> +	} else {
> +		new_quarantine_size -= percpu_quarantines;
> +	}
>  	WRITE_ONCE(quarantine_size, new_quarantine_size);
>  
>  	last = global_quarantine.head;
> 

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

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

[Dmitry Vyukov]

On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> 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.

I was reading code for unrelated reason.

>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>> implementation")
>> Signed-off-by: Alexander Potapenko <glider@google.com>
>> ---
>>  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();


I would suggest to just do something useful. Setting quarantine
new_quarantine_size to 0 looks fine.
What would user do with this warning? Number of CPUs and amount of
memory are generally fixed. Why is it an issue for end user at all? We
still have some quarantine per-cpu. A WARNING means a [non-critical]
kernel bug. E.g. syzkaller will catch each and every boot of such
system as a bug.


>> +             new_quarantine_size = 0;
>> +     } else {
>> +             new_quarantine_size -= percpu_quarantines;
>> +     }
>>       WRITE_ONCE(quarantine_size, new_quarantine_size);
>>
>>       last = global_quarantine.head;
>>

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

[Dmitry Vyukov]

On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> 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.

I was reading code for unrelated reason.

>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>> implementation")
>> Signed-off-by: Alexander Potapenko <glider@google.com>
>> ---
>>  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();


I would suggest to just do something useful. Setting quarantine
new_quarantine_size to 0 looks fine.
What would user do with this warning? Number of CPUs and amount of
memory are generally fixed. Why is it an issue for end user at all? We
still have some quarantine per-cpu. A WARNING means a [non-critical]
kernel bug. E.g. syzkaller will catch each and every boot of such
system as a bug.


>> +             new_quarantine_size = 0;
>> +     } else {
>> +             new_quarantine_size -= percpu_quarantines;
>> +     }
>>       WRITE_ONCE(quarantine_size, new_quarantine_size);
>>
>>       last = global_quarantine.head;
>>

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

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

[Alexander Potapenko]

On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> 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@google.com>
>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>> implementation")
>> Signed-off-by: Alexander Potapenko <glider@google.com>
>> ---
>>  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

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

[Alexander Potapenko]

On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> 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@google.com>
>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>> implementation")
>> Signed-off-by: Alexander Potapenko <glider@google.com>
>> ---
>>  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

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

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

[Alexander Potapenko]

On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> 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.
>
> I was reading code for unrelated reason.
>
>>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>>> implementation")
>>> Signed-off-by: Alexander Potapenko <glider@google.com>
>>> ---
>>>  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();
>
>
> I would suggest to just do something useful. Setting quarantine
> new_quarantine_size to 0 looks fine.
> What would user do with this warning? Number of CPUs and amount of
> memory are generally fixed. Why is it an issue for end user at all? We
> still have some quarantine per-cpu. A WARNING means a [non-critical]
> kernel bug. E.g. syzkaller will catch each and every boot of such
> system as a bug.
How about printk_once then?
Silently setting the quarantine size to zero may puzzle the user.
>
>>> +             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

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

[Alexander Potapenko]

On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> 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.
>
> I was reading code for unrelated reason.
>
>>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>>> implementation")
>>> Signed-off-by: Alexander Potapenko <glider@google.com>
>>> ---
>>>  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();
>
>
> I would suggest to just do something useful. Setting quarantine
> new_quarantine_size to 0 looks fine.
> What would user do with this warning? Number of CPUs and amount of
> memory are generally fixed. Why is it an issue for end user at all? We
> still have some quarantine per-cpu. A WARNING means a [non-critical]
> kernel bug. E.g. syzkaller will catch each and every boot of such
> system as a bug.
How about printk_once then?
Silently setting the quarantine size to zero may puzzle the user.
>
>>> +             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

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

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

[Dmitry Vyukov]

On Tue, Aug 2, 2016 at 12:07 PM, Alexander Potapenko <glider@google.com> wrote:
> On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
>> <aryabinin@virtuozzo.com> 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.
>>
>> I was reading code for unrelated reason.
>>
>>>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>>>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>>>> implementation")
>>>> Signed-off-by: Alexander Potapenko <glider@google.com>
>>>> ---
>>>>  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();
>>
>>
>> I would suggest to just do something useful. Setting quarantine
>> new_quarantine_size to 0 looks fine.
>> What would user do with this warning? Number of CPUs and amount of
>> memory are generally fixed. Why is it an issue for end user at all? We
>> still have some quarantine per-cpu. A WARNING means a [non-critical]
>> kernel bug. E.g. syzkaller will catch each and every boot of such
>> system as a bug.
> How about printk_once then?
> Silently setting the quarantine size to zero may puzzle the user.


We still have per-cpu quarantine.
new_quarantine_size==0 is not radically different from
new_quarantine_size==1. Both limit KASAN ability to detect UAF. Why do
we WARN in the former case but not in the latter?
We can print per-cpu/global quarantine sizes to console. Then if we
got any complaints we can figure out what happens from the log.



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

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

[Dmitry Vyukov]

On Tue, Aug 2, 2016 at 12:07 PM, Alexander Potapenko <glider@google.com> wrote:
> On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
>> <aryabinin@virtuozzo.com> 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.
>>
>> I was reading code for unrelated reason.
>>
>>>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>>>> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
>>>> implementation")
>>>> Signed-off-by: Alexander Potapenko <glider@google.com>
>>>> ---
>>>>  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();
>>
>>
>> I would suggest to just do something useful. Setting quarantine
>> new_quarantine_size to 0 looks fine.
>> What would user do with this warning? Number of CPUs and amount of
>> memory are generally fixed. Why is it an issue for end user at all? We
>> still have some quarantine per-cpu. A WARNING means a [non-critical]
>> kernel bug. E.g. syzkaller will catch each and every boot of such
>> system as a bug.
> How about printk_once then?
> Silently setting the quarantine size to zero may puzzle the user.


We still have per-cpu quarantine.
new_quarantine_size==0 is not radically different from
new_quarantine_size==1. Both limit KASAN ability to detect UAF. Why do
we WARN in the former case but not in the latter?
We can print per-cpu/global quarantine sizes to console. Then if we
got any complaints we can figure out what happens from the log.



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

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

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

[Andrey Ryabinin]

On 08/02/2016 01:07 PM, Alexander Potapenko wrote:
> On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
>>>
>>> Why WARN? I'd suggest pr_warn_once();
>>
>>
>> I would suggest to just do something useful. Setting quarantine
>> new_quarantine_size to 0 looks fine.
>> What would user do with this warning? Number of CPUs and amount of
>> memory are generally fixed. Why is it an issue for end user at all? We
>> still have some quarantine per-cpu. A WARNING means a [non-critical]
>> kernel bug. E.g. syzkaller will catch each and every boot of such
>> system as a bug.
> How about printk_once then?
> Silently setting the quarantine size to zero may puzzle the user.
>

Nope, user will not notice anything. So keeping it silent would be better.
Plus it's very unlikely that this will ever happen in real life.

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

[Andrey Ryabinin]

On 08/02/2016 01:07 PM, Alexander Potapenko wrote:
> On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
>>>
>>> Why WARN? I'd suggest pr_warn_once();
>>
>>
>> I would suggest to just do something useful. Setting quarantine
>> new_quarantine_size to 0 looks fine.
>> What would user do with this warning? Number of CPUs and amount of
>> memory are generally fixed. Why is it an issue for end user at all? We
>> still have some quarantine per-cpu. A WARNING means a [non-critical]
>> kernel bug. E.g. syzkaller will catch each and every boot of such
>> system as a bug.
> How about printk_once then?
> Silently setting the quarantine size to zero may puzzle the user.
>

Nope, user will not notice anything. So keeping it silent would be better.
Plus it's very unlikely that this will ever happen in real life.

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

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

[Alexander Potapenko]

On Tue, Aug 2, 2016 at 12:23 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
>
>
> On 08/02/2016 01:07 PM, Alexander Potapenko wrote:
>> On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
>>>>
>>>> Why WARN? I'd suggest pr_warn_once();
>>>
>>>
>>> I would suggest to just do something useful. Setting quarantine
>>> new_quarantine_size to 0 looks fine.
>>> What would user do with this warning? Number of CPUs and amount of
>>> memory are generally fixed. Why is it an issue for end user at all? We
>>> still have some quarantine per-cpu. A WARNING means a [non-critical]
>>> kernel bug. E.g. syzkaller will catch each and every boot of such
>>> system as a bug.
>> How about printk_once then?
>> Silently setting the quarantine size to zero may puzzle the user.
>>
>
> Nope, user will not notice anything. So keeping it silent would be better.
> Plus it's very unlikely that this will ever happen in real life.
>
Ok, I've sent out v2, please take a look.


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

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

[Alexander Potapenko]

On Tue, Aug 2, 2016 at 12:23 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
>
>
> On 08/02/2016 01:07 PM, Alexander Potapenko wrote:
>> On Tue, Aug 2, 2016 at 12:05 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> On Tue, Aug 2, 2016 at 12:00 PM, Andrey Ryabinin
>>>>
>>>> Why WARN? I'd suggest pr_warn_once();
>>>
>>>
>>> I would suggest to just do something useful. Setting quarantine
>>> new_quarantine_size to 0 looks fine.
>>> What would user do with this warning? Number of CPUs and amount of
>>> memory are generally fixed. Why is it an issue for end user at all? We
>>> still have some quarantine per-cpu. A WARNING means a [non-critical]
>>> kernel bug. E.g. syzkaller will catch each and every boot of such
>>> system as a bug.
>> How about printk_once then?
>> Silently setting the quarantine size to zero may puzzle the user.
>>
>
> Nope, user will not notice anything. So keeping it silent would be better.
> Plus it's very unlikely that this will ever happen in real life.
>
Ok, I've sent out v2, please take a look.


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

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

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

[amanda4ray]

[-- Attachment #1: Type: text/plain, Size: 3382 bytes --]

On Monday, August 1, 2016 at 10:59:29 AM UTC-4, 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.
> 
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
> implementation")
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
>  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",
> +		);
> +		new_quarantine_size = 0;
> +	} else {
> +		new_quarantine_size -= percpu_quarantines;
> +	}
>  	WRITE_ONCE(quarantine_size, new_quarantine_size);
>  
>  	last = global_quarantine.head;
> -- 
> 2.8.0.rc3.226.g39d4020



On Monday, August 1, 2016 at 10:59:29 AM UTC-4, 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.
> 
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Fixes: 55834c59098d ("mm: kasan: initial memory quarantine
> implementation")
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
>  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",
> +		);
> +		new_quarantine_size = 0;
> +	} else {
> +		new_quarantine_size -= percpu_quarantines;
> +	}
>  	WRITE_ONCE(quarantine_size, new_quarantine_size);
>  
>  	last = global_quarantine.head;
> -- 
> 2.8.0.rc3.226.g39d4020