All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kasan: report only the first error
@ 2017-03-22 16:06 ` Andrey Ryabinin
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-22 16:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mark Rutland, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
	linux-mm, linux-kernel, Andrey Ryabinin

Disable kasan after the first report. There are several reasons for this:
 * Single bug quite often has multiple invalid memory accesses causing
    storm in the dmesg.
 * Write OOB access might corrupt metadata so the next report will print
    bogus alloc/free stacktraces.
 * Reports after the first easily could be not bugs by itself but just side
    effects of the first one.

Given that multiple reports only do harm, it makes sense to disable
kasan after the first one. Except for the tests in lib/test_kasan.c
as we obviously want to see all reports from test.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 lib/test_kasan.c  | 9 +++++++++
 mm/kasan/report.c | 7 +++++++
 2 files changed, 16 insertions(+)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..5112663 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -11,6 +11,7 @@
 
 #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
 
+#include <linux/atomic.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
@@ -21,6 +22,8 @@
 #include <linux/uaccess.h>
 #include <linux/module.h>
 
+extern atomic_t kasan_report_count;
+
 /*
  * Note: test functions are marked noinline so that their names appear in
  * reports.
@@ -474,6 +477,9 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/* Rise reports limit high enough to see all the following bugs */
+	atomic_set(&kasan_report_count, 100);
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +505,9 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	/* kasan is unreliable now, disable reports */
+	atomic_set(&kasan_report_count, 0);
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 718a10a..7eab229 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,6 +13,7 @@
  *
  */
 
+#include <linux/atomic.h>
 #include <linux/ftrace.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -354,6 +355,9 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+atomic_t kasan_report_count = ATOMIC_INIT(1);
+EXPORT_SYMBOL_GPL(kasan_report_count);
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {
@@ -362,6 +366,9 @@ void kasan_report(unsigned long addr, size_t size,
 	if (likely(!kasan_report_enabled()))
 		return;
 
+	if (atomic_dec_if_positive(&kasan_report_count) < 0)
+		return;
+
 	disable_trace_on_warning();
 
 	info.access_addr = (void *)addr;
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH] kasan: report only the first error
@ 2017-03-22 16:06 ` Andrey Ryabinin
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-22 16:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mark Rutland, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
	linux-mm, linux-kernel, Andrey Ryabinin

Disable kasan after the first report. There are several reasons for this:
 * Single bug quite often has multiple invalid memory accesses causing
    storm in the dmesg.
 * Write OOB access might corrupt metadata so the next report will print
    bogus alloc/free stacktraces.
 * Reports after the first easily could be not bugs by itself but just side
    effects of the first one.

Given that multiple reports only do harm, it makes sense to disable
kasan after the first one. Except for the tests in lib/test_kasan.c
as we obviously want to see all reports from test.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 lib/test_kasan.c  | 9 +++++++++
 mm/kasan/report.c | 7 +++++++
 2 files changed, 16 insertions(+)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..5112663 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -11,6 +11,7 @@
 
 #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
 
+#include <linux/atomic.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
@@ -21,6 +22,8 @@
 #include <linux/uaccess.h>
 #include <linux/module.h>
 
+extern atomic_t kasan_report_count;
+
 /*
  * Note: test functions are marked noinline so that their names appear in
  * reports.
@@ -474,6 +477,9 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/* Rise reports limit high enough to see all the following bugs */
+	atomic_set(&kasan_report_count, 100);
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +505,9 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	/* kasan is unreliable now, disable reports */
+	atomic_set(&kasan_report_count, 0);
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 718a10a..7eab229 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,6 +13,7 @@
  *
  */
 
+#include <linux/atomic.h>
 #include <linux/ftrace.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -354,6 +355,9 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+atomic_t kasan_report_count = ATOMIC_INIT(1);
