[tip:locking/core] locking/barriers: Validate lockless_dereference() is used on a pointer type
Commit-ID: 331b6d8c7afc2e5b900b9dcd850c265e1ba8d8e7
Gitweb: http://git.kernel.org/tip/331b6d8c7afc2e5b900b9dcd850c265e1ba8d8e7
Author: Peter Zijlstra
AuthorDate: Sun, 22 May 2016 12:48:27 +0200
Committer: Ingo Molnar
CommitDate: Wed, 8 Jun 2016 14:22:47 +0200
locking/barriers: Validate lockless_dereference() is used on a pointer type
Use the type to validate the argument @p is indeed a pointer type.
Signed-off-by: Peter Zijlstra (Intel)
Cc: Alexey Dobriyan
Cc: Andrew Morton
Cc: Linus Torvalds
Cc: Paul E. McKenney
Cc: Paul McKenney
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Link:
http://lkml.kernel.org/r/20160522104827.gp3...@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar
---
include/linux/compiler.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 793c082..06f27fd 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -545,10 +545,14 @@ static __always_inline void __write_once_size(volatile
void *p, void *res, int s
* Similar to rcu_dereference(), but for situations where the pointed-to
* object's lifetime is managed by something other than RCU. That
* "something other" might be reference counting or simple immortality.
+ *
+ * The seemingly unused void * variable is to validate @p is indeed a pointer
+ * type. All pointer types silently cast to void *.
*/
#define lockless_dereference(p) \
({ \
typeof(p) _p1 = READ_ONCE(p); \
+ __maybe_unused const void * const _p2 = _p1; \
smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
(_p1); \
})
Re: [tip:locking/core] locking/barriers: Validate lockless_dereference() is used on a pointer type
On Mon, Jun 06, 2016 at 11:31:39PM +0200, Mateusz Guzik wrote:
> The reason is that leaf_walk_rcu does get_child_rcu(pn, cindex++), which
> ends up in lockless_dereference as pn[cindex++], which is now evaluated
> twice.
Indeed, bad that; yes I think the below will work, will you send a
proper patch so I can apply?
> The simplest fix I see does the assignment later, that is:
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index e9c6417..06f27fd 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -551,8 +551,8 @@ static __always_inline void __write_once_size(volatile
> void *p, void *res, int s
> */
> #define lockless_dereference(p) \
> ({ \
> - __maybe_unused const void * const _p2 = p; \
> typeof(p) _p1 = READ_ONCE(p); \
> + __maybe_unused const void * const _p2 = _p1; \
> smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
> (_p1); \
> })
>
> --
> Mateusz Guzik
Re: [tip:locking/core] locking/barriers: Validate lockless_dereference() is used on a pointer type
On Fri, Jun 03, 2016 at 03:58:09AM -0700, tip-bot for Peter Zijlstra wrote:
> Commit-ID: 25841ee0e9d2a1d952828138416701f20ea831eb
> Gitweb: http://git.kernel.org/tip/25841ee0e9d2a1d952828138416701f20ea831eb
> Author: Peter Zijlstra
> AuthorDate: Sun, 22 May 2016 12:48:27 +0200
> Committer: Ingo Molnar
> CommitDate: Fri, 3 Jun 2016 12:06:11 +0200
>
> locking/barriers: Validate lockless_dereference() is used on a pointer type
>
> Add a cast to void * to validate the argument @p is indeed a pointer.
>
> Signed-off-by: Peter Zijlstra (Intel)
> Cc: Alexey Dobriyan
> Cc: Andrew Morton
> Cc: Linus Torvalds
> Cc: Paul E. McKenney
> Cc: Paul McKenney
> Cc: Peter Zijlstra
> Cc: Thomas Gleixner
> Link:
> http://lkml.kernel.org/r/20160522104827.gp3...@twins.programming.kicks-ass.net
> Signed-off-by: Ingo Molnar
> ---
> include/linux/compiler.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 793c082..e9c6417 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -545,9 +545,13 @@ static __always_inline void __write_once_size(volatile
> void *p, void *res, int s
> * Similar to rcu_dereference(), but for situations where the pointed-to
> * object's lifetime is managed by something other than RCU. That
> * "something other" might be reference counting or simple immortality.
> + *
> + * The seemingly unused void * variable is to validate @p is indeed a pointer
> + * type. All pointer types silently cast to void *.
> */
> #define lockless_dereference(p) \
> ({ \
> + __maybe_unused const void * const _p2 = p; \
> typeof(p) _p1 = READ_ONCE(p); \
> smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
> (_p1); \
This causes issues, e.g.:
BUG: KASAN: user-memory-access on address 60ff95001931^M
Read of size 1 by task NetworkManager/897^M
CPU: 1 PID: 897 Comm: NetworkManager Not tainted 4.7.0-rc1dupa+ #355^M
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011^M
88003a1f8040 17d08b23 880033aff4b0 8187b2f8^M
88005b44f2c8 880033aff548 880033aff538 813506a6^M
81167e5esystemd[1]: sshd.service: Forked /usr/sbin/sshd as 904^M
^M
ed000675fe9d 0286 880033aff588^M
Call Trace:^M
[] dump_stack+0x85/0xcd^M
[] kasan_report_error+0x456/0x560^M
[] ? vprintk_default+0x3e/0x60^M
[] ? printk+0xa8/0xd8^M
[] ? power_down+0xa9/0xa9^M
[] kasan_report+0x58/0x60^M
[] ? leaf_walk_rcu+0x235/0x2d0^M
[] __asan_load1+0x47/0x50^M
[] leaf_walk_rcu+0x235/0x2d0^M
[snip]
The reason is that leaf_walk_rcu does get_child_rcu(pn, cindex++), which
ends up in lockless_dereference as pn[cindex++], which is now evaluated
twice.
The simplest fix I see does the assignment later, that is:
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index e9c6417..06f27fd 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -551,8 +551,8 @@ static __always_inline void __write_once_size(volatile void
*p, void *res, int s
*/
#define lockless_dereference(p) \
({ \
- __maybe_unused const void * const _p2 = p; \
typeof(p) _p1 = READ_ONCE(p); \
+ __maybe_unused const void * const _p2 = _p1; \
smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
(_p1); \
})
--
Mateusz Guzik
[tip:locking/core] locking/barriers: Validate lockless_dereference() is used on a pointer type
Commit-ID: 25841ee0e9d2a1d952828138416701f20ea831eb
Gitweb: http://git.kernel.org/tip/25841ee0e9d2a1d952828138416701f20ea831eb
Author: Peter Zijlstra
AuthorDate: Sun, 22 May 2016 12:48:27 +0200
Committer: Ingo Molnar
CommitDate: Fri, 3 Jun 2016 12:06:11 +0200
locking/barriers: Validate lockless_dereference() is used on a pointer type
Add a cast to void * to validate the argument @p is indeed a pointer.
Signed-off-by: Peter Zijlstra (Intel)
Cc: Alexey Dobriyan
Cc: Andrew Morton
Cc: Linus Torvalds
Cc: Paul E. McKenney
Cc: Paul McKenney
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Link:
http://lkml.kernel.org/r/20160522104827.gp3...@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar
---
include/linux/compiler.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 793c082..e9c6417 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -545,9 +545,13 @@ static __always_inline void __write_once_size(volatile
void *p, void *res, int s
* Similar to rcu_dereference(), but for situations where the pointed-to
* object's lifetime is managed by something other than RCU. That
* "something other" might be reference counting or simple immortality.
+ *
+ * The seemingly unused void * variable is to validate @p is indeed a pointer
+ * type. All pointer types silently cast to void *.
*/
#define lockless_dereference(p) \
({ \
+ __maybe_unused const void * const _p2 = p; \
typeof(p) _p1 = READ_ONCE(p); \
smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
(_p1); \
