Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1606731 > unrolled thread

Re: [PATCH] kasan: report only the first error

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] kasan: report only the first error Andrey Konovalov <andreyknvl@google.com> - 2017-03-22 17:40 +0100
    Re: [PATCH] kasan: report only the first error Andrey Konovalov <andreyknvl@google.com> - 2017-03-22 18:10 +0100
      Re: [PATCH] kasan: report only the first error Alexander Potapenko <glider@google.com> - 2017-03-22 18:40 +0100
        Re: [PATCH] kasan: report only the first error Dmitry Vyukov <dvyukov@google.com> - 2017-03-22 18:50 +0100

#1606731 — Re: [PATCH] kasan: report only the first error

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.

[toc] | [next] | [standalone]

#1606764

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

[toc] | [prev] | [next] | [standalone]

#1606800

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

[toc] | [prev] | [next] | [standalone]

#1606838

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web