lockdep: fix oops in processing workqueue

Under memory load, on x86_64, with lockdep enabled, the workqueue's
process_one_work() has been seen to oops in __lock_acquire(), barfing
on a 0xffffffff00000000 pointer in the lockdep_map's class_cache[].

Because it's permissible to free a work_struct from its callout function,
the map used is an onstack copy of the map given in the work_struct: and
that copy is made without any locking.

Surprisingly, gcc (4.5.1 in Hugh's case) uses "rep movsl" rather than
"rep movsq" for that structure copy: which might race with a workqueue
user's wait_on_work() doing lock_map_acquire() on the source of the
copy, putting a pointer into the class_cache[], but only in time for
the top half of that pointer to be copied to the destination map.

Boom when process_one_work() subsequently does lock_map_acquire()
on its onstack copy of the lockdep_map.

Fix this, and a similar instance in call_timer_fn(), with a
lockdep_copy_map() function which additionally NULLs the class_cache[].

Note: this oops was actually seen on 3.4-next, where flush_work() newly
does the racing lock_map_acquire(); but Tejun points out that 3.4 and
earlier are already vulnerable to the same through wait_on_work().

* Patch orginally from Peter.  Hugh modified it a bit and wrote the
  description.

Signed-off-by: Peter Zijlstra <[email protected]>
Reported-by: Hugh Dickins <[email protected]>
LKML-Reference: <alpine.LSU.2.00.1205070951170[email protected]>
Signed-off-by: Tejun Heo <[email protected]>

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index d36619ead3baee89555505ade8c8f0fcc060f587..00e46376e28f73d33bdfbf56480932dfa12eee9f 100644 (file)
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -157,6 +157,24 @@ struct lockdep_map {
 #endif
 };
 
+static inline void lockdep_copy_map(struct lockdep_map *to,
+                                   struct lockdep_map *from)
+{
+       int i;
+
+       *to = *from;
+       /*
+        * Since the class cache can be modified concurrently we could observe
+        * half pointers (64bit arch using 32bit copy insns). Therefore clear
+        * the caches and take the performance hit.
+        *
+        * XXX it doesn't work well with lockdep_set_class_and_subclass(), since
+        *     that relies on cache abuse.
+        */
+       for (i = 0; i < NR_LOCKDEP_CACHING_CLASSES; i++)
+               to->class_cache[i] = NULL;
+}
+
 /*
  * Every lock has a list of other locks that were taken after it.
  * We only grow the list, never remove from it:

diff --git a/kernel/timer.c b/kernel/timer.c
index a297ffcf888eb20093b0609485e03edbd85cd214..b12385244bb5b6af7694c824d58ee1ff63113046 100644 (file)
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1102,7 +1102,9 @@ static void call_timer_fn(struct timer_list *timer, void (*fn)(unsigned long),
         * warnings as well as problems when looking into
         * timer->lockdep_map, make a copy and use that here.
         */
-       struct lockdep_map lockdep_map = timer->lockdep_map;
+       struct lockdep_map lockdep_map;
+
+       lockdep_copy_map(&lockdep_map, &timer->lockdep_map);
 #endif
        /*
         * Couple the lock chain with the lock chain at

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index c36c86cf7900a3e116abdc46d41dd416ae181862..9a3128dc67df450d201969bb5fa9a99773d99c1b 100644 (file)
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1818,7 +1818,9 @@ __acquires(&gcwq->lock)
         * lock freed" warnings as well as problems when looking into
         * work->lockdep_map, make a copy and use that here.
         */
-       struct lockdep_map lockdep_map = work->lockdep_map;
+       struct lockdep_map lockdep_map;
+
+       lockdep_copy_map(&lockdep_map, &work->lockdep_map);
 #endif
        /*
         * A single work shouldn't be executed concurrently by