+EXPORT_SYMBOL_GPL(kasan_report_count);
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {
@@ -362,6 +366,9 @@ void kasan_report(unsigned long addr, size_t size,
 	if (likely(!kasan_report_enabled()))
 		return;
 
+	if (atomic_dec_if_positive(&kasan_report_count) < 0)
+		return;
+
 	disable_trace_on_warning();
 
 	info.access_addr = (void *)addr;
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
  2017-03-22 16:06 ` Andrey Ryabinin
@ 2017-03-22 16:34   ` Andrey Konovalov
  -1 siblings, 0 replies; 22+ messages in thread
From: Andrey Konovalov @ 2017-03-22 16:34 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Mark Rutland, Alexander Potapenko, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> Disable kasan after the first report. There are several reasons for this:
>  * Single bug quite often has multiple invalid memory accesses causing
>     storm in the dmesg.
>  * Write OOB access might corrupt metadata so the next report will print
>     bogus alloc/free stacktraces.
>  * Reports after the first easily could be not bugs by itself but just side
>     effects of the first one.
>
> Given that multiple reports only do harm, it makes sense to disable
> kasan after the first one. Except for the tests in lib/test_kasan.c
> as we obviously want to see all reports from test.

Hi Andrey,

Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
default to showing only the first report)?

I sometimes use KASAN to see what bad accesses a particular bug
causes, and seeing all of them (even knowing that they may be
corrupt/induced) helps a lot.

Thanks!

>
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  lib/test_kasan.c  | 9 +++++++++
>  mm/kasan/report.c | 7 +++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 0b1d314..5112663 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -11,6 +11,7 @@
>
>  #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
>
> +#include <linux/atomic.h>
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/mman.h>
> @@ -21,6 +22,8 @@
>  #include <linux/uaccess.h>
>  #include <linux/module.h>
>
> +extern atomic_t kasan_report_count;
> +
>  /*
>   * Note: test functions are marked noinline so that their names appear in
>   * reports.
> @@ -474,6 +477,9 @@ static noinline void __init use_after_scope_test(void)
>
>  static int __init kmalloc_tests_init(void)
>  {
> +       /* Rise reports limit high enough to see all the following bugs */
> +       atomic_set(&kasan_report_count, 100);
> +
>         kmalloc_oob_right();
>         kmalloc_oob_left();
>         kmalloc_node_oob_right();
> @@ -499,6 +505,9 @@ static int __init kmalloc_tests_init(void)
>         ksize_unpoisons_memory();
>         copy_user_test();
>         use_after_scope_test();
> +
> +       /* kasan is unreliable now, disable reports */
> +       atomic_set(&kasan_report_count, 0);
>         return -EAGAIN;
>  }
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 718a10a..7eab229 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -13,6 +13,7 @@
>   *
>   */
>
> +#include <linux/atomic.h>
>  #include <linux/ftrace.h>
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
> @@ -354,6 +355,9 @@ static void kasan_report_error(struct kasan_access_info *info)
>         kasan_end_report(&flags);
>  }
>
> +atomic_t kasan_report_count = ATOMIC_INIT(1);
> +EXPORT_SYMBOL_GPL(kasan_report_count);
> +
>  void kasan_report(unsigned long addr, size_t size,
>                 bool is_write, unsigned long ip)
>  {
> @@ -362,6 +366,9 @@ void kasan_report(unsigned long addr, size_t size,
>         if (likely(!kasan_report_enabled()))
>                 return;
>
> +       if (atomic_dec_if_positive(&kasan_report_count) < 0)
> +               return;
> +
>         disable_trace_on_warning();
>
>         info.access_addr = (void *)addr;
> --
> 2.10.2
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20170322160647.32032-1-aryabinin%40virtuozzo.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
@ 2017-03-22 16:34   ` Andrey Konovalov
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Konovalov @ 2017-03-22 16:34 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Mark Rutland, Alexander Potapenko, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> Disable kasan after the first report. There are several reasons for this:
>  * Single bug quite often has multiple invalid memory accesses causing
>     storm in the dmesg.
>  * Write OOB access might corrupt metadata so the next report will print
>     bogus alloc/free stacktraces.
>  * Reports after the first easily could be not bugs by itself but just side
>     effects of the first one.
>
> Given that multiple reports only do harm, it makes sense to disable
> kasan after the first one. Except for the tests in lib/test_kasan.c
> as we obviously want to see all reports from test.

Hi Andrey,

Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
default to showing only the first report)?

I sometimes use KASAN to see what bad accesses a particular bug
causes, and seeing all of them (even knowing that they may be
corrupt/induced) helps a lot.

Thanks!

>
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  lib/test_kasan.c  | 9 +++++++++
>  mm/kasan/report.c | 7 +++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 0b1d314..5112663 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -11,6 +11,7 @@
>
>  #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
>
> +#include <linux/atomic.h>
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/mman.h>
> @@ -21,6 +22,8 @@
>  #include <linux/uaccess.h>
>  #include <linux/module.h>
>
> +extern atomic_t kasan_report_count;
> +
>  /*
>   * Note: test functions are marked noinline so that their names appear in
>   * reports.
> @@ -474,6 +477,9 @@ static noinline void __init use_after_scope_test(void)
>
>  static int __init kmalloc_tests_init(void)
>  {
> +       /* Rise reports limit high enough to see all the following bugs */
> +       atomic_set(&kasan_report_count, 100);
> +
>         kmalloc_oob_right();
>         kmalloc_oob_left();
>         kmalloc_node_oob_right();
> @@ -499,6 +505,9 @@ static int __init kmalloc_tests_init(void)
>         ksize_unpoisons_memory();
>         copy_user_test();
>         use_after_scope_test();
> +
> +       /* kasan is unreliable now, disable reports */
> +       atomic_set(&kasan_report_count, 0);
>         return -EAGAIN;
>  }
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 718a10a..7eab229 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -13,6 +13,7 @@
>   *
>   */
>
> +#include <linux/atomic.h>
>  #include <linux/ftrace.h>
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
> @@ -354,6 +355,9 @@ static void kasan_report_error(struct kasan_access_info *info)
>         kasan_end_report(&flags);
>  }
>
> +atomic_t kasan_report_count = ATOMIC_INIT(1);
> +EXPORT_SYMBOL_GPL(kasan_report_count);
> +
>  void kasan_report(unsigned long addr, size_t size,
>                 bool is_write, unsigned long ip)
>  {
> @@ -362,6 +366,9 @@ void kasan_report(unsigned long addr, size_t size,
>         if (likely(!kasan_report_enabled()))
>                 return;
>
> +       if (atomic_dec_if_positive(&kasan_report_count) < 0)
> +               return;
> +
>         disable_trace_on_warning();
>
>         info.access_addr = (void *)addr;
> --
> 2.10.2
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20170322160647.32032-1-aryabinin%40virtuozzo.com.
> For more options, visit https://groups.google.com/d/optout.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
  2017-03-22 16:34   ` Andrey Konovalov
@ 2017-03-22 16:54     ` Andrey Ryabinin
  -1 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-22 16:54 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Andrew Morton, Mark Rutland, Alexander Potapenko, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
>> Disable kasan after the first report. There are several reasons for this:
>>  * Single bug quite often has multiple invalid memory accesses causing
>>     storm in the dmesg.
>>  * Write OOB access might corrupt metadata so the next report will print
>>     bogus alloc/free stacktraces.
>>  * Reports after the first easily could be not bugs by itself but just side
>>     effects of the first one.
>>
>> Given that multiple reports only do harm, it makes sense to disable
>> kasan after the first one. Except for the tests in lib/test_kasan.c
>> as we obviously want to see all reports from test.
> 
> Hi Andrey,
> 
> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
> default to showing only the first report)?

I'd rather make this boot time configurable, but wouldn't want to without
a good reason.


> I sometimes use KASAN to see what bad accesses a particular bug
> causes, and seeing all of them (even knowing that they may be
> corrupt/induced) helps a lot.

I'm wondering why you need to see all reports?

> 
> Thanks!
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
@ 2017-03-22 16:54     ` Andrey Ryabinin
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-22 16:54 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Andrew Morton, Mark Rutland, Alexander Potapenko, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
>> Disable kasan after the first report. There are several reasons for this:
>>  * Single bug quite often has multiple invalid memory accesses causing
>>     storm in the dmesg.
>>  * Write OOB access might corrupt metadata so the next report will print
>>     bogus alloc/free stacktraces.
>>  * Reports after the first easily could be not bugs by itself but just side
>>     effects of the first one.
>>
>> Given that multiple reports only do harm, it makes sense to disable
>> kasan after the first one. Except for the tests in lib/test_kasan.c
>> as we obviously want to see all reports from test.
> 
> Hi Andrey,
> 
> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
> default to showing only the first report)?

I'd rather make this boot time configurable, but wouldn't want to without
a good reason.


> I sometimes use KASAN to see what bad accesses a particular bug
> causes, and seeing all of them (even knowing that they may be
> corrupt/induced) helps a lot.

I'm wondering why you need to see all reports?

> 
> Thanks!
> 

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
  2017-03-22 16:54     ` Andrey Ryabinin
@ 2017-03-22 17:07       ` Andrey Konovalov
  -1 siblings, 0 replies; 22+ messages in thread
From: Andrey Konovalov @ 2017-03-22 17:07 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Mark Rutland, Alexander Potapenko, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 5:54 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
>> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
>> <aryabinin@virtuozzo.com> wrote:
>>> Disable kasan after the first report. There are several reasons for this:
>>>  * Single bug quite often has multiple invalid memory accesses causing
>>>     storm in the dmesg.
>>>  * Write OOB access might corrupt metadata so the next report will print
>>>     bogus alloc/free stacktraces.
>>>  * Reports after the first easily could be not bugs by itself but just side
>>>     effects of the first one.
>>>
>>> Given that multiple reports only do harm, it makes sense to disable
>>> kasan after the first one. Except for the tests in lib/test_kasan.c
>>> as we obviously want to see all reports from test.
>>
>> Hi Andrey,
>>
>> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
>> default to showing only the first report)?
>
> I'd rather make this boot time configurable, but wouldn't want to without
> a good reason.

