linux-kernel - Re: [PATCH 1/2] kcsan: Add __kcsan_{enable,disable}

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]

Date:   Fri, 24 Apr 2020 17:57:04 +0200
From:   Marco Elver <elver@...gle.com>
To:     Marco Elver <elver@...gle.com>
Cc:     "Paul E. McKenney" <paulmck@...nel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Will Deacon <will@...nel.org>,
        kasan-dev <kasan-dev@...glegroups.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 1/2] kcsan: Add __kcsan_{enable,disable}_current() variants

On Fri, 24 Apr 2020 at 17:47, Marco Elver <elver@...gle.com> wrote:
>
> The __kcsan_{enable,disable}_current() variants only call into KCSAN if
> KCSAN is enabled for the current compilation unit. Note: This is
> typically not what we want, as we usually want to ensure that even calls
> into other functions still have KCSAN disabled.
>
> These variants may safely be used in header files that are shared
> between regular kernel code and code that does not link the KCSAN
> runtime.
>
> Signed-off-by: Marco Elver <elver@...gle.com>
> ---
> This is to help with the new READ_ONCE()/WRITE_ONCE():
> https://lkml.kernel.org/r/20200424134238.GE21141@willie-the-truck
>
> These should be using __kcsan_disable_current() and
> __kcsan_enable_current(), instead of the non-'__' variants.
> ---

Paul: These 2 patches may want to be in the set for 5.8, depending on
what Will wants to do.

An alternative would be that Will takes my 2 patches and carries them,
avoiding some complex patch-dependency. That is assuming his set of
patches will go in -tip on top of KCSAN.

Thanks,
-- Marco

>  include/linux/kcsan-checks.h | 17 ++++++++++++++---
>  kernel/kcsan/core.c          |  7 +++++++
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
> index ef95ddc49182..7b0b9c44f5f3 100644
> --- a/include/linux/kcsan-checks.h
> +++ b/include/linux/kcsan-checks.h
> @@ -49,6 +49,7 @@ void kcsan_disable_current(void);
>   * Supports nesting.
>   */
>  void kcsan_enable_current(void);
> +void kcsan_enable_current_nowarn(void); /* Safe in uaccess regions. */
>
>  /**
>   * kcsan_nestable_atomic_begin - begin nestable atomic region
> @@ -149,6 +150,7 @@ static inline void __kcsan_check_access(const volatile void *ptr, size_t size,
>
>  static inline void kcsan_disable_current(void)         { }
>  static inline void kcsan_enable_current(void)          { }
> +static inline void kcsan_enable_current_nowarn(void)   { }
>  static inline void kcsan_nestable_atomic_begin(void)   { }
>  static inline void kcsan_nestable_atomic_end(void)     { }
>  static inline void kcsan_flat_atomic_begin(void)       { }
> @@ -165,15 +167,24 @@ static inline void kcsan_end_scoped_access(struct kcsan_scoped_access *sa) { }
>
>  #endif /* CONFIG_KCSAN */
>
> +#ifdef __SANITIZE_THREAD__
>  /*
> - * kcsan_*: Only calls into the runtime when the particular compilation unit has
> - * KCSAN instrumentation enabled. May be used in header files.
> + * Only calls into the runtime when the particular compilation unit has KCSAN
> + * instrumentation enabled. May be used in header files.
>   */
> -#ifdef __SANITIZE_THREAD__
>  #define kcsan_check_access __kcsan_check_access
> +
> +/*
> + * Only use these to disable KCSAN for accesses in the current compilation unit;
> + * calls into libraries may still perform KCSAN checks.
> + */
> +#define __kcsan_disable_current kcsan_disable_current
> +#define __kcsan_enable_current kcsan_enable_current_nowarn
>  #else
>  static inline void kcsan_check_access(const volatile void *ptr, size_t size,
>                                       int type) { }
> +static inline void __kcsan_enable_current(void)  { }
> +static inline void __kcsan_disable_current(void) { }
>  #endif
>
>  /**
> diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
> index 40919943617b..0a0f018cb154 100644
> --- a/kernel/kcsan/core.c
> +++ b/kernel/kcsan/core.c
> @@ -625,6 +625,13 @@ void kcsan_enable_current(void)
>  }
>  EXPORT_SYMBOL(kcsan_enable_current);
>
> +void kcsan_enable_current_nowarn(void)
> +{
> +       if (get_ctx()->disable_count-- == 0)
> +               kcsan_disable_current();
> +}
> +EXPORT_SYMBOL(kcsan_enable_current_nowarn);
> +
>  void kcsan_nestable_atomic_begin(void)
>  {
>         /*
> --
> 2.26.2.303.gf8c07b1a785-goog
>