| 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
| ||
|
Message-ID: <CANpmjNNDRb+wokzagQtLRVvZrj-8eH87gOX1JwG9hWf+eicRNg@mail.gmail.com>
Date: Thu, 21 May 2020 15:26:48 +0200
From: Marco Elver <elver@...gle.com>
To: Will Deacon <will@...nel.org>
Cc: "Paul E. McKenney" <paulmck@...nel.org>,
Dmitry Vyukov <dvyukov@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Andrey Konovalov <andreyknvl@...gle.com>,
kasan-dev <kasan-dev@...glegroups.com>,
LKML <linux-kernel@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
clang-built-linux <clang-built-linux@...glegroups.com>,
Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH -tip v2 03/11] kcsan: Support distinguishing volatile accesses
On Thu, 21 May 2020 at 15:18, Will Deacon <will@...nel.org> wrote:
>
> On Thu, May 21, 2020 at 01:08:46PM +0200, Marco Elver wrote:
> > In the kernel, volatile is used in various concurrent context, whether
> > in low-level synchronization primitives or for legacy reasons. If
> > supported by the compiler, we will assume that aligned volatile accesses
> > up to sizeof(long long) (matching compiletime_assert_rwonce_type()) are
> > atomic.
> >
> > Recent versions Clang [1] (GCC tentative [2]) can instrument volatile
> > accesses differently. Add the option (required) to enable the
> > instrumentation, and provide the necessary runtime functions. None of
> > the updated compilers are widely available yet (Clang 11 will be the
> > first release to support the feature).
> >
> > [1] https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814
> > [2] https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544452.html
> >
> > This patch allows removing any explicit checks in primitives such as
> > READ_ONCE() and WRITE_ONCE().
> >
> > Signed-off-by: Marco Elver <elver@...gle.com>
> > ---
> > v2:
> > * Reword Makefile comment.
> > ---
> > kernel/kcsan/core.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > scripts/Makefile.kcsan | 5 ++++-
> > 2 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
> > index a73a66cf79df..15f67949d11e 100644
> > --- a/kernel/kcsan/core.c
> > +++ b/kernel/kcsan/core.c
> > @@ -789,6 +789,49 @@ void __tsan_write_range(void *ptr, size_t size)
> > }
> > EXPORT_SYMBOL(__tsan_write_range);
> >
> > +/*
> > + * Use of explicit volatile is generally disallowed [1], however, volatile is
> > + * still used in various concurrent context, whether in low-level
> > + * synchronization primitives or for legacy reasons.
> > + * [1] https://lwn.net/Articles/233479/
> > + *
> > + * We only consider volatile accesses atomic if they are aligned and would pass
> > + * the size-check of compiletime_assert_rwonce_type().
> > + */
> > +#define DEFINE_TSAN_VOLATILE_READ_WRITE(size) \
> > + void __tsan_volatile_read##size(void *ptr) \
> > + { \
> > + const bool is_atomic = size <= sizeof(long long) && \
> > + IS_ALIGNED((unsigned long)ptr, size); \
> > + if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
> > + return; \
> > + check_access(ptr, size, is_atomic ? KCSAN_ACCESS_ATOMIC : 0); \
> > + } \
> > + EXPORT_SYMBOL(__tsan_volatile_read##size); \
> > + void __tsan_unaligned_volatile_read##size(void *ptr) \
> > + __alias(__tsan_volatile_read##size); \
> > + EXPORT_SYMBOL(__tsan_unaligned_volatile_read##size); \
> > + void __tsan_volatile_write##size(void *ptr) \
> > + { \
> > + const bool is_atomic = size <= sizeof(long long) && \
> > + IS_ALIGNED((unsigned long)ptr, size); \
> > + if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
> > + return; \
> > + check_access(ptr, size, \
> > + KCSAN_ACCESS_WRITE | \
> > + (is_atomic ? KCSAN_ACCESS_ATOMIC : 0)); \
> > + } \
> > + EXPORT_SYMBOL(__tsan_volatile_write##size); \
> > + void __tsan_unaligned_volatile_write##size(void *ptr) \
> > + __alias(__tsan_volatile_write##size); \
> > + EXPORT_SYMBOL(__tsan_unaligned_volatile_write##size)
> > +
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(1);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(2);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(4);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(8);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(16);
>
> Having a 16-byte case seems a bit weird to me, but I guess clang needs this
> for some reason?
Yes, the emitted fixed-size instrumentation is up to 16 bytes, so
we'll need it (for both volatile and non-volatile -- otherwise we'll
get linker errors). It doesn't mean we'll consider 16 byte volatile
accesses as atomic, because of the size check to compute is_atomic
above.
Thanks,
-- Marco
Powered by blists - more mailing lists