That would work for me.

>
>
>> I sometimes use KASAN to see what bad accesses a particular bug
>> causes, and seeing all of them (even knowing that they may be
>> corrupt/induced) helps a lot.
>
> I'm wondering why you need to see all reports?

To get a better picture of what are the consequences of a bug. For
example whether it leads to some bad or controllable memory
corruption. Sometimes it's easier to let KASAN track the memory
accesses then do that manually.

>
>>
>> Thanks!
>>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
@ 2017-03-22 17:07       ` Andrey Konovalov
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Konovalov @ 2017-03-22 17:07 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Mark Rutland, Alexander Potapenko, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 5:54 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
>> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
>> <aryabinin@virtuozzo.com> wrote:
>>> Disable kasan after the first report. There are several reasons for this:
>>>  * Single bug quite often has multiple invalid memory accesses causing
>>>     storm in the dmesg.
>>>  * Write OOB access might corrupt metadata so the next report will print
>>>     bogus alloc/free stacktraces.
>>>  * Reports after the first easily could be not bugs by itself but just side
>>>     effects of the first one.
>>>
>>> Given that multiple reports only do harm, it makes sense to disable
>>> kasan after the first one. Except for the tests in lib/test_kasan.c
>>> as we obviously want to see all reports from test.
>>
>> Hi Andrey,
>>
>> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
>> default to showing only the first report)?
>
> I'd rather make this boot time configurable, but wouldn't want to without
> a good reason.

That would work for me.

>
>
>> I sometimes use KASAN to see what bad accesses a particular bug
>> causes, and seeing all of them (even knowing that they may be
>> corrupt/induced) helps a lot.
>
> I'm wondering why you need to see all reports?

To get a better picture of what are the consequences of a bug. For
example whether it leads to some bad or controllable memory
corruption. Sometimes it's easier to let KASAN track the memory
accesses then do that manually.

>
>>
>> Thanks!
>>

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
  2017-03-22 17:07       ` Andrey Konovalov
@ 2017-03-22 17:33         ` Alexander Potapenko
  -1 siblings, 0 replies; 22+ messages in thread
From: Alexander Potapenko @ 2017-03-22 17:33 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Andrey Ryabinin, Andrew Morton, Mark Rutland, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 6:07 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Wed, Mar 22, 2017 at 5:54 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
>> On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
>>> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
>>> <aryabinin@virtuozzo.com> wrote:
>>>> Disable kasan after the first report. There are several reasons for this:
>>>>  * Single bug quite often has multiple invalid memory accesses causing
>>>>     storm in the dmesg.
>>>>  * Write OOB access might corrupt metadata so the next report will print
>>>>     bogus alloc/free stacktraces.
>>>>  * Reports after the first easily could be not bugs by itself but just side
>>>>     effects of the first one.
>>>>
>>>> Given that multiple reports only do harm, it makes sense to disable
>>>> kasan after the first one. Except for the tests in lib/test_kasan.c
>>>> as we obviously want to see all reports from test.
>>>
>>> Hi Andrey,
>>>
>>> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
>>> default to showing only the first report)?
>>
>> I'd rather make this boot time configurable, but wouldn't want to without
>> a good reason.
>
> That would work for me.
>
>>
>>
>>> I sometimes use KASAN to see what bad accesses a particular bug
>>> causes, and seeing all of them (even knowing that they may be
>>> corrupt/induced) helps a lot.
>>
>> I'm wondering why you need to see all reports?
>
> To get a better picture of what are the consequences of a bug. For
> example whether it leads to some bad or controllable memory
> corruption. Sometimes it's easier to let KASAN track the memory
> accesses then do that manually.
Another case is when you're seeing an OOB read at boot time, which has
limited impact, and you don't want to wait for the code owner to fix
it to move forward.
>>
>>>
>>> Thanks!
>>>



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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
@ 2017-03-22 17:33         ` Alexander Potapenko
  0 siblings, 0 replies; 22+ messages in thread
From: Alexander Potapenko @ 2017-03-22 17:33 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Andrey Ryabinin, Andrew Morton, Mark Rutland, Dmitry Vyukov,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 6:07 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Wed, Mar 22, 2017 at 5:54 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
>> On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
>>> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
>>> <aryabinin@virtuozzo.com> wrote:
>>>> Disable kasan after the first report. There are several reasons for this:
>>>>  * Single bug quite often has multiple invalid memory accesses causing
>>>>     storm in the dmesg.
>>>>  * Write OOB access might corrupt metadata so the next report will print
>>>>     bogus alloc/free stacktraces.
>>>>  * Reports after the first easily could be not bugs by itself but just side
>>>>     effects of the first one.
>>>>
>>>> Given that multiple reports only do harm, it makes sense to disable
>>>> kasan after the first one. Except for the tests in lib/test_kasan.c
>>>> as we obviously want to see all reports from test.
>>>
>>> Hi Andrey,
>>>
>>> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
>>> default to showing only the first report)?
>>
>> I'd rather make this boot time configurable, but wouldn't want to without
>> a good reason.
>
> That would work for me.
>
>>
>>
>>> I sometimes use KASAN to see what bad accesses a particular bug
>>> causes, and seeing all of them (even knowing that they may be
>>> corrupt/induced) helps a lot.
>>
>> I'm wondering why you need to see all reports?
>
> To get a better picture of what are the consequences of a bug. For
> example whether it leads to some bad or controllable memory
> corruption. Sometimes it's easier to let KASAN track the memory
> accesses then do that manually.
Another case is when you're seeing an OOB read at boot time, which has
limited impact, and you don't want to wait for the code owner to fix
it to move forward.
>>
>>>
>>> Thanks!
>>>



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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
  2017-03-22 17:33         ` Alexander Potapenko
@ 2017-03-22 17:42           ` Dmitry Vyukov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Vyukov @ 2017-03-22 17:42 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Andrey Konovalov, Andrey Ryabinin, Andrew Morton, Mark Rutland,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 6:33 PM, Alexander Potapenko <glider@google.com> wrote:
> On Wed, Mar 22, 2017 at 6:07 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
>> On Wed, Mar 22, 2017 at 5:54 PM, Andrey Ryabinin
>> <aryabinin@virtuozzo.com> wrote:
>>> On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
>>>> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
>>>> <aryabinin@virtuozzo.com> wrote:
>>>>> Disable kasan after the first report. There are several reasons for this:
>>>>>  * Single bug quite often has multiple invalid memory accesses causing
>>>>>     storm in the dmesg.
>>>>>  * Write OOB access might corrupt metadata so the next report will print
>>>>>     bogus alloc/free stacktraces.
>>>>>  * Reports after the first easily could be not bugs by itself but just side
>>>>>     effects of the first one.
>>>>>
>>>>> Given that multiple reports only do harm, it makes sense to disable
>>>>> kasan after the first one. Except for the tests in lib/test_kasan.c
>>>>> as we obviously want to see all reports from test.
>>>>
>>>> Hi Andrey,
>>>>
>>>> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
>>>> default to showing only the first report)?
>>>
>>> I'd rather make this boot time configurable, but wouldn't want to without
>>> a good reason.
>>
>> That would work for me.

