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

[Andrey Ryabinin]

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)