slub: tid must be retrieved from the percpu area of the current processor
authorChristoph Lameter <cl@linux.com>
Wed, 23 Jan 2013 21:45:48 +0000 (21:45 +0000)
committerPekka Enberg <penberg@kernel.org>
Fri, 5 Apr 2013 11:23:06 +0000 (14:23 +0300)
As Steven Rostedt has pointer out: rescheduling could occur on a
different processor after the determination of the per cpu pointer and
before the tid is retrieved. This could result in allocation from the
wrong node in slab_alloc().

The effect is much more severe in slab_free() where we could free to the
freelist of the wrong page.

The window for something like that occurring is pretty small but it is
possible.

Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
mm/slub.c

index 8b1b99d399cbe0b33b0ae507a78fe24335a849f6..4df2c0c337fb9ebd3fd0b8aead967b338a5b4453 100644 (file)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2332,13 +2332,18 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
 
        s = memcg_kmem_get_cache(s, gfpflags);
 redo:
-
        /*
         * Must read kmem_cache cpu data via this cpu ptr. Preemption is
         * enabled. We may switch back and forth between cpus while
         * reading from one cpu area. That does not matter as long
         * as we end up on the original cpu again when doing the cmpxchg.
+        *
+        * Preemption is disabled for the retrieval of the tid because that
+        * must occur from the current processor. We cannot allow rescheduling
+        * on a different processor between the determination of the pointer
+        * and the retrieval of the tid.
         */
+       preempt_disable();
        c = __this_cpu_ptr(s->cpu_slab);
 
        /*
@@ -2348,7 +2353,7 @@ redo:
         * linked list in between.
         */
        tid = c->tid;
-       barrier();
+       preempt_enable();
 
        object = c->freelist;
        page = c->page;
@@ -2595,10 +2600,11 @@ redo:
         * data is retrieved via this pointer. If we are on the same cpu
         * during the cmpxchg then the free will succedd.
         */
+       preempt_disable();
        c = __this_cpu_ptr(s->cpu_slab);
 
        tid = c->tid;
-       barrier();
+       preempt_enable();
 
        if (likely(page == c->page)) {
                set_freepointer(s, object, c->freelist);