Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1245678 > unrolled thread

[PATCH v2 0/2] Silence KASAN warnings in get_wchan()

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/2] Silence KASAN warnings in get_wchan() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-13 14:50 +0200
    [PATCH v3 0/2] Silence KASAN warnings in get_wchan() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-13 17:30 +0200
      [PATCH v3 1/2] Provide READ_ONCE_NOCHECK() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-13 17:40 +0200
        [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK  () tip-bot for Andrey Ryabinin <tipbot@zytor.com> - 2015-10-14 17:40 +0200
          Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-14 17:50 +0200
            Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Dmitry Vyukov <dvyukov@google.com> - 2015-10-14 18:00 +0200
              Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Dmitry Vyukov <dvyukov@google.com> - 2015-10-14 18:10 +0200
                Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () Peter Zijlstra <peterz@infradead.org> - 2015-10-14 18:20 +0200
                  Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Dmitry Vyukov <dvyukov@google.com> - 2015-10-14 18:20 +0200
                    Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Andy Lutomirski <luto@amacapital.net> - 2015-10-14 18:30 +0200
                      Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () Peter Zijlstra <peterz@infradead.org> - 2015-10-14 18:40 +0200
                        Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () Ingo Molnar <mingo@kernel.org> - 2015-10-14 19:50 +0200
                          Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Andy Lutomirski <luto@amacapital.net> - 2015-10-14 20:00 +0200
                    Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () Peter Zijlstra <peterz@infradead.org> - 2015-10-14 18:30 +0200
                      Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Dmitry Vyukov <dvyukov@google.com> - 2015-10-14 18:40 +0200
                        Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () Peter Zijlstra <peterz@infradead.org> - 2015-10-14 19:00 +0200
                Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-14 18:30 +0200
                  Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Dmitry Vyukov <dvyukov@google.com> - 2015-10-14 18:40 +0200
                    Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-14 19:10 +0200
                      Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Dmitry Vyukov <dvyukov@google.com> - 2015-10-14 19:30 +0200
                        Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-14 19:40 +0200
              Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-14 18:10 +0200
              Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK  () Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-14 18:20 +0200
                Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK () Dmitry Vyukov <dvyukov@google.com> - 2015-10-14 18:40 +0200
                  Re: [tip:locking/urgent] compiler, atomics: Provide  READ_ONCE_NOCHECK () "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-14 19:10 +0200
        linux-next: build problems (Was: [PATCH v3 1/2] Provide  READ_ONCE_NOCHECK()) Stephen Rothwell <sfr@canb.auug.org.au> - 2015-10-15 11:20 +0200
          Re: linux-next: build problems (Was: [PATCH v3 1/2] Provide  READ_ONCE_NOCHECK()) Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-15 12:10 +0200
          [PATCH] compiler, READ_ONCE: Fix build failure with some older GCC Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-15 12:30 +0200
            Re: [PATCH] compiler, READ_ONCE: Fix build failure with some older  GCC Ingo Molnar <mingo@kernel.org> - 2015-10-15 13:40 +0200
      [PATCH v3 2/2] x86/process: Silence KASAN warnings in get_wchan() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-13 17:40 +0200
        [tip:locking/urgent] x86/mm: Silence KASAN warnings in get_wchan(  ) tip-bot for Andrey Ryabinin <tipbot@zytor.com> - 2015-10-14 17:40 +0200
    [PATCH v4 2/2] x86/mm: Silence KASAN warnings in get_wchan() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-16 11:50 +0200
    [PATCH v4 0/2] Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-16 11:50 +0200
      [PATCH v4 1/2] compiler, atomics: Provide READ_ONCE_NOKSAN() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-16 11:50 +0200
        Re: [PATCH v4 1/2] compiler, atomics: Provide READ_ONCE_NOKSAN() Peter Zijlstra <peterz@infradead.org> - 2015-10-16 12:10 +0200
          Re: [PATCH v4 1/2] compiler, atomics: Provide READ_ONCE_NOKSAN() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-16 13:00 +0200
            Re: [PATCH v4 1/2] compiler, atomics: Provide READ_ONCE_NOKSAN() Peter Zijlstra <peterz@infradead.org> - 2015-10-16 13:10 +0200
        Re: [PATCH v4 1/2] compiler, atomics: Provide READ_ONCE_NOKSAN() Borislav Petkov <bp@alien8.de> - 2015-10-16 12:40 +0200
          Re: [PATCH v4 1/2] compiler, atomics: Provide READ_ONCE_NOKSAN() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-16 14:00 +0200
          Re: [PATCH v4 1/2] compiler, atomics: Provide READ_ONCE_NOKSAN() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-16 18:10 +0200
      [PATCH v4 0/2] Silence KASAN warnings in get_wchan() Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-10-16 11:50 +0200

Page 1 of 3  [1] 2 3  Next page →

#1245678 — [PATCH v2 0/2] Silence KASAN warnings in get_wchan()

Originally I suggested to implement READ_ONCE_NOCHECK(), like this:
#define READ_ONCE_NOCHECK(x) \
{(	typeof(x) __val; \
	kasan_disable_local(); \
	__val = READ_ONCE(x); \
	kasan_enable_local(); \
	__val;\
)}

But then I realised that we can't put this into linux/compiler.h
since it requires to add some includes (we need linux/kasan.h which
also requires linux/sched.h). It's not even that simple to put this into kernel.h

So I've come to another approach, see the first patch for details.
Pros:
 - It generates more efficient code rather than variant above
Cons:
 - REAS_ONCE() becomes rather messy.


Andrey Ryabinin (2):
  Provide READ_ONCE_NOCHECK()
  x86/process: Silence KASAN warnings in get_wchan()

 arch/x86/kernel/process.c    |  6 +++---
 include/linux/compiler-gcc.h | 13 ++++++++++++
 include/linux/compiler.h     | 49 ++++++++++++++++++++++++++++++++------------
 3 files changed, 52 insertions(+), 16 deletions(-)

-- 
2.4.9

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1245836 — [PATCH v3 0/2] Silence KASAN warnings in get_wchan()

Changes since v2:
 - Added some code comments in the first patch.
 
Andrey Ryabinin (2):
  Provide READ_ONCE_NOCHECK()
  x86/process: Silence KASAN warnings in get_wchan()

 arch/x86/kernel/process.c    |  6 ++---
 include/linux/compiler-gcc.h | 13 ++++++++++
 include/linux/compiler.h     | 60 ++++++++++++++++++++++++++++++++++----------
 3 files changed, 63 insertions(+), 16 deletions(-)

-- 
2.4.9

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245842 — [PATCH v3 1/2] Provide READ_ONCE_NOCHECK()

Some code may perform racy by design memory reads. This could be harmless,
yet such code may produce KASAN warnings.

To hide such accesses from KASAN this patch introduces READ_ONCE_NOCHECK()
macro. KASAN will not check the memory accessed by READ_ONCE_NOCHECK().

This patch creates __read_once_size_nocheck() a clone of
__read_once_size_check() (renamed __read_once_size()).
The only difference between them is 'no_sanitized_address' attribute
appended to '*_nocheck' function. This attribute tells the compiler that
instrumentation of memory accesses should not be applied to that function.
We declare it as static '__maybe_unsed' because GCC is not capable to
inline such function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368

With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 include/linux/compiler-gcc.h | 13 ++++++++++
 include/linux/compiler.h     | 60 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index dfaa7b3..f2a9aec 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -237,12 +237,25 @@
 #define KASAN_ABI_VERSION 3
 #endif
 
+#if GCC_VERSION >= 40902
+/*
+ * Tell the compiler that address safety instrumentation (KASAN)
+ * should not be applied to that function.
+ * Confilcts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
+ */
+#define __no_sanitize_address __attribute__((no_sanitize_address))
+#endif
+
 #endif	/* gcc version >= 40000 specific checks */
 
 #if !defined(__noclone)
 #define __noclone	/* not needed */
 #endif
 
+#if !defined(__no_sanitize_address)
+#define __no_sanitize_address
+#endif
+
 /*
  * A trick to suppress uninitialized variable warning without generating any
  * code
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index c836eb2..aa2ae4c 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -198,19 +198,42 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
 
 #include <uapi/linux/types.h>
 
-static __always_inline void __read_once_size(const volatile void *p, void *res, int size)
+#define __READ_ONCE_SIZE						\
+({									\
+	switch (size) {							\
+	case 1: *(__u8 *)res = *(volatile __u8 *)p; break;		\
+	case 2: *(__u16 *)res = *(volatile __u16 *)p; break;		\
+	case 4: *(__u32 *)res = *(volatile __u32 *)p; break;		\
+	case 8: *(__u64 *)res = *(volatile __u64 *)p; break;		\
+	default:							\
+		barrier();						\
+		__builtin_memcpy((void *)res, (const void *)p, size);	\
+		barrier();						\
+	}								\
+})
+
+static __always_inline
+void __read_once_size_check(const volatile void *p, void *res, int size)
 {
-	switch (size) {
-	case 1: *(__u8 *)res = *(volatile __u8 *)p; break;
-	case 2: *(__u16 *)res = *(volatile __u16 *)p; break;
-	case 4: *(__u32 *)res = *(volatile __u32 *)p; break;
-	case 8: *(__u64 *)res = *(volatile __u64 *)p; break;
-	default:
-		barrier();
-		__builtin_memcpy((void *)res, (const void *)p, size);
-		barrier();
-	}
+	__READ_ONCE_SIZE;
+}
+
+#ifdef CONFIG_KASAN
+/*
+ * This function is not 'inline' because __no_sanitize_address confilcts
+ * with inlining. Attempt to inline it may cause a build failure.
+ * 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
+ * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
+ */
+static __no_sanitize_address __maybe_unused
+void __read_once_size_nocheck(const volatile void *p, void *res, int size)
+{
+	__READ_ONCE_SIZE;
 }
+#else
+static __always_inline __alias(__read_once_size_check)
+void __read_once_size_nocheck(const volatile void *p, void *res, int size);
+#endif
 
 static __always_inline void __write_once_size(volatile void *p, void *res, int size)
 {
@@ -248,8 +271,19 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
  * required ordering.
  */
 
-#define READ_ONCE(x) \
-	({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
+#define __READ_ONCE(x, check)					\
+({								\
+	union { typeof(x) __val; char __c[1]; } __u;		\
+	__read_once_size##check(&(x), __u.__c, sizeof(x));	\
+	__u.__val;						\
+})
+#define READ_ONCE(x) __READ_ONCE(x, _check)
+
+/*
+ * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
+ * to hide memory access from KASAN.
+ */
+#define READ_ONCE_NOCHECK(x) __READ_ONCE(x, _nocheck)
 
 #define WRITE_ONCE(x, val) \
 ({							\
-- 
2.4.9

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246905 — [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 14 Oct 2015 16:44:06 +0200

compiler, atomics: Provide READ_ONCE_NOCHECK()

Some code may perform racy by design memory reads. This could be
harmless, yet such code may produce KASAN warnings.

To hide such accesses from KASAN this patch introduces
READ_ONCE_NOCHECK() macro. KASAN will not check the memory
accessed by READ_ONCE_NOCHECK().

This patch creates __read_once_size_nocheck() a clone of
__read_once_size_check() (renamed __read_once_size()).
The only difference between them is 'no_sanitized_address'
attribute appended to '*_nocheck' function. This attribute tells
the compiler that instrumentation of memory accesses should not
be applied to that function. We declare it as static
'__maybe_unsed' because GCC is not capable to inline such
function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368

With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wolfram Gloger <wmglo@dent.med.uni-muenchen.de>
Cc: kasan-dev <kasan-dev@googlegroups.com>
Link: http://lkml.kernel.org/r/1444750088-24444-2-git-send-email-aryabinin@virtuozzo.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/compiler-gcc.h | 13 ++++++++++
 include/linux/compiler.h     | 60 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index dfaa7b3..f2a9aec 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -237,12 +237,25 @@
 #define KASAN_ABI_VERSION 3
 #endif
 
+#if GCC_VERSION >= 40902
+/*
+ * Tell the compiler that address safety instrumentation (KASAN)
+ * should not be applied to that function.
+ * Confilcts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
+ */
+#define __no_sanitize_address __attribute__((no_sanitize_address))
+#endif
+
 #endif	/* gcc version >= 40000 specific checks */
 
 #if !defined(__noclone)
 #define __noclone	/* not needed */
 #endif
 
+#if !defined(__no_sanitize_address)
+#define __no_sanitize_address
+#endif
+
 /*
  * A trick to suppress uninitialized variable warning without generating any
  * code
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index c836eb2..aa2ae4c 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -198,19 +198,42 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
 
 #include <uapi/linux/types.h>
 
-static __always_inline void __read_once_size(const volatile void *p, void *res, int size)
+#define __READ_ONCE_SIZE						\
+({									\
+	switch (size) {							\
+	case 1: *(__u8 *)res = *(volatile __u8 *)p; break;		\
+	case 2: *(__u16 *)res = *(volatile __u16 *)p; break;		\
+	case 4: *(__u32 *)res = *(volatile __u32 *)p; break;		\
+	case 8: *(__u64 *)res = *(volatile __u64 *)p; break;		\
+	default:							\
+		barrier();						\
+		__builtin_memcpy((void *)res, (const void *)p, size);	\
+		barrier();						\
+	}								\
+})
+
+static __always_inline
+void __read_once_size_check(const volatile void *p, void *res, int size)
 {
-	switch (size) {
-	case 1: *(__u8 *)res = *(volatile __u8 *)p; break;
-	case 2: *(__u16 *)res = *(volatile __u16 *)p; break;
-	case 4: *(__u32 *)res = *(volatile __u32 *)p; break;
-	case 8: *(__u64 *)res = *(volatile __u64 *)p; break;
-	default:
-		barrier();
-		__builtin_memcpy((void *)res, (const void *)p, size);
-		barrier();
-	}
+	__READ_ONCE_SIZE;
+}
+
+#ifdef CONFIG_KASAN
+/*
+ * This function is not 'inline' because __no_sanitize_address confilcts
+ * with inlining. Attempt to inline it may cause a build failure.
+ * 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
+ * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
+ */
+static __no_sanitize_address __maybe_unused
+void __read_once_size_nocheck(const volatile void *p, void *res, int size)
+{
+	__READ_ONCE_SIZE;
 }
+#else
+static __always_inline __alias(__read_once_size_check)
+void __read_once_size_nocheck(const volatile void *p, void *res, int size);
+#endif
 
 static __always_inline void __write_once_size(volatile void *p, void *res, int size)
 {
@@ -248,8 +271,19 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
  * required ordering.
  */
 
-#define READ_ONCE(x) \
-	({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
+#define __READ_ONCE(x, check)					\
+({								\
+	union { typeof(x) __val; char __c[1]; } __u;		\
+	__read_once_size##check(&(x), __u.__c, sizeof(x));	\
+	__u.__val;						\
+})
+#define READ_ONCE(x) __READ_ONCE(x, _check)
+
+/*
+ * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
+ * to hide memory access from KASAN.
+ */
+#define READ_ONCE_NOCHECK(x) __READ_ONCE(x, _nocheck)
 
 #define WRITE_ONCE(x, val) \
 ({							\
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246924 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 08:28:43AM -0700, tip-bot for Andrey Ryabinin wrote:
> Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
> Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
> Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
> AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Wed, 14 Oct 2015 16:44:06 +0200
> 
> compiler, atomics: Provide READ_ONCE_NOCHECK()
> 
> Some code may perform racy by design memory reads. This could be
> harmless, yet such code may produce KASAN warnings.
> 
> To hide such accesses from KASAN this patch introduces
> READ_ONCE_NOCHECK() macro. KASAN will not check the memory
> accessed by READ_ONCE_NOCHECK().
> 
> This patch creates __read_once_size_nocheck() a clone of
> __read_once_size_check() (renamed __read_once_size()).
> The only difference between them is 'no_sanitized_address'
> attribute appended to '*_nocheck' function. This attribute tells
> the compiler that instrumentation of memory accesses should not
> be applied to that function. We declare it as static
> '__maybe_unsed' because GCC is not capable to inline such
> function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> 
> With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().

So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
prove safe address for KASAN's benefit, but READ_ONCE() suffices for
the data-race-detection logic in KTSAN, correct?

							Thanx, Paul

> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Kostya Serebryany <kcc@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Wolfram Gloger <wmglo@dent.med.uni-muenchen.de>
> Cc: kasan-dev <kasan-dev@googlegroups.com>
> Link: http://lkml.kernel.org/r/1444750088-24444-2-git-send-email-aryabinin@virtuozzo.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  include/linux/compiler-gcc.h | 13 ++++++++++
>  include/linux/compiler.h     | 60 ++++++++++++++++++++++++++++++++++----------
>  2 files changed, 60 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index dfaa7b3..f2a9aec 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -237,12 +237,25 @@
>  #define KASAN_ABI_VERSION 3
>  #endif
> 
> +#if GCC_VERSION >= 40902
> +/*
> + * Tell the compiler that address safety instrumentation (KASAN)
> + * should not be applied to that function.
> + * Confilcts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> + */
> +#define __no_sanitize_address __attribute__((no_sanitize_address))
> +#endif
> +
>  #endif	/* gcc version >= 40000 specific checks */
> 
>  #if !defined(__noclone)
>  #define __noclone	/* not needed */
>  #endif
> 
> +#if !defined(__no_sanitize_address)
> +#define __no_sanitize_address
> +#endif
> +
>  /*
>   * A trick to suppress uninitialized variable warning without generating any
>   * code
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index c836eb2..aa2ae4c 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -198,19 +198,42 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
> 
>  #include <uapi/linux/types.h>
> 
> -static __always_inline void __read_once_size(const volatile void *p, void *res, int size)
> +#define __READ_ONCE_SIZE						\
> +({									\
> +	switch (size) {							\
> +	case 1: *(__u8 *)res = *(volatile __u8 *)p; break;		\
> +	case 2: *(__u16 *)res = *(volatile __u16 *)p; break;		\
> +	case 4: *(__u32 *)res = *(volatile __u32 *)p; break;		\
> +	case 8: *(__u64 *)res = *(volatile __u64 *)p; break;		\
> +	default:							\
> +		barrier();						\
> +		__builtin_memcpy((void *)res, (const void *)p, size);	\
> +		barrier();						\
> +	}								\
> +})
> +
> +static __always_inline
> +void __read_once_size_check(const volatile void *p, void *res, int size)
>  {
> -	switch (size) {
> -	case 1: *(__u8 *)res = *(volatile __u8 *)p; break;
> -	case 2: *(__u16 *)res = *(volatile __u16 *)p; break;
> -	case 4: *(__u32 *)res = *(volatile __u32 *)p; break;
> -	case 8: *(__u64 *)res = *(volatile __u64 *)p; break;
> -	default:
> -		barrier();
> -		__builtin_memcpy((void *)res, (const void *)p, size);
> -		barrier();
> -	}
> +	__READ_ONCE_SIZE;
> +}
> +
> +#ifdef CONFIG_KASAN
> +/*
> + * This function is not 'inline' because __no_sanitize_address confilcts
> + * with inlining. Attempt to inline it may cause a build failure.
> + * 	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> + * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
> + */
> +static __no_sanitize_address __maybe_unused
> +void __read_once_size_nocheck(const volatile void *p, void *res, int size)
> +{
> +	__READ_ONCE_SIZE;
>  }
> +#else
> +static __always_inline __alias(__read_once_size_check)
> +void __read_once_size_nocheck(const volatile void *p, void *res, int size);
> +#endif
> 
>  static __always_inline void __write_once_size(volatile void *p, void *res, int size)
>  {
> @@ -248,8 +271,19 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>   * required ordering.
>   */
> 
> -#define READ_ONCE(x) \
> -	({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
> +#define __READ_ONCE(x, check)					\
> +({								\
> +	union { typeof(x) __val; char __c[1]; } __u;		\
> +	__read_once_size##check(&(x), __u.__c, sizeof(x));	\
> +	__u.__val;						\
> +})
> +#define READ_ONCE(x) __READ_ONCE(x, _check)
> +
> +/*
> + * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
> + * to hide memory access from KASAN.
> + */
> +#define READ_ONCE_NOCHECK(x) __READ_ONCE(x, _nocheck)
> 
>  #define WRITE_ONCE(x, val) \
>  ({							\
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246946 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 5:45 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Oct 14, 2015 at 08:28:43AM -0700, tip-bot for Andrey Ryabinin wrote:
>> Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
>> AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
>> Committer:  Ingo Molnar <mingo@kernel.org>
>> CommitDate: Wed, 14 Oct 2015 16:44:06 +0200
>>
>> compiler, atomics: Provide READ_ONCE_NOCHECK()
>>
>> Some code may perform racy by design memory reads. This could be
>> harmless, yet such code may produce KASAN warnings.
>>
>> To hide such accesses from KASAN this patch introduces
>> READ_ONCE_NOCHECK() macro. KASAN will not check the memory
>> accessed by READ_ONCE_NOCHECK().
>>
>> This patch creates __read_once_size_nocheck() a clone of
>> __read_once_size_check() (renamed __read_once_size()).
>> The only difference between them is 'no_sanitized_address'
>> attribute appended to '*_nocheck' function. This attribute tells
>> the compiler that instrumentation of memory accesses should not
>> be applied to that function. We declare it as static
>> '__maybe_unsed' because GCC is not capable to inline such
>> function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
>>
>> With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().
>
> So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
> prove safe address for KASAN's benefit, but READ_ONCE() suffices for
> the data-race-detection logic in KTSAN, correct?

KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
between get_wchan() and the thread accesses to own stack even more
aggressively than KASAN, because KTSAN won't like get_wchan() accesses
even to non-poisoned areas of other thread stack.



>                                                         Thanx, Paul
>
>> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
>> Cc: Alexander Potapenko <glider@google.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Andrey Konovalov <andreyknvl@google.com>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: Denys Vlasenko <dvlasenk@redhat.com>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Kostya Serebryany <kcc@google.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Sasha Levin <sasha.levin@oracle.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Wolfram Gloger <wmglo@dent.med.uni-muenchen.de>
>> Cc: kasan-dev <kasan-dev@googlegroups.com>
>> Link: http://lkml.kernel.org/r/1444750088-24444-2-git-send-email-aryabinin@virtuozzo.com
>> Signed-off-by: Ingo Molnar <mingo@kernel.org>
>> ---
>>  include/linux/compiler-gcc.h | 13 ++++++++++
>>  include/linux/compiler.h     | 60 ++++++++++++++++++++++++++++++++++----------
>>  2 files changed, 60 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index dfaa7b3..f2a9aec 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -237,12 +237,25 @@
>>  #define KASAN_ABI_VERSION 3
>>  #endif
>>
>> +#if GCC_VERSION >= 40902
>> +/*
>> + * Tell the compiler that address safety instrumentation (KASAN)
>> + * should not be applied to that function.
>> + * Confilcts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
>> + */
>> +#define __no_sanitize_address __attribute__((no_sanitize_address))
>> +#endif
>> +
>>  #endif       /* gcc version >= 40000 specific checks */
>>
>>  #if !defined(__noclone)
>>  #define __noclone    /* not needed */
>>  #endif
>>
>> +#if !defined(__no_sanitize_address)
>> +#define __no_sanitize_address
>> +#endif
>> +
>>  /*
>>   * A trick to suppress uninitialized variable warning without generating any
>>   * code
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index c836eb2..aa2ae4c 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -198,19 +198,42 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
>>
>>  #include <uapi/linux/types.h>
>>
>> -static __always_inline void __read_once_size(const volatile void *p, void *res, int size)
>> +#define __READ_ONCE_SIZE                                             \
>> +({                                                                   \
>> +     switch (size) {                                                 \
>> +     case 1: *(__u8 *)res = *(volatile __u8 *)p; break;              \
>> +     case 2: *(__u16 *)res = *(volatile __u16 *)p; break;            \
>> +     case 4: *(__u32 *)res = *(volatile __u32 *)p; break;            \
>> +     case 8: *(__u64 *)res = *(volatile __u64 *)p; break;            \
>> +     default:                                                        \
>> +             barrier();                                              \
>> +             __builtin_memcpy((void *)res, (const void *)p, size);   \
>> +             barrier();                                              \
>> +     }                                                               \
>> +})
>> +
>> +static __always_inline
>> +void __read_once_size_check(const volatile void *p, void *res, int size)
>>  {
>> -     switch (size) {
>> -     case 1: *(__u8 *)res = *(volatile __u8 *)p; break;
>> -     case 2: *(__u16 *)res = *(volatile __u16 *)p; break;
>> -     case 4: *(__u32 *)res = *(volatile __u32 *)p; break;
>> -     case 8: *(__u64 *)res = *(volatile __u64 *)p; break;
>> -     default:
>> -             barrier();
>> -             __builtin_memcpy((void *)res, (const void *)p, size);
>> -             barrier();
>> -     }
>> +     __READ_ONCE_SIZE;
>> +}
>> +
>> +#ifdef CONFIG_KASAN
>> +/*
>> + * This function is not 'inline' because __no_sanitize_address confilcts
>> + * with inlining. Attempt to inline it may cause a build failure.
>> + *   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
>> + * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
>> + */
>> +static __no_sanitize_address __maybe_unused
>> +void __read_once_size_nocheck(const volatile void *p, void *res, int size)
>> +{
>> +     __READ_ONCE_SIZE;
>>  }
>> +#else
>> +static __always_inline __alias(__read_once_size_check)
>> +void __read_once_size_nocheck(const volatile void *p, void *res, int size);
>> +#endif
>>
>>  static __always_inline void __write_once_size(volatile void *p, void *res, int size)
>>  {
>> @@ -248,8 +271,19 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>>   * required ordering.
>>   */
>>
>> -#define READ_ONCE(x) \
>> -     ({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
>> +#define __READ_ONCE(x, check)                                        \
>> +({                                                           \
>> +     union { typeof(x) __val; char __c[1]; } __u;            \
>> +     __read_once_size##check(&(x), __u.__c, sizeof(x));      \
>> +     __u.__val;                                              \
>> +})
>> +#define READ_ONCE(x) __READ_ONCE(x, _check)
>> +
>> +/*
>> + * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
>> + * to hide memory access from KASAN.
>> + */
>> +#define READ_ONCE_NOCHECK(x) __READ_ONCE(x, _nocheck)
>>
>>  #define WRITE_ONCE(x, val) \
>>  ({                                                   \
>>
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20151014154532.GV3910%40linux.vnet.ibm.com.
> For more options, visit https://groups.google.com/d/optout.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246952 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 6:01 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Oct 14, 2015 at 05:50:34PM +0200, Dmitry Vyukov wrote:
>> On Wed, Oct 14, 2015 at 5:45 PM, Paul E. McKenney
>> <paulmck@linux.vnet.ibm.com> wrote:
>> > On Wed, Oct 14, 2015 at 08:28:43AM -0700, tip-bot for Andrey Ryabinin wrote:
>> >> Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> >> Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> >> Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
>> >> AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
>> >> Committer:  Ingo Molnar <mingo@kernel.org>
>> >> CommitDate: Wed, 14 Oct 2015 16:44:06 +0200
>> >>
>> >> compiler, atomics: Provide READ_ONCE_NOCHECK()
>> >>
>> >> Some code may perform racy by design memory reads. This could be
>> >> harmless, yet such code may produce KASAN warnings.
>> >>
>> >> To hide such accesses from KASAN this patch introduces
>> >> READ_ONCE_NOCHECK() macro. KASAN will not check the memory
>> >> accessed by READ_ONCE_NOCHECK().
>> >>
>> >> This patch creates __read_once_size_nocheck() a clone of
>> >> __read_once_size_check() (renamed __read_once_size()).
>> >> The only difference between them is 'no_sanitized_address'
>> >> attribute appended to '*_nocheck' function. This attribute tells
>> >> the compiler that instrumentation of memory accesses should not
>> >> be applied to that function. We declare it as static
>> >> '__maybe_unsed' because GCC is not capable to inline such
>> >> function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
>> >>
>> >> With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().
>> >
>> > So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
>> > prove safe address for KASAN's benefit, but READ_ONCE() suffices for
>> > the data-race-detection logic in KTSAN, correct?
>>
>> KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
>> between get_wchan() and the thread accesses to own stack even more
>> aggressively than KASAN, because KTSAN won't like get_wchan() accesses
>> even to non-poisoned areas of other thread stack.
>
> So to keep KTSAN happy, any read from some other thread's stack requires
> READ_ONCE_NOCHECK()?  What if the access is via a locking primitive or
> read-modify-write atomic operation?
>
> This is of some interest in RCU, which implements synchronous grace
> periods using completions that are allocated on the calling task's stack
> and manipulated by RCU callbacks that are likely executing elsewhere.


KTSAN does not have any special logic for stacks. It just generally
flags pairs of accesses when (1) at least one access is not atomic,
(2) at least one access is a write and (3) these accesses are not
synchronized by means of other synchronization.
There is a bunch of cases when kernel code allocates objects on stack
and then passes them to other threads, but as far as there is proper
synchronization it is OK.

For the record, KTSAN is this:
https://github.com/google/ktsan
https://github.com/google/ktsan/wiki/Found-Bugs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246965 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 06:08:16PM +0200, Dmitry Vyukov wrote:
> >> > So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
> >> > prove safe address for KASAN's benefit, but READ_ONCE() suffices for
> >> > the data-race-detection logic in KTSAN, correct?
> >>
> >> KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
> >> between get_wchan() and the thread accesses to own stack even more
> >> aggressively than KASAN, because KTSAN won't like get_wchan() accesses
> >> even to non-poisoned areas of other thread stack.
> >
> > So to keep KTSAN happy, any read from some other thread's stack requires
> > READ_ONCE_NOCHECK()?  What if the access is via a locking primitive or
> > read-modify-write atomic operation?
> >
> > This is of some interest in RCU, which implements synchronous grace
> > periods using completions that are allocated on the calling task's stack
> > and manipulated by RCU callbacks that are likely executing elsewhere.
> 
> 
> KTSAN does not have any special logic for stacks. It just generally
> flags pairs of accesses when (1) at least one access is not atomic,
> (2) at least one access is a write and (3) these accesses are not
> synchronized by means of other synchronization.

But but but.. WRITE_ONCE/READ_ONCE _are_ atomic when on naturally
aligned machine word sized thingies. We very much rely on that.

And the wchan thing is very much that, its not some weird large object,
its a single word, read with an explicit 'volatile' cast.

This is good, and should not require more magic annotations.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246967 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 6:16 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Oct 14, 2015 at 06:08:16PM +0200, Dmitry Vyukov wrote:
>> >> > So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
>> >> > prove safe address for KASAN's benefit, but READ_ONCE() suffices for
>> >> > the data-race-detection logic in KTSAN, correct?
>> >>
>> >> KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
>> >> between get_wchan() and the thread accesses to own stack even more
>> >> aggressively than KASAN, because KTSAN won't like get_wchan() accesses
>> >> even to non-poisoned areas of other thread stack.
>> >
>> > So to keep KTSAN happy, any read from some other thread's stack requires
>> > READ_ONCE_NOCHECK()?  What if the access is via a locking primitive or
>> > read-modify-write atomic operation?
>> >
>> > This is of some interest in RCU, which implements synchronous grace
>> > periods using completions that are allocated on the calling task's stack
>> > and manipulated by RCU callbacks that are likely executing elsewhere.
>>
>>
>> KTSAN does not have any special logic for stacks. It just generally
>> flags pairs of accesses when (1) at least one access is not atomic,
>> (2) at least one access is a write and (3) these accesses are not
>> synchronized by means of other synchronization.
>
> But but but.. WRITE_ONCE/READ_ONCE _are_ atomic when on naturally
> aligned machine word sized thingies. We very much rely on that.
>
> And the wchan thing is very much that, its not some weird large object,
> its a single word, read with an explicit 'volatile' cast.
>
> This is good, and should not require more magic annotations.


Well, if another thread writes it byte-by-byte, it pretty much does
not matter how you read it.
Note that I said "at least one access is not atomic". If both are
atomic, then this is, of course, legal. And KTSAN considers
READ/WRITE_ONCE as atomic operations.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246968 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 9:20 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Oct 14, 2015 at 06:18:58PM +0200, Dmitry Vyukov wrote:
>>
>> Well, if another thread writes it byte-by-byte, it pretty much does
>> not matter how you read it.
>> Note that I said "at least one access is not atomic". If both are
>> atomic, then this is, of course, legal. And KTSAN considers
>> READ/WRITE_ONCE as atomic operations.
>
> OK, then I'm confused on what exactly the annotation does, but less
> worried.

The annotation says "hey, KASAN (etc), don't worry if you think that
the memory being accessed is out of bounds".  Presumably KTSAN is okay
with the operation because it's atomic, but KASAN dislikes it because
it's accessing memory that is out of bounds from the perspective of a
C program.

I'd still rather find a way to just delete get_wchan, but whatever.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246990 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 09:23:33AM -0700, Andy Lutomirski wrote:
> On Wed, Oct 14, 2015 at 9:20 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Wed, Oct 14, 2015 at 06:18:58PM +0200, Dmitry Vyukov wrote:
> >>
> >> Well, if another thread writes it byte-by-byte, it pretty much does
> >> not matter how you read it.
> >> Note that I said "at least one access is not atomic". If both are
> >> atomic, then this is, of course, legal. And KTSAN considers
> >> READ/WRITE_ONCE as atomic operations.
> >
> > OK, then I'm confused on what exactly the annotation does, but less
> > worried.
> 
> The annotation says "hey, KASAN (etc), don't worry if you think that
> the memory being accessed is out of bounds".  Presumably KTSAN is okay
> with the operation because it's atomic, but KASAN dislikes it because
> it's accessing memory that is out of bounds from the perspective of a
> C program.

There's going to be more of that..

> I'd still rather find a way to just delete get_wchan, but whatever.

:-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247034 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

* Peter Zijlstra <peterz@infradead.org> wrote:

> > I'd still rather find a way to just delete get_wchan, but whatever.
> 
> :-)

AFAICS can only do that at the price of slowing down various scheduler functions 
by saving the caller address.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247047 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 10:48 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
>> > I'd still rather find a way to just delete get_wchan, but whatever.
>>
>> :-)
>
> AFAICS can only do that at the price of slowing down various scheduler functions
> by saving the caller address.
>

A quick check on my machine:

# for i in /proc/*/wchan; do cat $i && echo ''; done |sort |uniq

doesn't have very much of interest to say:


0
devtmpfsd
dmcrypt_write
do_sigtimedwait
do_wait
ep_poll
fsnotify_mark_destroy
futex_wait_queue_me
hrtimer_nanosleep
irq_thread
kauditd_thread
khugepaged
kjournald2
ksm_scan_thread
kswapd
kthreadd
pipe_wait
poll_schedule_timeout
rcu_gp_kthread
rcu_nocb_kthread
rescuer_thread
scsi_error_handler
sk_wait_data
smpboot_thread_fn
unix_stream_recvmsg
wait_woken
worker_thread
xfsaild

A bunch of those look like they're specific to kernel threads, for
which this whole mechanism is probably pointless -- just reading
/proc/PID/stack (with suitable privilege) is probably better.

For the rest, a few are useful, but I find myself wondering whether
this mechanism is really useful enough to be worth keeping.  We could
just return "asleep" in /proc/PID/wchan.

A more interesting thing to do might be to try to decode regs->orig_ax
to give a guess as to which syscall is asleep.  But this seems like a
lot of fiddling and a lot of worry about security issues for a
mechanism of dubious value.  Also:

$ cat /proc/1/wchan
ep_poll

Why do we allow unprivileged queries like that at all?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246972 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 06:18:58PM +0200, Dmitry Vyukov wrote:
> 
> Well, if another thread writes it byte-by-byte, it pretty much does
> not matter how you read it.
> Note that I said "at least one access is not atomic". If both are
> atomic, then this is, of course, legal. And KTSAN considers
> READ/WRITE_ONCE as atomic operations.

OK, then I'm confused on what exactly the annotation does, but less
worried.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246991 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 6:20 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Oct 14, 2015 at 06:18:58PM +0200, Dmitry Vyukov wrote:
>>
>> Well, if another thread writes it byte-by-byte, it pretty much does
>> not matter how you read it.
>> Note that I said "at least one access is not atomic". If both are
>> atomic, then this is, of course, legal. And KTSAN considers
>> READ/WRITE_ONCE as atomic operations.
>
> OK, then I'm confused on what exactly the annotation does, but less
> worried.


The plan is to make READ_ONCE_NOCHECK ignored by KTSAN, just it is
ignored by KASAN. So that it never leads to a report ("not checked").
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247001 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 06:34:16PM +0200, Dmitry Vyukov wrote:
> On Wed, Oct 14, 2015 at 6:20 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Wed, Oct 14, 2015 at 06:18:58PM +0200, Dmitry Vyukov wrote:
> >>
> >> Well, if another thread writes it byte-by-byte, it pretty much does
> >> not matter how you read it.
> >> Note that I said "at least one access is not atomic". If both are
> >> atomic, then this is, of course, legal. And KTSAN considers
> >> READ/WRITE_ONCE as atomic operations.
> >
> > OK, then I'm confused on what exactly the annotation does, but less
> > worried.
> 
> The plan is to make READ_ONCE_NOCHECK ignored by KTSAN, just it is
> ignored by KASAN. So that it never leads to a report ("not checked").

Would a _NOKSAN suffix not be more appropriate? NOCHECK seems somewhat
generic.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246973 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 06:08:16PM +0200, Dmitry Vyukov wrote:
> On Wed, Oct 14, 2015 at 6:01 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Wed, Oct 14, 2015 at 05:50:34PM +0200, Dmitry Vyukov wrote:
> >> On Wed, Oct 14, 2015 at 5:45 PM, Paul E. McKenney
> >> <paulmck@linux.vnet.ibm.com> wrote:
> >> > On Wed, Oct 14, 2015 at 08:28:43AM -0700, tip-bot for Andrey Ryabinin wrote:
> >> >> Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
> >> >> Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
> >> >> Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
> >> >> AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
> >> >> Committer:  Ingo Molnar <mingo@kernel.org>
> >> >> CommitDate: Wed, 14 Oct 2015 16:44:06 +0200
> >> >>
> >> >> compiler, atomics: Provide READ_ONCE_NOCHECK()
> >> >>
> >> >> Some code may perform racy by design memory reads. This could be
> >> >> harmless, yet such code may produce KASAN warnings.
> >> >>
> >> >> To hide such accesses from KASAN this patch introduces
> >> >> READ_ONCE_NOCHECK() macro. KASAN will not check the memory
> >> >> accessed by READ_ONCE_NOCHECK().
> >> >>
> >> >> This patch creates __read_once_size_nocheck() a clone of
> >> >> __read_once_size_check() (renamed __read_once_size()).
> >> >> The only difference between them is 'no_sanitized_address'
> >> >> attribute appended to '*_nocheck' function. This attribute tells
> >> >> the compiler that instrumentation of memory accesses should not
> >> >> be applied to that function. We declare it as static
> >> >> '__maybe_unsed' because GCC is not capable to inline such
> >> >> function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> >> >>
> >> >> With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().
> >> >
> >> > So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
> >> > prove safe address for KASAN's benefit, but READ_ONCE() suffices for
> >> > the data-race-detection logic in KTSAN, correct?
> >>
> >> KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
> >> between get_wchan() and the thread accesses to own stack even more
> >> aggressively than KASAN, because KTSAN won't like get_wchan() accesses
> >> even to non-poisoned areas of other thread stack.
> >
> > So to keep KTSAN happy, any read from some other thread's stack requires
> > READ_ONCE_NOCHECK()?  What if the access is via a locking primitive or
> > read-modify-write atomic operation?
> >
> > This is of some interest in RCU, which implements synchronous grace
> > periods using completions that are allocated on the calling task's stack
> > and manipulated by RCU callbacks that are likely executing elsewhere.
> 
> KTSAN does not have any special logic for stacks. It just generally
> flags pairs of accesses when (1) at least one access is not atomic,
> (2) at least one access is a write and (3) these accesses are not
> synchronized by means of other synchronization.
> There is a bunch of cases when kernel code allocates objects on stack
> and then passes them to other threads, but as far as there is proper
> synchronization it is OK.

OK, so let me see if I understand this.  ;-)

KASAN requires READ_ONCE_NOCHECK() for get_wchan().  KTSAN would be
just as happy with READ_ONCE(), but READ_ONCE_NOCHECK() works for
both.

Did I get it right?

							Thanx, Paul

> For the record, KTSAN is this:
> https://github.com/google/ktsan
> https://github.com/google/ktsan/wiki/Found-Bugs
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246989 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 6:20 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Oct 14, 2015 at 06:08:16PM +0200, Dmitry Vyukov wrote:
>> On Wed, Oct 14, 2015 at 6:01 PM, Paul E. McKenney
>> <paulmck@linux.vnet.ibm.com> wrote:
>> > On Wed, Oct 14, 2015 at 05:50:34PM +0200, Dmitry Vyukov wrote:
>> >> On Wed, Oct 14, 2015 at 5:45 PM, Paul E. McKenney
>> >> <paulmck@linux.vnet.ibm.com> wrote:
>> >> > On Wed, Oct 14, 2015 at 08:28:43AM -0700, tip-bot for Andrey Ryabinin wrote:
>> >> >> Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> >> >> Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> >> >> Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
>> >> >> AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
>> >> >> Committer:  Ingo Molnar <mingo@kernel.org>
>> >> >> CommitDate: Wed, 14 Oct 2015 16:44:06 +0200
>> >> >>
>> >> >> compiler, atomics: Provide READ_ONCE_NOCHECK()
>> >> >>
>> >> >> Some code may perform racy by design memory reads. This could be
>> >> >> harmless, yet such code may produce KASAN warnings.
>> >> >>
>> >> >> To hide such accesses from KASAN this patch introduces
>> >> >> READ_ONCE_NOCHECK() macro. KASAN will not check the memory
>> >> >> accessed by READ_ONCE_NOCHECK().
>> >> >>
>> >> >> This patch creates __read_once_size_nocheck() a clone of
>> >> >> __read_once_size_check() (renamed __read_once_size()).
>> >> >> The only difference between them is 'no_sanitized_address'
>> >> >> attribute appended to '*_nocheck' function. This attribute tells
>> >> >> the compiler that instrumentation of memory accesses should not
>> >> >> be applied to that function. We declare it as static
>> >> >> '__maybe_unsed' because GCC is not capable to inline such
>> >> >> function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
>> >> >>
>> >> >> With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().
>> >> >
>> >> > So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
>> >> > prove safe address for KASAN's benefit, but READ_ONCE() suffices for
>> >> > the data-race-detection logic in KTSAN, correct?
>> >>
>> >> KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
>> >> between get_wchan() and the thread accesses to own stack even more
>> >> aggressively than KASAN, because KTSAN won't like get_wchan() accesses
>> >> even to non-poisoned areas of other thread stack.
>> >
>> > So to keep KTSAN happy, any read from some other thread's stack requires
>> > READ_ONCE_NOCHECK()?  What if the access is via a locking primitive or
>> > read-modify-write atomic operation?
>> >
>> > This is of some interest in RCU, which implements synchronous grace
>> > periods using completions that are allocated on the calling task's stack
>> > and manipulated by RCU callbacks that are likely executing elsewhere.
>>
>> KTSAN does not have any special logic for stacks. It just generally
>> flags pairs of accesses when (1) at least one access is not atomic,
>> (2) at least one access is a write and (3) these accesses are not
>> synchronized by means of other synchronization.
>> There is a bunch of cases when kernel code allocates objects on stack
>> and then passes them to other threads, but as far as there is proper
>> synchronization it is OK.
>
> OK, so let me see if I understand this.  ;-)
>
> KASAN requires READ_ONCE_NOCHECK() for get_wchan().  KTSAN would be
> just as happy with READ_ONCE(), but READ_ONCE_NOCHECK() works for
> both.
>
> Did I get it right?


No, KTSAN also needs READ_ONCE_NOCHECK.
READ_ONCE in get_wchan can lead to a data race report.
Consider:

// the other thead
some_stack_var = ...;

// get_wchan
bp = READ_ONCE(p); // where p happens to point to some_stack_var in
the other thread

This is generally not atomic and not safe. And this is a data race by
all possible definitions.
Only READ_ONCE on reading side is not enough to ensure atomicity, also
all concurrent writes must be done with atomic operations.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247010 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 06:32:58PM +0200, Dmitry Vyukov wrote:
> On Wed, Oct 14, 2015 at 6:20 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Wed, Oct 14, 2015 at 06:08:16PM +0200, Dmitry Vyukov wrote:
> >> On Wed, Oct 14, 2015 at 6:01 PM, Paul E. McKenney
> >> <paulmck@linux.vnet.ibm.com> wrote:
> >> > On Wed, Oct 14, 2015 at 05:50:34PM +0200, Dmitry Vyukov wrote:
> >> >> On Wed, Oct 14, 2015 at 5:45 PM, Paul E. McKenney
> >> >> <paulmck@linux.vnet.ibm.com> wrote:
> >> >> > On Wed, Oct 14, 2015 at 08:28:43AM -0700, tip-bot for Andrey Ryabinin wrote:
> >> >> >> Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
> >> >> >> Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
> >> >> >> Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
> >> >> >> AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
> >> >> >> Committer:  Ingo Molnar <mingo@kernel.org>
> >> >> >> CommitDate: Wed, 14 Oct 2015 16:44:06 +0200
> >> >> >>
> >> >> >> compiler, atomics: Provide READ_ONCE_NOCHECK()
> >> >> >>
> >> >> >> Some code may perform racy by design memory reads. This could be
> >> >> >> harmless, yet such code may produce KASAN warnings.
> >> >> >>
> >> >> >> To hide such accesses from KASAN this patch introduces
> >> >> >> READ_ONCE_NOCHECK() macro. KASAN will not check the memory
> >> >> >> accessed by READ_ONCE_NOCHECK().
> >> >> >>
> >> >> >> This patch creates __read_once_size_nocheck() a clone of
> >> >> >> __read_once_size_check() (renamed __read_once_size()).
> >> >> >> The only difference between them is 'no_sanitized_address'
> >> >> >> attribute appended to '*_nocheck' function. This attribute tells
> >> >> >> the compiler that instrumentation of memory accesses should not
> >> >> >> be applied to that function. We declare it as static
> >> >> >> '__maybe_unsed' because GCC is not capable to inline such
> >> >> >> function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> >> >> >>
> >> >> >> With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().
> >> >> >
> >> >> > So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
> >> >> > prove safe address for KASAN's benefit, but READ_ONCE() suffices for
> >> >> > the data-race-detection logic in KTSAN, correct?
> >> >>
> >> >> KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
> >> >> between get_wchan() and the thread accesses to own stack even more
> >> >> aggressively than KASAN, because KTSAN won't like get_wchan() accesses
> >> >> even to non-poisoned areas of other thread stack.
> >> >
> >> > So to keep KTSAN happy, any read from some other thread's stack requires
> >> > READ_ONCE_NOCHECK()?  What if the access is via a locking primitive or
> >> > read-modify-write atomic operation?
> >> >
> >> > This is of some interest in RCU, which implements synchronous grace
> >> > periods using completions that are allocated on the calling task's stack
> >> > and manipulated by RCU callbacks that are likely executing elsewhere.
> >>
> >> KTSAN does not have any special logic for stacks. It just generally
> >> flags pairs of accesses when (1) at least one access is not atomic,
> >> (2) at least one access is a write and (3) these accesses are not
> >> synchronized by means of other synchronization.
> >> There is a bunch of cases when kernel code allocates objects on stack
> >> and then passes them to other threads, but as far as there is proper
> >> synchronization it is OK.
> >
> > OK, so let me see if I understand this.  ;-)
> >
> > KASAN requires READ_ONCE_NOCHECK() for get_wchan().  KTSAN would be
> > just as happy with READ_ONCE(), but READ_ONCE_NOCHECK() works for
> > both.
> >
> > Did I get it right?
> 
> 
> No, KTSAN also needs READ_ONCE_NOCHECK.
> READ_ONCE in get_wchan can lead to a data race report.
> Consider:
> 
> // the other thead
> some_stack_var = ...;
> 
> // get_wchan
> bp = READ_ONCE(p); // where p happens to point to some_stack_var in
> the other thread
> 
> This is generally not atomic and not safe. And this is a data race by
> all possible definitions.
> Only READ_ONCE on reading side is not enough to ensure atomicity, also
> all concurrent writes must be done with atomic operations.

OK.  However, this is specific to get_wchan()'s out-of-bounds stack, right?
If I have multiple tasks accessing some other task's on-stack variable,
then as long as all other potentially concurrent accesses use atomic
operations, READ_ONCE() suffices.  Or am I still missing something here?

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247020 — Re: [tip:locking/urgent] compiler, atomics: Provide READ_ONCE_NOCHECK ()

On Wed, Oct 14, 2015 at 7:04 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
>> >> >> > On Wed, Oct 14, 2015 at 08:28:43AM -0700, tip-bot for Andrey Ryabinin wrote:
>> >> >> >> Commit-ID:  4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> >> >> >> Gitweb:     http://git.kernel.org/tip/4115ffdf4d6f8986a7abe1dd522c163f599bc0e6
>> >> >> >> Author:     Andrey Ryabinin <aryabinin@virtuozzo.com>
>> >> >> >> AuthorDate: Tue, 13 Oct 2015 18:28:07 +0300
>> >> >> >> Committer:  Ingo Molnar <mingo@kernel.org>
>> >> >> >> CommitDate: Wed, 14 Oct 2015 16:44:06 +0200
>> >> >> >>
>> >> >> >> compiler, atomics: Provide READ_ONCE_NOCHECK()
>> >> >> >>
>> >> >> >> Some code may perform racy by design memory reads. This could be
>> >> >> >> harmless, yet such code may produce KASAN warnings.
>> >> >> >>
>> >> >> >> To hide such accesses from KASAN this patch introduces
>> >> >> >> READ_ONCE_NOCHECK() macro. KASAN will not check the memory
>> >> >> >> accessed by READ_ONCE_NOCHECK().
>> >> >> >>
>> >> >> >> This patch creates __read_once_size_nocheck() a clone of
>> >> >> >> __read_once_size_check() (renamed __read_once_size()).
>> >> >> >> The only difference between them is 'no_sanitized_address'
>> >> >> >> attribute appended to '*_nocheck' function. This attribute tells
>> >> >> >> the compiler that instrumentation of memory accesses should not
>> >> >> >> be applied to that function. We declare it as static
>> >> >> >> '__maybe_unsed' because GCC is not capable to inline such
>> >> >> >> function: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
>> >> >> >>
>> >> >> >> With KASAN=n READ_ONCE_NOCHECK() is just a clone of READ_ONCE().
>> >> >> >
>> >> >> > So I add READ_ONCE_NOCHECK() for accesses for which the compiler cannot
>> >> >> > prove safe address for KASAN's benefit, but READ_ONCE() suffices for
>> >> >> > the data-race-detection logic in KTSAN, correct?
>> >> >>
>> >> >> KTSAN also needs READ_ONCE_NOCHECK() here. KTSAN will flag races
>> >> >> between get_wchan() and the thread accesses to own stack even more
>> >> >> aggressively than KASAN, because KTSAN won't like get_wchan() accesses
>> >> >> even to non-poisoned areas of other thread stack.
>> >> >
>> >> > So to keep KTSAN happy, any read from some other thread's stack requires
>> >> > READ_ONCE_NOCHECK()?  What if the access is via a locking primitive or
>> >> > read-modify-write atomic operation?
>> >> >
>> >> > This is of some interest in RCU, which implements synchronous grace
>> >> > periods using completions that are allocated on the calling task's stack
>> >> > and manipulated by RCU callbacks that are likely executing elsewhere.
>> >>
>> >> KTSAN does not have any special logic for stacks. It just generally
>> >> flags pairs of accesses when (1) at least one access is not atomic,
>> >> (2) at least one access is a write and (3) these accesses are not
>> >> synchronized by means of other synchronization.
>> >> There is a bunch of cases when kernel code allocates objects on stack
>> >> and then passes them to other threads, but as far as there is proper
>> >> synchronization it is OK.
>> >
>> > OK, so let me see if I understand this.  ;-)
>> >
>> > KASAN requires READ_ONCE_NOCHECK() for get_wchan().  KTSAN would be
>> > just as happy with READ_ONCE(), but READ_ONCE_NOCHECK() works for
>> > both.
>> >
>> > Did I get it right?
>>
>>
>> No, KTSAN also needs READ_ONCE_NOCHECK.
>> READ_ONCE in get_wchan can lead to a data race report.
>> Consider:
>>
>> // the other thead
>> some_stack_var = ...;
>>
>> // get_wchan
>> bp = READ_ONCE(p); // where p happens to point to some_stack_var in
>> the other thread
>>
>> This is generally not atomic and not safe. And this is a data race by
>> all possible definitions.
>> Only READ_ONCE on reading side is not enough to ensure atomicity, also
>> all concurrent writes must be done with atomic operations.
>
> OK.  However, this is specific to get_wchan()'s out-of-bounds stack, right?

Yes... and no.
This is specific to racy accesses, not necessary to out-of-bounds
stack. If you have a data race in-bounds, it is also not OK :)


> If I have multiple tasks accessing some other task's on-stack variable,
> then as long as all other potentially concurrent accesses use atomic
> operations, READ_ONCE() suffices.  Or am I still missing something here?

This is correct.
Generally, the idea is that KTSAN flags what you think is a bug, and
does not flag what you think is not a bug. If you have all proper
synchronization in place, then KTSAN should not flag that; whether the
object is on stack or not is irrelevant.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web