Also note that KASAN now supports panic_on_warn=1, which achieves more
or less the same. Of course, WARNINGs may be not that bad, but KASAN
reports may be not tool bad as well (e.g. off-by-one reads).


>>>> I sometimes use KASAN to see what bad accesses a particular bug
>>>> causes, and seeing all of them (even knowing that they may be
>>>> corrupt/induced) helps a lot.
>>>
>>> I'm wondering why you need to see all reports?
>>
>> To get a better picture of what are the consequences of a bug. For
>> example whether it leads to some bad or controllable memory
>> corruption. Sometimes it's easier to let KASAN track the memory
>> accesses then do that manually.
> Another case is when you're seeing an OOB read at boot time, which has
> limited impact, and you don't want to wait for the code owner to fix
> it to move forward.
>>>
>>>>
>>>> Thanks!
>>>>
>
>
>
> --
> 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] kasan: report only the first error
@ 2017-03-22 17:42           ` Dmitry Vyukov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Vyukov @ 2017-03-22 17:42 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Andrey Konovalov, Andrey Ryabinin, Andrew Morton, Mark Rutland,
	kasan-dev, Linux Memory Management List, LKML

On Wed, Mar 22, 2017 at 6:33 PM, Alexander Potapenko <glider@google.com> wrote:
> On Wed, Mar 22, 2017 at 6:07 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
>> On Wed, Mar 22, 2017 at 5:54 PM, Andrey Ryabinin
>> <aryabinin@virtuozzo.com> wrote:
>>> On 03/22/2017 07:34 PM, Andrey Konovalov wrote:
>>>> On Wed, Mar 22, 2017 at 5:06 PM, Andrey Ryabinin
>>>> <aryabinin@virtuozzo.com> wrote:
>>>>> Disable kasan after the first report. There are several reasons for this:
>>>>>  * Single bug quite often has multiple invalid memory accesses causing
>>>>>     storm in the dmesg.
>>>>>  * Write OOB access might corrupt metadata so the next report will print
>>>>>     bogus alloc/free stacktraces.
>>>>>  * Reports after the first easily could be not bugs by itself but just side
>>>>>     effects of the first one.
>>>>>
>>>>> Given that multiple reports only do harm, it makes sense to disable
>>>>> kasan after the first one. Except for the tests in lib/test_kasan.c
>>>>> as we obviously want to see all reports from test.
>>>>
>>>> Hi Andrey,
>>>>
>>>> Could you make it configurable via CONFIG_KASAN_SOMETHING (which can
>>>> default to showing only the first report)?
>>>
>>> I'd rather make this boot time configurable, but wouldn't want to without
>>> a good reason.
>>
>> That would work for me.

Also note that KASAN now supports panic_on_warn=1, which achieves more
or less the same. Of course, WARNINGs may be not that bad, but KASAN
reports may be not tool bad as well (e.g. off-by-one reads).


>>>> I sometimes use KASAN to see what bad accesses a particular bug
>>>> causes, and seeing all of them (even knowing that they may be
>>>> corrupt/induced) helps a lot.
>>>
>>> I'm wondering why you need to see all reports?
>>
>> To get a better picture of what are the consequences of a bug. For
>> example whether it leads to some bad or controllable memory
>> corruption. Sometimes it's easier to let KASAN track the memory
>> accesses then do that manually.
> Another case is when you're seeing an OOB read at boot time, which has
> limited impact, and you don't want to wait for the code owner to fix
> it to move forward.
>>>
>>>>
>>>> Thanks!
>>>>
>
>
>
> --
> 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>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] kasan: report only the first error by default
  2017-03-22 16:06 ` Andrey Ryabinin
@ 2017-03-23 11:49   ` Andrey Ryabinin
  -1 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-23 11:49 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Mark Rutland, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel,
	Andrey Ryabinin

Disable kasan after the first report. There are several reasons for this:
 * Single bug quite often has multiple invalid memory accesses causing
    storm in the dmesg.
 * Write OOB access might corrupt metadata so the next report will print
    bogus alloc/free stacktraces.
 * Reports after the first easily could be not bugs by itself but just side
    effects of the first one.

Given that multiple reports usually only do harm, it makes sense to disable
kasan after the first one. If user wants to see all the reports, the
boot-time parameter kasan_multi_shot must be used.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
Changes since v1:
        - provide kasan_multi_shot boot parameter.

 Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
 lib/test_kasan.c                                | 12 ++++++++++++
 mm/kasan/kasan.h                                |  5 -----
 mm/kasan/report.c                               | 18 ++++++++++++++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2906987..f88d60e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1726,6 +1726,12 @@
 			kernel and module base offset ASLR (Address Space
 			Layout Randomization).
 
+	kasan_multi_shot
+			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
+			report on every invalid memory access. Without this
+			parameter KASAN will print report only for the first
+			invalid access.
+
 	keepinitrd	[HW,ARM]
 
 	kernelcore=	[KNL,X86,IA-64,PPC]
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..f3acece 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -11,6 +11,7 @@
 
 #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
 
+#include <linux/atomic.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
@@ -21,6 +22,8 @@
 #include <linux/uaccess.h>
 #include <linux/module.h>
 
+extern atomic_t kasan_report_count;
+
 /*
  * Note: test functions are marked noinline so that their names appear in
  * reports.
@@ -474,6 +477,9 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/* Rise reports limit high enough to see all the following bugs */
+	atomic_add(100, &kasan_report_count);
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +505,12 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	/*
+	 * kasan is unreliable now, disable reports if
+	 * we are in single shot mode
+	 */
+	atomic_sub(100, &kasan_report_count);
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 7572917..1229298 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,11 +96,6 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 		<< KASAN_SHADOW_SCALE_SHIFT);
 }
 
-static inline bool kasan_report_enabled(void)
-{
-	return !current->kasan_depth;
-}
-
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 void kasan_report_double_free(struct kmem_cache *cache, void *object,
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 718a10a..5650534 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,7 +13,9 @@
  *
  */
 
+#include <linux/atomic.h>
 #include <linux/ftrace.h>
+#include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/printk.h>
@@ -354,6 +356,22 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+atomic_t kasan_report_count = ATOMIC_INIT(1);
+EXPORT_SYMBOL_GPL(kasan_report_count);
+
+static int __init kasan_set_multi_shot(char *str)
+{
+	atomic_set(&kasan_report_count, 1000000000);
+	return 1;
+}
+__setup("kasan_multi_shot", kasan_set_multi_shot);
+
+static inline bool kasan_report_enabled(void)
+{
+	return !current->kasan_depth &&
+		(atomic_dec_if_positive(&kasan_report_count) >= 0);
+}
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2] kasan: report only the first error by default
@ 2017-03-23 11:49   ` Andrey Ryabinin
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-23 11:49 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Mark Rutland, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel,
	Andrey Ryabinin

