Re: [PATCH 2/3] asm-generic, x86: wrap atomic operations

[Andrey Ryabinin]

On 03/20/2017 08:17 PM, Mark Rutland wrote:
> Hi,
> 
> On Tue, Mar 14, 2017 at 08:24:13PM +0100, Dmitry Vyukov wrote:
>>  /**
>> - * atomic_read - read atomic variable
>> + * arch_atomic_read - read atomic variable
>>   * @v: pointer of type atomic_t
>>   *
>>   * Atomically reads the value of @v.
>>   */
>> -static __always_inline int atomic_read(const atomic_t *v)
>> +static __always_inline int arch_atomic_read(const atomic_t *v)
>>  {
>> -	return READ_ONCE((v)->counter);
>> +	/*
>> +	 * We use READ_ONCE_NOCHECK() because atomic_read() contains KASAN
>> +	 * instrumentation. Double instrumentation is unnecessary.
>> +	 */
>> +	return READ_ONCE_NOCHECK((v)->counter);
>>  }
> 
> Just to check, we do this to avoid duplicate reports, right?
> 
> If so, double instrumentation isn't solely "unnecessary"; it has a
> functional difference, and we should explicitly describe that in the
> comment.
> 
> ... or are duplicate reports supressed somehow?
> 

They are not suppressed yet. But I think we should just switch kasan to single shot mode,
i.e. report only the first error. Single bug quite often has multiple invalid memory accesses
causing storm in dmesg. Also write OOB might corrupt metadata so the next report will print
bogus alloc/free stacktraces.
In most cases we need to look only at the first report, so reporting anything after the first
is just counterproductive.