Re: [PATCH] mm/kasan/kasan.h: Fix boolean checking issue for kasan_report_enabled()

[Dmitry Vyukov]

On Mon, May 2, 2016 at 7:36 AM,  <chengang@xxxxxxxxxxxxxxxx> wrote:
> From: Chen Gang <chengang@xxxxxxxxxxxxxxxx>
>
> According to kasan_[dis|en]able_current() comments and the kasan_depth'
> s initialization, if kasan_depth is zero, it means disable.
>
> So need use "!!kasan_depth" instead of "!kasan_depth" for checking
> enable.
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@xxxxxxxxx>
> ---
>  mm/kasan/kasan.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 7da78a6..6464b8f 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -102,7 +102,7 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
>
>  static inline bool kasan_report_enabled(void)
>  {
> -       return !current->kasan_depth;
> +       return !!current->kasan_depth;
>  }
>
>  void kasan_report(unsigned long addr, size_t size,

Hi Chen,

I don't think this is correct.
We seem to have some incorrect comments around kasan_depth, and a
weird way of manipulating it (disable should increment, and enable
should decrement). But in the end it is working. This change will
suppress all true reports and enable all false reports.

If you want to improve kasan_depth handling, then please fix the
comments and make disable increment and enable decrement (potentially
with WARNING on overflow/underflow). It's better to produce a WARNING
rather than silently ignore the error. We've ate enough unmatched
annotations in user space (e.g. enable is skipped on an error path).
These unmatched annotations are hard to notice (they suppress
reports). So in user space we bark loudly on overflows/underflows and
also check that a thread does not exit with enabled suppressions.

Thanks.