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: <20230504200527.1935944-2-mathieu.desnoyers@efficios.com>
Date:   Thu,  4 May 2023 16:05:15 -0400
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Josh Triplett <josh@...htriplett.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Zqiang <qiang1.zhang@...el.com>
Subject: [RFC PATCH 01/13] rcu: rcupdate.h: Fix parentheses around macro parameter use

linux/rcupdate.h macros use the *p parameter without parentheses, e.g.:

  typeof(*p)

rather than

  typeof(*(p))

The following test-case shows how it can generate confusion due to C
operator precedence being reversed compared to the expectations:

    #define m(p) \
    do { \
            __typeof__(*p) v = 0; \
    } while (0)

    void fct(unsigned long long *p1)
    {
            m(p1 + 1);      /* works */
            m(1 + p1);      /* broken */
    }

Add missing parentheses around macro parameter use in the following
pattern to ensure operator precedence behaves as expected:

- m(&p) is changed for m(&(p)).

Remove useless parentheses in the following macro argument usage
patterns:

- m((x), y) is changed for m(x, y), because comma has the lowest
  operator precedence, and thus the extra parentheses are useless.
- "(_r_a_p__v) where _r_a_p__v is a variable and not a macro parameter,
  is changed for "_r_a_p__v",

An extra whitespace is also removed in kvfree_rcu_arg_2() for
consistency.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: "Paul E. McKenney" <paulmck@...nel.org>
Cc: Joel Fernandes <joel@...lfernandes.org>
Cc: Josh Triplett <josh@...htriplett.org>
Cc: Boqun Feng <boqun.feng@...il.com>
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Lai Jiangshan <jiangshanlai@...il.com>
Cc: Zqiang <qiang1.zhang@...el.com>
---
 include/linux/rcupdate.h | 46 ++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index dcd2cf1e8326..4f26c3080953 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -215,7 +215,7 @@ void rcu_tasks_trace_qs_blkd(struct task_struct *t);
 										\
 		if (likely(!READ_ONCE((t)->trc_reader_special.b.need_qs)) &&	\
 		    likely(!___rttq_nesting)) {					\
-			rcu_trc_cmpxchg_need_qs((t), 0,	TRC_NEED_QS_CHECKED);	\
+			rcu_trc_cmpxchg_need_qs(t, 0, TRC_NEED_QS_CHECKED);	\
 		} else if (___rttq_nesting && ___rttq_nesting != INT_MIN &&	\
 			   !READ_ONCE((t)->trc_reader_special.b.blocked)) {	\
 			rcu_tasks_trace_qs_blkd(t);				\
@@ -227,7 +227,7 @@ void rcu_tasks_trace_qs_blkd(struct task_struct *t);
 
 #define rcu_tasks_qs(t, preempt)					\
 do {									\
-	rcu_tasks_classic_qs((t), (preempt));				\
+	rcu_tasks_classic_qs(t, preempt);				\
 	rcu_tasks_trace_qs(t);						\
 } while (0)
 
@@ -430,16 +430,16 @@ static inline void rcu_preempt_sleep_check(void) { }
 
 #ifdef __CHECKER__
 #define rcu_check_sparse(p, space) \
-	((void)(((typeof(*p) space *)p) == p))
+	((void)(((typeof(*(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);			\
+	typeof(*(p)) *local = (typeof(*(p)) *__force)(p);		\
 	rcu_check_sparse(p, __rcu);					\
-	((typeof(*p) __force __kernel *)(local)); 			\
+	((typeof(*(p)) __force __kernel *)(local));			\
 })
 /**
  * unrcu_pointer - mark a pointer as not being RCU protected
@@ -452,29 +452,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); \
+	typeof(*(p)) *local = (typeof(*(p)) *__force)READ_ONCE(p); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((typeof(*(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); \
+	typeof(*(p)) *local = (typeof(*(p)) *__force)READ_ONCE(p); \
 	RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((typeof(*(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)); \
+	((typeof(*(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)); \
+	((typeof(*(p)) __force __kernel *)(local)); \
 })
 #define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
 
@@ -520,10 +520,10 @@ do {									      \
 	uintptr_t _r_a_p__v = (uintptr_t)(v);				      \
 	rcu_check_sparse(p, __rcu);					      \
 									      \
-	if (__builtin_constant_p(v) && (_r_a_p__v) == (uintptr_t)NULL)	      \
-		WRITE_ONCE((p), (typeof(p))(_r_a_p__v));		      \
+	if (__builtin_constant_p(v) && _r_a_p__v == (uintptr_t)NULL)	      \
+		WRITE_ONCE(p, (typeof(p))_r_a_p__v);			      \
 	else								      \
-		smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
+		smp_store_release(&(p), RCU_INITIALIZER((typeof(p))_r_a_p__v));\
 } while (0)
 
 /**
@@ -539,8 +539,8 @@ do {									      \
  */
 #define rcu_replace_pointer(rcu_ptr, ptr, c)				\
 ({									\
-	typeof(ptr) __tmp = rcu_dereference_protected((rcu_ptr), (c));	\
-	rcu_assign_pointer((rcu_ptr), (ptr));				\
+	typeof(ptr) __tmp = rcu_dereference_protected(rcu_ptr, c);	\
+	rcu_assign_pointer(rcu_ptr, ptr);				\
 	__tmp;								\
 })
 
@@ -571,7 +571,7 @@ do {									      \
  * down multi-linked structures after a grace period has elapsed.  However,
  * rcu_dereference_protected() is normally preferred for this use case.
  */
-#define rcu_access_pointer(p) __rcu_access_pointer((p), __UNIQUE_ID(rcu), __rcu)
+#define rcu_access_pointer(p) __rcu_access_pointer(p, __UNIQUE_ID(rcu), __rcu)
 
 /**
  * rcu_dereference_check() - rcu_dereference with debug checking
@@ -607,7 +607,7 @@ do {									      \
  * annotated as __rcu.
  */
 #define rcu_dereference_check(p, c) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), \
 				(c) || rcu_read_lock_held(), __rcu)
 
 /**
@@ -623,7 +623,7 @@ do {									      \
  * rcu_read_lock() but also rcu_read_lock_bh() into account.
  */
 #define rcu_dereference_bh_check(p, c) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), \
 				(c) || rcu_read_lock_bh_held(), __rcu)
 
 /**
@@ -639,7 +639,7 @@ do {									      \
  * only rcu_read_lock() but also rcu_read_lock_sched() into account.
  */
 #define rcu_dereference_sched_check(p, c) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), \
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), \
 				(c) || rcu_read_lock_sched_held(), \
 				__rcu)
 
@@ -651,7 +651,7 @@ do {									      \
  * rcu_read_lock_held().
  */
 #define rcu_dereference_raw_check(p) \
-	__rcu_dereference_check((p), __UNIQUE_ID(rcu), 1, __rcu)
+	__rcu_dereference_check(p, __UNIQUE_ID(rcu), 1, __rcu)
 
 /**
  * rcu_dereference_protected() - fetch RCU pointer when updates prevented
@@ -670,7 +670,7 @@ do {									      \
  * but very ugly failures.
  */
 #define rcu_dereference_protected(p, c) \
-	__rcu_dereference_protected((p), __UNIQUE_ID(rcu), (c), __rcu)
+	__rcu_dereference_protected(p, __UNIQUE_ID(rcu), c, __rcu)
 
 
 /**
@@ -1021,7 +1021,7 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
 #define KVFREE_GET_MACRO(_1, _2, NAME, ...) NAME
 #define kvfree_rcu_arg_2(ptr, rhf)					\
 do {									\
-	typeof (ptr) ___p = (ptr);					\
+	typeof(ptr) ___p = (ptr);					\
 									\
 	if (___p) {									\
 		BUILD_BUG_ON(!__is_kvfree_rcu_offset(offsetof(typeof(*(ptr)), rhf)));	\
-- 
2.25.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