[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