linux-kernel - Re: [PATCH v2] kasan: report only the first error by default

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d9be02d7-af87-208a-c51b-c890b549434b@virtuozzo.com>
Date:   Thu, 23 Mar 2017 16:06:59 +0300
From:   Andrey Ryabinin <aryabinin@...tuozzo.com>
To:     Mark Rutland <mark.rutland@....com>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        <kasan-dev@...glegroups.com>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] kasan: report only the first error by default



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)