Disable kasan after the first report. There are several reasons for this:
 * Single bug quite often has multiple invalid memory accesses causing
    storm in the dmesg.
 * Write OOB access might corrupt metadata so the next report will print
    bogus alloc/free stacktraces.
 * Reports after the first easily could be not bugs by itself but just side
    effects of the first one.

Given that multiple reports usually only do harm, it makes sense to disable
kasan after the first one. If user wants to see all the reports, the
boot-time parameter kasan_multi_shot must be used.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
Changes since v1:
        - provide kasan_multi_shot boot parameter.

 Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
 lib/test_kasan.c                                | 12 ++++++++++++
 mm/kasan/kasan.h                                |  5 -----
 mm/kasan/report.c                               | 18 ++++++++++++++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2906987..f88d60e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1726,6 +1726,12 @@
 			kernel and module base offset ASLR (Address Space
 			Layout Randomization).
 
+	kasan_multi_shot
+			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
+			report on every invalid memory access. Without this
+			parameter KASAN will print report only for the first
+			invalid access.
+
 	keepinitrd	[HW,ARM]
 
 	kernelcore=	[KNL,X86,IA-64,PPC]
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..f3acece 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -11,6 +11,7 @@
 
 #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
 
+#include <linux/atomic.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
@@ -21,6 +22,8 @@
 #include <linux/uaccess.h>
 #include <linux/module.h>
 
+extern atomic_t kasan_report_count;
+
 /*
  * Note: test functions are marked noinline so that their names appear in
  * reports.
@@ -474,6 +477,9 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/* Rise reports limit high enough to see all the following bugs */
+	atomic_add(100, &kasan_report_count);
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +505,12 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	/*
+	 * kasan is unreliable now, disable reports if
+	 * we are in single shot mode
+	 */
+	atomic_sub(100, &kasan_report_count);
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 7572917..1229298 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,11 +96,6 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 		<< KASAN_SHADOW_SCALE_SHIFT);
 }
 
-static inline bool kasan_report_enabled(void)
-{
-	return !current->kasan_depth;
-}
-
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 void kasan_report_double_free(struct kmem_cache *cache, void *object,
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 718a10a..5650534 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,7 +13,9 @@
  *
  */
 
+#include <linux/atomic.h>
 #include <linux/ftrace.h>
+#include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/printk.h>
@@ -354,6 +356,22 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+atomic_t kasan_report_count = ATOMIC_INIT(1);
+EXPORT_SYMBOL_GPL(kasan_report_count);
+
+static int __init kasan_set_multi_shot(char *str)
+{
+	atomic_set(&kasan_report_count, 1000000000);
+	return 1;
+}
+__setup("kasan_multi_shot", kasan_set_multi_shot);
+
+static inline bool kasan_report_enabled(void)
+{
+	return !current->kasan_depth &&
+		(atomic_dec_if_positive(&kasan_report_count) >= 0);
+}
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] kasan: report only the first error by default
  2017-03-23 11:49   ` Andrey Ryabinin
@ 2017-03-23 12:41     ` Mark Rutland
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2017-03-23 12:41 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Andrey Konovalov, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel

On Thu, Mar 23, 2017 at 02:49:16PM +0300, Andrey Ryabinin wrote:
> +	kasan_multi_shot
> +			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
> +			report on every invalid memory access. Without this
> +			parameter KASAN will print report only for the first
> +			invalid access.
> +

The option looks fine to me.

>  static int __init kmalloc_tests_init(void)
>  {
> +	/* Rise reports limit high enough to see all the following bugs */
> +	atomic_add(100, &kasan_report_count);

> +
> +	/*
> +	 * kasan is unreliable now, disable reports if
> +	 * we are in single shot mode
> +	 */
> +	atomic_sub(100, &kasan_report_count);
>  	return -EAGAIN;
>  }

... but these magic numbers look rather messy.

[...]

> +atomic_t kasan_report_count = ATOMIC_INIT(1);
> +EXPORT_SYMBOL_GPL(kasan_report_count);
> +
> +static int __init kasan_set_multi_shot(char *str)
> +{
> +	atomic_set(&kasan_report_count, 1000000000);
> +	return 1;
> +}
> +__setup("kasan_multi_shot", kasan_set_multi_shot);

... likewise.

Rather than trying to pick an arbitrarily large number, how about we use
separate flags to determine whether we're in multi-shot mode, and
whether a (oneshot) report has been made.

How about the below?

Thanks,
Mark.

---->8----
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index ceb3fe7..291d7b3 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -74,6 +74,9 @@ struct kasan_cache {
 static inline void kasan_unpoison_slab(const void *ptr) { ksize(ptr); }
 size_t kasan_metadata_size(struct kmem_cache *cache);
 
+bool kasan_save_enable_multi_shot(void);
+void kasan_restore_multi_shot(bool enabled);
+
 #else /* CONFIG_KASAN */
 
 static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..ae59dc2 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -12,6 +12,7 @@
 #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
 
 #include <linux/delay.h>
+#include <linux/kasan.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
 #include <linux/mm.h>
@@ -474,6 +475,12 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/*
+	 * Temporarily enable multi-shot mode. Otherwise, we'd only get a
+	 * report for the first case.
+	 */
+	bool multishot = kasan_save_enable_multi_shot();
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +506,9 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	kasan_restore_multi_shot(multishot);
+
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 1c260e6..dd2dea8 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,11 +96,6 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 		<< KASAN_SHADOW_SCALE_SHIFT);
 }
 
