Re: [PATCH 1/2] kcsan: Add __kcsan_{enable,disable}_current() variants

From: Paul E. McKenney
Date: Fri Apr 24 2020 - 20:17:13 EST


On Fri, Apr 24, 2020 at 05:57:04PM +0200, Marco Elver wrote:
> On Fri, 24 Apr 2020 at 17:47, Marco Elver <elver@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > This is to help with the new READ_ONCE()/WRITE_ONCE():
> > 20200424134238.GE21141@willie-the-truck">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.

For the moment I have pulled them into -rcu and am testing them,
thank you! I will leave them in the v5.9 bucket for the moment,
but please let me know how things proceed with Will.

Thanx, Paul

> 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
> >