[Devel] [PATCH RHEL7 COMMIT] rcuupdate.h: Switch to READ_ONCE and fix NULL-deref in __task_pid_nr_ns() again

Konstantin Khorenko khorenko at virtuozzo.com
Fri Jul 13 15:29:41 MSK 2018


The commit is pushed to "branch-rh7-3.10.0-862.6.3.vz7.62.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.6.3.vz7.62.3
------>
commit 1a66691dfc972ff0a1ee32909336355eb4875964
Author: Andrey Ryabinin <aryabinin at virtuozzo.com>
Date:   Fri Jul 13 15:29:41 2018 +0300

    rcuupdate.h: Switch to READ_ONCE and fix NULL-deref in __task_pid_nr_ns() again
    
    In upstream ACCESS_ONCE() was deprecated in favor of READ_ONCE()/WRITE_ONCE()
    because it has problems with gcc 4.6/4.7. The GCC bug fixed, however
    it looks like it resurrected in some form in recent GCC version.
    
    On the kernel, compiled with gcc 7.3.0 or gcc-8.1.0 i'm hitting
    NULL-ptr in __task_pid_nr_ns() which is supposed to be fixed by
    the upstream commit 81b1a832d797 ("fix NULL dereference in __task_pid_nr_ns()")
    
    It appears that rcu_dereference() in __task_pid_nr_ns() somehow doesn't work
    properly. task->pids[type].pid loaded 2 times, the first one to check for NULL
    and the second load to get the 'pid->level'.
    
    Replacing ACCESS_ONCE() with READ_ONCE() in __rcu_access_pointer() magically
    fix things for me. So let's do that.
    
    Note: our release kernel seems not affected by this, because of different gcc
    version.
    
    Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
---
 include/linux/rcupdate.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 981261775a41..08370a755067 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -537,13 +537,13 @@ static inline void rcu_preempt_sleep_check(void)
 
 #define __rcu_access_pointer(p, space) \
 	({ \
-		typeof(*p) *_________p1 = (typeof(*p)*__force )ACCESS_ONCE(p); \
+		typeof(*p) *_________p1 = (typeof(*p)*__force )READ_ONCE(p); \
 		rcu_dereference_sparse(p, space); \
 		((typeof(*p) __force __kernel *)(_________p1)); \
 	})
 #define __rcu_dereference_check(p, c, space) \
 	({ \
-		typeof(*p) *_________p1 = (typeof(*p)*__force )ACCESS_ONCE(p); \
+		typeof(*p) *_________p1 = (typeof(*p)*__force )READ_ONCE(p); \
 		rcu_lockdep_assert(c, "suspicious rcu_dereference_check()" \
 				      " usage"); \
 		rcu_dereference_sparse(p, space); \
@@ -560,13 +560,13 @@ static inline void rcu_preempt_sleep_check(void)
 
 #define __rcu_access_index(p, space) \
 	({ \
-		typeof(p) _________p1 = ACCESS_ONCE(p); \
+		typeof(p) _________p1 = READ_ONCE(p); \
 		rcu_dereference_sparse(p, space); \
 		(_________p1); \
 	})
 #define __rcu_dereference_index_check(p, c) \
 	({ \
-		typeof(p) _________p1 = ACCESS_ONCE(p); \
+		typeof(p) _________p1 = READ_ONCE(p); \
 		rcu_lockdep_assert(c, \
 				   "suspicious rcu_dereference_index_check()" \
 				   " usage"); \
@@ -584,7 +584,7 @@ static inline void rcu_preempt_sleep_check(void)
  * @p: The pointer to read
  *
  * Return the value of the specified RCU-protected pointer, but omit the
- * smp_read_barrier_depends() and keep the ACCESS_ONCE().  This is useful
+ * smp_read_barrier_depends() and keep the READ_ONCE().  This is useful
  * when the value of this pointer is accessed, but the pointer is not
  * dereferenced, for example, when testing an RCU-protected pointer against
  * NULL.  Although rcu_access_pointer() may also be used in cases where
@@ -673,7 +673,7 @@ static inline void rcu_preempt_sleep_check(void)
  * @p: The index to read
  *
  * Return the value of the specified RCU-protected index, but omit the
- * smp_read_barrier_depends() and keep the ACCESS_ONCE().  This is useful
+ * smp_read_barrier_depends() and keep the READ_ONCE().  This is useful
  * when the value of this index is accessed, but the index is not
  * dereferenced, for example, when testing an RCU-protected index against
  * -1.  Although rcu_access_index() may also be used in cases where
@@ -709,7 +709,7 @@ static inline void rcu_preempt_sleep_check(void)
  * @c: The conditions under which the dereference will take place
  *
  * Return the value of the specified RCU-protected pointer, but omit
- * both the smp_read_barrier_depends() and the ACCESS_ONCE().  This
+ * both the smp_read_barrier_depends() and the READ_ONCE().  This
  * is useful in cases where update-side locks prevent the value of the
  * pointer from changing.  Please note that this primitive does -not-
  * prevent the compiler from repeating this reference or combining it


More information about the Devel mailing list