-static inline bool kasan_report_enabled(void)
-{
-	return !current->kasan_depth;
-}
-
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 void kasan_report_double_free(struct kmem_cache *cache, void *object,
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index f479365..f1c5892 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,6 +13,7 @@
  *
  */
 
+#include <linux/bitops.h>
 #include <linux/ftrace.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -293,6 +294,40 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+static unsigned long kasan_flags;
+
+#define KASAN_BIT_REPORTED	0
+#define KASAN_BIT_MULTI_SHOT	1
+
+bool kasan_save_enable_multi_shot(void)
+{
+	return test_and_set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_save_enable_multi_shot);
+
+void kasan_restore_multi_shot(bool enabled)
+{
+	if (!enabled)
+		clear_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);
+
+static int __init kasan_set_multi_shot(char *str)
+{
+	set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+	return 1;
+}
+__setup("kasan_multi_shot", kasan_set_multi_shot);
+
+static inline bool kasan_report_enabled(void)
+{
+	if (current->kasan_depth)
+		return false;
+	if (test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
+		return true;
+	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
+}
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] kasan: report only the first error by default
@ 2017-03-23 12:41     ` Mark Rutland
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2017-03-23 12:41 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Andrey Konovalov, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel

On Thu, Mar 23, 2017 at 02:49:16PM +0300, Andrey Ryabinin wrote:
> +	kasan_multi_shot
> +			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
> +			report on every invalid memory access. Without this
> +			parameter KASAN will print report only for the first
> +			invalid access.
> +

The option looks fine to me.

>  static int __init kmalloc_tests_init(void)
>  {
> +	/* Rise reports limit high enough to see all the following bugs */
> +	atomic_add(100, &kasan_report_count);

> +
> +	/*
> +	 * kasan is unreliable now, disable reports if
> +	 * we are in single shot mode
> +	 */
> +	atomic_sub(100, &kasan_report_count);
>  	return -EAGAIN;
>  }

... but these magic numbers look rather messy.

[...]

> +atomic_t kasan_report_count = ATOMIC_INIT(1);
> +EXPORT_SYMBOL_GPL(kasan_report_count);
> +
> +static int __init kasan_set_multi_shot(char *str)
> +{
> +	atomic_set(&kasan_report_count, 1000000000);
> +	return 1;
> +}
> +__setup("kasan_multi_shot", kasan_set_multi_shot);

... likewise.

Rather than trying to pick an arbitrarily large number, how about we use
separate flags to determine whether we're in multi-shot mode, and
whether a (oneshot) report has been made.

How about the below?

Thanks,
Mark.

---->8----
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index ceb3fe7..291d7b3 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -74,6 +74,9 @@ struct kasan_cache {
 static inline void kasan_unpoison_slab(const void *ptr) { ksize(ptr); }
 size_t kasan_metadata_size(struct kmem_cache *cache);
 
+bool kasan_save_enable_multi_shot(void);
+void kasan_restore_multi_shot(bool enabled);
+
 #else /* CONFIG_KASAN */
 
 static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..ae59dc2 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -12,6 +12,7 @@
 #define pr_fmt(fmt) "kasan test: %s " fmt, __func__
 
 #include <linux/delay.h>
+#include <linux/kasan.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
 #include <linux/mm.h>
@@ -474,6 +475,12 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/*
+	 * Temporarily enable multi-shot mode. Otherwise, we'd only get a
+	 * report for the first case.
+	 */
+	bool multishot = kasan_save_enable_multi_shot();
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +506,9 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	kasan_restore_multi_shot(multishot);
+
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 1c260e6..dd2dea8 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,11 +96,6 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 		<< KASAN_SHADOW_SCALE_SHIFT);
 }
 
-static inline bool kasan_report_enabled(void)
-{
-	return !current->kasan_depth;
-}
-
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 void kasan_report_double_free(struct kmem_cache *cache, void *object,
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index f479365..f1c5892 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,6 +13,7 @@
  *
  */
 
+#include <linux/bitops.h>
 #include <linux/ftrace.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -293,6 +294,40 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+static unsigned long kasan_flags;
+
+#define KASAN_BIT_REPORTED	0
+#define KASAN_BIT_MULTI_SHOT	1
+
+bool kasan_save_enable_multi_shot(void)
+{
+	return test_and_set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_save_enable_multi_shot);
+
+void kasan_restore_multi_shot(bool enabled)
+{
+	if (!enabled)
+		clear_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);
+
+static int __init kasan_set_multi_shot(char *str)
+{
+	set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+	return 1;
+}
+__setup("kasan_multi_shot", kasan_set_multi_shot);
+
+static inline bool kasan_report_enabled(void)
+{
+	if (current->kasan_depth)
+		return false;
+	if (test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
+		return true;
+	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
+}
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] kasan: report only the first error by default
  2017-03-23 12:41     ` Mark Rutland
@ 2017-03-23 13:06       ` Andrey Ryabinin
  -1 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-23 13:06 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Andrew Morton, Andrey Konovalov, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel



On 03/23/2017 03:41 PM, Mark Rutland wrote:
> On Thu, Mar 23, 2017 at 02:49:16PM +0300, Andrey Ryabinin wrote:
>> +	kasan_multi_shot
>> +			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
>> +			report on every invalid memory access. Without this
>> +			parameter KASAN will print report only for the first
>> +			invalid access.
>> +
> 
> The option looks fine to me.
> 
>>  static int __init kmalloc_tests_init(void)
>>  {
>> +	/* Rise reports limit high enough to see all the following bugs */
>> +	atomic_add(100, &kasan_report_count);
> 
>> +
>> +	/*
>> +	 * kasan is unreliable now, disable reports if
>> +	 * we are in single shot mode
>> +	 */
>> +	atomic_sub(100, &kasan_report_count);
>>  	return -EAGAIN;
>>  }
> 
> ... but these magic numbers look rather messy.
> 
> [...]
> 
>> +atomic_t kasan_report_count = ATOMIC_INIT(1);
>> +EXPORT_SYMBOL_GPL(kasan_report_count);
>> +
>> +static int __init kasan_set_multi_shot(char *str)
>> +{
>> +	atomic_set(&kasan_report_count, 1000000000);
>> +	return 1;
>> +}
>> +__setup("kasan_multi_shot", kasan_set_multi_shot);
> 
> ... likewise.
> 
> Rather than trying to pick an arbitrarily large number, how about we use
> separate flags to determine whether we're in multi-shot mode, and
> whether a (oneshot) report has been made.
> 
> How about the below?
 
Yes, it deferentially looks better.
Can you send a patch with a changelog, or do you want me to care of it?

> Thanks,
> Mark.
> 

> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index f479365..f1c5892 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -13,6 +13,7 @@
>   *
>   */
>  
> +#include <linux/bitops.h>
>  #include <linux/ftrace.h>

We also need <linux/init.h> for __setup().

>  #include <linux/kernel.h>
>  #include <linux/mm.h>
> @@ -293,6 +294,40 @@ static void kasan_report_error(struct kasan_access_info *info)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] kasan: report only the first error by default
@ 2017-03-23 13:06       ` Andrey Ryabinin
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-23 13:06 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Andrew Morton, Andrey Konovalov, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel



On 03/23/2017 03:41 PM, Mark Rutland wrote:
> On Thu, Mar 23, 2017 at 02:49:16PM +0300, Andrey Ryabinin wrote:
>> +	kasan_multi_shot
>> +			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
>> +			report on every invalid memory access. Without this
>> +			parameter KASAN will print report only for the first
>> +			invalid access.
>> +
> 
> The option looks fine to me.
> 
>>  static int __init kmalloc_tests_init(void)
>>  {
>> +	/* Rise reports limit high enough to see all the following bugs */
>> +	atomic_add(100, &kasan_report_count);
> 
>> +
>> +	/*
>> +	 * kasan is unreliable now, disable reports if
>> +	 * we are in single shot mode
>> +	 */
>> +	atomic_sub(100, &kasan_report_count);
>>  	return -EAGAIN;
>>  }
> 
> ... but these magic numbers look rather messy.
> 
> [...]
> 
>> +atomic_t kasan_report_count = ATOMIC_INIT(1);
>> +EXPORT_SYMBOL_GPL(kasan_report_count);
>> +
>> +static int __init kasan_set_multi_shot(char *str)
>> +{
>> +	atomic_set(&kasan_report_count, 1000000000);
>> +	return 1;
>> +}
>> +__setup("kasan_multi_shot", kasan_set_multi_shot);
> 
> ... likewise.
> 
> Rather than trying to pick an arbitrarily large number, how about we use
> separate flags to determine whether we're in multi-shot mode, and
> whether a (oneshot) report has been made.
> 
> How about the below?
 
Yes, it deferentially looks better.
Can you send a patch with a changelog, or do you want me to care of it?

> Thanks,
> Mark.
> 

> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index f479365..f1c5892 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -13,6 +13,7 @@
>   *
>   */
>  
> +#include <linux/bitops.h>
>  #include <linux/ftrace.h>

We also need <linux/init.h> for __setup().

>  #include <linux/kernel.h>
>  #include <linux/mm.h>
> @@ -293,6 +294,40 @@ static void kasan_report_error(struct kasan_access_info *info)

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] kasan: report only the first error by default
  2017-03-23 13:06       ` Andrey Ryabinin
@ 2017-03-23 13:29         ` Mark Rutland
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2017-03-23 13:29 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Andrey Konovalov, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel

