linux-kernel - Re: [RFC PATCH] rcu: rcupdate.h: Add missing parentheses around macro pointer dereference

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: <CAEXW_YRkPrwL5uGthkmaBaxn9b4tcWDofdcTE=Lu6=yXeDNpWg@mail.gmail.com>
Date:   Wed, 3 May 2023 21:17:13 -0400
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     "Paul E. McKenney" <paulmck@...nel.org>,
        linux-kernel@...r.kernel.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: Re: [RFC PATCH] rcu: rcupdate.h: Add missing parentheses around macro
 pointer dereference

On Wed, May 3, 2023 at 4:32 PM Mathieu Desnoyers
<mathieu.desnoyers@...icios.com> wrote:
>
> 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 */
>     }
>
> 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>

Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org>

thanks,

 - Joel


> ---
>  include/linux/rcupdate.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index dcd2cf1e8326..1565012fa47f 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -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))
>
> --
> 2.25.1
>