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]
Message-Id: <20220628194812.1453059-38-alexandr.lobakin@intel.com>
Date:   Tue, 28 Jun 2022 21:47:57 +0200
From:   Alexander Lobakin <alexandr.lobakin@...el.com>
To:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>
Cc:     Alexander Lobakin <alexandr.lobakin@...el.com>,
        Larysa Zaremba <larysa.zaremba@...el.com>,
        Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Björn Töpel <bjorn@...nel.org>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Toke Hoiland-Jorgensen <toke@...hat.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        John Fastabend <john.fastabend@...il.com>,
        Yajun Deng <yajun.deng@...ux.dev>,
        Willem de Bruijn <willemb@...gle.com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        xdp-hints@...-project.net
Subject: [PATCH RFC bpf-next 37/52] rcupdate: fix access helpers for incomplete struct pointers on GCC < 10

It's been found that currently it is impossible to use RCU for
incomplete struct pointers.
RCU access helpers have the following construct:

	typeof(*p) *local = ...

GCC versions older than 10 don't look at the whole sentence and
believe that there's a dereference happening inside the typeof(),
although it does not.
As RCU doesn't imply any dereference, but only the way to store and
access pointers, this is not a valid case. Moreover, Clang and GCC
10 onwards evaluate it with no issues.
Fix this by introducing a new macro, __rcutype(), which will take
care of pointer annotations inside the RCU access helpers, in two
different ways depending on the compiler used. For sane compilers,
leave it as it is for now, as it ensures that the passed argument
is a pointer, and for the affected ones use...
`typeof(0 ? (p) : (p))`. As:

void fc(void) { }

...
	pr_info("%d", __builtin_types_compatible(typeof(*fn) *, typeof(fn)));
	pr_info("%d", __builtin_types_compatible(typeof(*fn) *, typeof(&fn)));
	pr_info("%d", __builtin_types_compatible(typeof(*fn) *,
						 typeof(0 ? (fn) : (fn)));

emits:

011

and we can't use the second for non-functions.

Fixes: ca5ecddfa8fc ("rcu: define __rcu address space modifier for sparse")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@...el.com>
---
 include/linux/rcupdate.h | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 1a32036c918c..f5971fccf852 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -358,18 +358,33 @@ static inline void rcu_preempt_sleep_check(void) { }
  * (e.g., __srcu), should this make sense in the future.
  */
 
+/*
+ * Unfortunately, GCC versions older than 10 don't look at the whole sentence
+ * and treat `typeof(*(p)) *` as dereferencing although it is not. This makes
+ * it impossible to use those helpers with pointers to incomplete structures.
+ * Plain `typeof(p)` is not the same, as `typeof(func)` returns the type of a
+ * function, not a pointer to it, as `typeof(*(func)) *` does.
+ * `typeof(<anything> ? (func) : (func))` is silly; however, it works just as
+ * the original definition.
+ */
+#if defined(CONFIG_CC_IS_GCC) && CONFIG_GCC_VERSION < 100000
+#define __rcutype(p, ...)	typeof(0 ? (p) : (p)) __VA_ARGS__
+#else
+#define __rcutype(p, ...)	typeof(*(p)) __VA_ARGS__ *
+#endif
+
 #ifdef __CHECKER__
 #define rcu_check_sparse(p, space) \
-	((void)(((typeof(*p) space *)p) == p))
+	((void)((__rcutype(p, space))(p) == (p)))
 #else /* #ifdef __CHECKER__ */
 #define rcu_check_sparse(p, space)
 #endif /* #else #ifdef __CHECKER__ */
 
 #define __unrcu_pointer(p, local)					\
 ({									\
-	typeof(*p) *local = (typeof(*p) *__force)(p);			\
+	__rcutype(p) local = (__rcutype(p, __force))(p);		\
 	rcu_check_sparse(p, __rcu);					\
-	((typeof(*p) __force __kernel *)(local)); 			\
+	((__rcutype(p, __force __kernel))(local)); 			\
 })
 /**
  * unrcu_pointer - mark a pointer as not being RCU protected
@@ -382,29 +397,29 @@ static inline void rcu_preempt_sleep_check(void) { }
 
 #define __rcu_access_pointer(p, local, space) \
 ({ \
-	typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
+	__rcutype(p) local = (__rcutype(p, __force))READ_ONCE(p); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((__rcutype(p, __force __kernel))(local)); \
 })
 #define __rcu_dereference_check(p, local, c, space) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
+	__rcutype(p) local = (__rcutype(p, __force))READ_ONCE(p); \
 	RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((__rcutype(p, __force __kernel))(local)); \
 })
 #define __rcu_dereference_protected(p, local, c, space) \
 ({ \
 	RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(p)); \
+	((__rcutype(p, __force __kernel))(p)); \
 })
 #define __rcu_dereference_raw(p, local) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(p) local = READ_ONCE(p); \
-	((typeof(*p) __force __kernel *)(local)); \
+	__rcutype(p) local = READ_ONCE(p); \
+	((__rcutype(p, __force __kernel))(local)); \
 })
 #define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
 
@@ -412,7 +427,7 @@ static inline void rcu_preempt_sleep_check(void) { }
  * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
  * @v: The value to statically initialize with.
  */
-#define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
+#define RCU_INITIALIZER(v) (__rcutype(v, __force __rcu))(v)
 
 /**
  * rcu_assign_pointer() - assign to RCU-protected pointer
-- 
2.36.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