On Thu, Mar 23, 2017 at 04:06:59PM +0300, Andrey Ryabinin wrote:
> On 03/23/2017 03:41 PM, Mark Rutland wrote:

> > Rather than trying to pick an arbitrarily large number, how about we use
> > separate flags to determine whether we're in multi-shot mode, and
> > whether a (oneshot) report has been made.
> > 
> > How about the below?
>  
> Yes, it deferentially looks better.
> Can you send a patch with a changelog, or do you want me to care of it?

Would you be happy to take care of it, along with the fixup you
suggested below, as v3?

You can add my:

Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.


> >  
> > +#include <linux/bitops.h>
> >  #include <linux/ftrace.h>
> 
> We also need <linux/init.h> for __setup().
> 
> >  #include <linux/kernel.h>
> >  #include <linux/mm.h>
> > @@ -293,6 +294,40 @@ static void kasan_report_error(struct kasan_access_info *info)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] kasan: report only the first error by default
@ 2017-03-23 13:29         ` Mark Rutland
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2017-03-23 13:29 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Andrew Morton, Andrey Konovalov, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel

On Thu, Mar 23, 2017 at 04:06:59PM +0300, Andrey Ryabinin wrote:
> On 03/23/2017 03:41 PM, Mark Rutland wrote:

> > Rather than trying to pick an arbitrarily large number, how about we use
> > separate flags to determine whether we're in multi-shot mode, and
> > whether a (oneshot) report has been made.
> > 
> > How about the below?
>  
> Yes, it deferentially looks better.
> Can you send a patch with a changelog, or do you want me to care of it?

Would you be happy to take care of it, along with the fixup you
suggested below, as v3?

You can add my:

Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.


> >  
> > +#include <linux/bitops.h>
> >  #include <linux/ftrace.h>
> 
> We also need <linux/init.h> for __setup().
> 
> >  #include <linux/kernel.h>
> >  #include <linux/mm.h>
> > @@ -293,6 +294,40 @@ static void kasan_report_error(struct kasan_access_info *info)

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3] kasan: report only the first error by default
  2017-03-22 16:06 ` Andrey Ryabinin
@ 2017-03-23 15:44   ` Andrey Ryabinin
  -1 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-23 15:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Mark Rutland, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel,
	Andrey Ryabinin

From: Mark Rutland <mark.rutland@arm.com>

Disable kasan after the first report. There are several reasons for this:
 * Single bug quite often has multiple invalid memory accesses causing
    storm in the dmesg.
 * Write OOB access might corrupt metadata so the next report will print
    bogus alloc/free stacktraces.
 * Reports after the first easily could be not bugs by itself but just side
    effects of the first one.

Given that multiple reports usually only do harm, it makes sense to disable
kasan after the first one. If user wants to see all the reports, the
boot-time parameter kasan_multi_shot must be used.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
[ aryabinin: wrote changelog and doc, added missing include ]
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
Changes since v2:
        - Instead of using atomic report counter, use separate flags to determine
           whether we're in multi-shot mode, and whether a (oneshot) report
           has been made.
Changes since v1:
        - provide kasan_multi_shot boot parameter.

 Documentation/admin-guide/kernel-parameters.txt |  6 +++++
 include/linux/kasan.h                           |  3 +++
 lib/test_kasan.c                                | 10 +++++++
 mm/kasan/kasan.h                                |  5 ----
 mm/kasan/report.c                               | 36 +++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2906987..f88d60e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1726,6 +1726,12 @@
 			kernel and module base offset ASLR (Address Space
 			Layout Randomization).
 
+	kasan_multi_shot
+			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
+			report on every invalid memory access. Without this
+			parameter KASAN will print report only for the first
+			invalid access.
+
 	keepinitrd	[HW,ARM]
 
 	kernelcore=	[KNL,X86,IA-64,PPC]
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 5734480c9..a5c7046 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -76,6 +76,9 @@ size_t ksize(const void *);
 static inline void kasan_unpoison_slab(const void *ptr) { ksize(ptr); }
 size_t kasan_metadata_size(struct kmem_cache *cache);
 
+bool kasan_save_enable_multi_shot(void);
+void kasan_restore_multi_shot(bool enabled);
+
 #else /* CONFIG_KASAN */
 
 static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..a25c976 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -20,6 +20,7 @@
 #include <linux/string.h>
 #include <linux/uaccess.h>
 #include <linux/module.h>
+#include <linux/kasan.h>
 
 /*
  * Note: test functions are marked noinline so that their names appear in
@@ -474,6 +475,12 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/*
+	 * Temporarily enable multi-shot mode. Otherwise, we'd only get a
+	 * report for the first case.
+	 */
+	bool multishot = kasan_save_enable_multi_shot();
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +506,9 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	kasan_restore_multi_shot(multishot);
+
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 7572917..1229298 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,11 +96,6 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 		<< KASAN_SHADOW_SCALE_SHIFT);
 }
 
-static inline bool kasan_report_enabled(void)
-{
-	return !current->kasan_depth;
-}
-
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 void kasan_report_double_free(struct kmem_cache *cache, void *object,
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 718a10a..beee0e9 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,7 +13,9 @@
  *
  */
 
+#include <linux/bitops.h>
 #include <linux/ftrace.h>
+#include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/printk.h>
@@ -354,6 +356,40 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+static unsigned long kasan_flags;
+
+#define KASAN_BIT_REPORTED	0
+#define KASAN_BIT_MULTI_SHOT	1
+
+bool kasan_save_enable_multi_shot(void)
+{
+	return test_and_set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_save_enable_multi_shot);
+
+void kasan_restore_multi_shot(bool enabled)
+{
+	if (!enabled)
+		clear_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);
+
+static int __init kasan_set_multi_shot(char *str)
+{
+	set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+	return 1;
+}
+__setup("kasan_multi_shot", kasan_set_multi_shot);
+
+static inline bool kasan_report_enabled(void)
+{
+	if (current->kasan_depth)
+		return false;
+	if (test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
+		return true;
+	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
+}
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3] kasan: report only the first error by default
@ 2017-03-23 15:44   ` Andrey Ryabinin
  0 siblings, 0 replies; 22+ messages in thread
