Jann Horn identified a racy access to p->mm in the global expedited
command of the membarrier system call.

The suggested fix is to hold the task_lock() around the accesses to
p->mm and to the mm_struct membarrier_state field to guarantee the
existence of the mm_struct.

Link: 
https://lore.kernel.org/lkml/cag48ez2g8ctf8dhs42tf37pthfr3y0rnooytmxvacm4u8yu...@mail.gmail.com
Signed-off-by: Mathieu Desnoyers <[email protected]>
Tested-by: Jann Horn <[email protected]>
CC: Jann Horn <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Peter Zijlstra (Intel) <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Andrea Parri <[email protected]>
CC: Andy Lutomirski <[email protected]>
CC: Avi Kivity <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Boqun Feng <[email protected]>
CC: Dave Watson <[email protected]>
CC: David Sehr <[email protected]>
CC: H. Peter Anvin <[email protected]>
CC: Linus Torvalds <[email protected]>
CC: Maged Michael <[email protected]>
CC: Michael Ellerman <[email protected]>
CC: Paul E. McKenney <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Russell King <[email protected]>
CC: Will Deacon <[email protected]>
CC: [email protected] # v4.16+
CC: [email protected]
---
 kernel/sched/membarrier.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c
index 76e0eaf4654e..305fdcc4c5f7 100644
--- a/kernel/sched/membarrier.c
+++ b/kernel/sched/membarrier.c
@@ -81,12 +81,27 @@ static int membarrier_global_expedited(void)
 
                rcu_read_lock();
                p = task_rcu_dereference(&cpu_rq(cpu)->curr);
-               if (p && p->mm && (atomic_read(&p->mm->membarrier_state) &
-                                  MEMBARRIER_STATE_GLOBAL_EXPEDITED)) {
-                       if (!fallback)
-                               __cpumask_set_cpu(cpu, tmpmask);
-                       else
-                               smp_call_function_single(cpu, ipi_mb, NULL, 1);
+               /*
+                * Skip this CPU if the runqueue's current task is NULL or if
+                * it is a kernel thread.
+                */
+               if (p && READ_ONCE(p->mm)) {
+                       bool mm_match;
+
+                       /*
+                        * Read p->mm and access membarrier_state while holding
+                        * the task lock to ensure existence of mm.
+                        */
+                       task_lock(p);
+                       mm_match = p->mm && 
(atomic_read(&p->mm->membarrier_state) &
+                                            MEMBARRIER_STATE_GLOBAL_EXPEDITED);
+                       task_unlock(p);
+                       if (mm_match) {
+                               if (!fallback)
+                                       __cpumask_set_cpu(cpu, tmpmask);
+                               else
+                                       smp_call_function_single(cpu, ipi_mb, 
NULL, 1);
+                       }
                }
                rcu_read_unlock();
        }
-- 
2.17.1

Reply via email to