From: Andrey Ryabinin @ 2017-03-23 15:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Mark Rutland, Alexander Potapenko,
	Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel,
	Andrey Ryabinin

From: Mark Rutland <mark.rutland@arm.com>

Disable kasan after the first report. There are several reasons for this:
 * Single bug quite often has multiple invalid memory accesses causing
    storm in the dmesg.
 * Write OOB access might corrupt metadata so the next report will print
    bogus alloc/free stacktraces.
 * Reports after the first easily could be not bugs by itself but just side
    effects of the first one.

Given that multiple reports usually only do harm, it makes sense to disable
kasan after the first one. If user wants to see all the reports, the
boot-time parameter kasan_multi_shot must be used.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
[ aryabinin: wrote changelog and doc, added missing include ]
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
Changes since v2:
        - Instead of using atomic report counter, use separate flags to determine
           whether we're in multi-shot mode, and whether a (oneshot) report
           has been made.
Changes since v1:
        - provide kasan_multi_shot boot parameter.

 Documentation/admin-guide/kernel-parameters.txt |  6 +++++
 include/linux/kasan.h                           |  3 +++
 lib/test_kasan.c                                | 10 +++++++
 mm/kasan/kasan.h                                |  5 ----
 mm/kasan/report.c                               | 36 +++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2906987..f88d60e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1726,6 +1726,12 @@
 			kernel and module base offset ASLR (Address Space
 			Layout Randomization).
 
+	kasan_multi_shot
+			[KNL] Enforce KASAN (Kernel Address Sanitizer) to print
+			report on every invalid memory access. Without this
+			parameter KASAN will print report only for the first
+			invalid access.
+
 	keepinitrd	[HW,ARM]
 
 	kernelcore=	[KNL,X86,IA-64,PPC]
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 5734480c9..a5c7046 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -76,6 +76,9 @@ size_t ksize(const void *);
 static inline void kasan_unpoison_slab(const void *ptr) { ksize(ptr); }
 size_t kasan_metadata_size(struct kmem_cache *cache);
 
+bool kasan_save_enable_multi_shot(void);
+void kasan_restore_multi_shot(bool enabled);
+
 #else /* CONFIG_KASAN */
 
 static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b1d314..a25c976 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -20,6 +20,7 @@
 #include <linux/string.h>
 #include <linux/uaccess.h>
 #include <linux/module.h>
+#include <linux/kasan.h>
 
 /*
  * Note: test functions are marked noinline so that their names appear in
@@ -474,6 +475,12 @@ static noinline void __init use_after_scope_test(void)
 
 static int __init kmalloc_tests_init(void)
 {
+	/*
+	 * Temporarily enable multi-shot mode. Otherwise, we'd only get a
+	 * report for the first case.
+	 */
+	bool multishot = kasan_save_enable_multi_shot();
+
 	kmalloc_oob_right();
 	kmalloc_oob_left();
 	kmalloc_node_oob_right();
@@ -499,6 +506,9 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+
+	kasan_restore_multi_shot(multishot);
+
 	return -EAGAIN;
 }
 
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 7572917..1229298 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -96,11 +96,6 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 		<< KASAN_SHADOW_SCALE_SHIFT);
 }
 
-static inline bool kasan_report_enabled(void)
-{
-	return !current->kasan_depth;
-}
-
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
 void kasan_report_double_free(struct kmem_cache *cache, void *object,
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 718a10a..beee0e9 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -13,7 +13,9 @@
  *
  */
 
+#include <linux/bitops.h>
 #include <linux/ftrace.h>
+#include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/printk.h>
@@ -354,6 +356,40 @@ static void kasan_report_error(struct kasan_access_info *info)
 	kasan_end_report(&flags);
 }
 
+static unsigned long kasan_flags;
+
+#define KASAN_BIT_REPORTED	0
+#define KASAN_BIT_MULTI_SHOT	1
+
+bool kasan_save_enable_multi_shot(void)
+{
+	return test_and_set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_save_enable_multi_shot);
+
+void kasan_restore_multi_shot(bool enabled)
+{
+	if (!enabled)
+		clear_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+}
+EXPORT_SYMBOL_GPL(kasan_restore_multi_shot);
+
+static int __init kasan_set_multi_shot(char *str)
+{
+	set_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags);
+	return 1;
+}
+__setup("kasan_multi_shot", kasan_set_multi_shot);
+
+static inline bool kasan_report_enabled(void)
+{
+	if (current->kasan_depth)
+		return false;
+	if (test_bit(KASAN_BIT_MULTI_SHOT, &kasan_flags))
+		return true;
+	return !test_and_set_bit(KASAN_BIT_REPORTED, &kasan_flags);
+}
+
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip)
 {
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2017-03-23 15:44 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22 16:06 [PATCH] kasan: report only the first error Andrey Ryabinin
2017-03-22 16:06 ` Andrey Ryabinin
2017-03-22 16:34 ` Andrey Konovalov
2017-03-22 16:34   ` Andrey Konovalov
2017-03-22 16:54   ` Andrey Ryabinin
2017-03-22 16:54     ` Andrey Ryabinin
2017-03-22 17:07     ` Andrey Konovalov
2017-03-22 17:07       ` Andrey Konovalov
2017-03-22 17:33       ` Alexander Potapenko
2017-03-22 17:33         ` Alexander Potapenko
2017-03-22 17:42         ` Dmitry Vyukov
2017-03-22 17:42           ` Dmitry Vyukov
2017-03-23 11:49 ` [PATCH v2] kasan: report only the first error by default Andrey Ryabinin
2017-03-23 11:49   ` Andrey Ryabinin
2017-03-23 12:41   ` Mark Rutland
2017-03-23 12:41     ` Mark Rutland
2017-03-23 13:06     ` Andrey Ryabinin
2017-03-23 13:06       ` Andrey Ryabinin
2017-03-23 13:29       ` Mark Rutland
2017-03-23 13:29         ` Mark Rutland
2017-03-23 15:44 ` [PATCH v3] " Andrey Ryabinin
2017-03-23 15:44   ` Andrey Ryabinin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.