Commit bdb21928 authored Jul 10, 2008 by Dmitry Adamushko Committed by Linus Torvalds Jul 10, 2008

slub: Fix use-after-preempt of per-CPU data structure



Vegard Nossum reported a crash in kmem_cache_alloc():

	BUG: unable to handle kernel paging request at da87d000
	IP: [<c01991c7>] kmem_cache_alloc+0xc7/0xe0
	*pde = 28180163 *pte = 1a87d160
	Oops: 0002 [#1] PREEMPT SMP DEBUG_PAGEALLOC
	Pid: 3850, comm: grep Not tainted (2.6.26-rc9-00059-gb190333 #5)
	EIP: 0060:[<c01991c7>] EFLAGS: 00210203 CPU: 0
	EIP is at kmem_cache_alloc+0xc7/0xe0
	EAX: 00000000 EBX: da87c100 ECX: 1adad71a EDX: 6b6b6b6b
	ESI: 00200282 EDI: da87d000 EBP: f60bfe74 ESP: f60bfe54
	DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068

and analyzed it:

  "The register %ecx looks innocent but is very important here. The disassembly:

       mov    %edx,%ecx
       shr    $0x2,%ecx
       rep stos %eax,%es:(%edi) <-- the fault

   So %ecx has been loaded from %edx... which is 0x6b6b6b6b/POISON_FREE.
   (0x6b6b6b6b >> 2 == 0x1adadada.)

   %ecx is the counter for the memset, from here:

       memset(object, 0, c->objsize);

  i.e. %ecx was loaded from c->objsize, so "c" must have been freed.
  Where did "c" come from? Uh-oh...

       c = get_cpu_slab(s, smp_processor_id());

  This looks like it has very much to do with CPU hotplug/unplug. Is
  there a race between SLUB/hotplug since the CPU slab is used after it
  has been freed?"

Good analysis.

Yeah, it's possible that a caller of kmem_cache_alloc() -> slab_alloc()
can be migrated on another CPU right after local_irq_restore() and
before memset().  The inital cpu can become offline in the mean time (or
a migration is a consequence of the CPU going offline) so its
'kmem_cache_cpu' structure gets freed ( slab_cpuup_callback).

At some point of time the caller continues on another CPU having an
obsolete pointer...

Signed-off-by: Dmitry Adamushko <dmitry.adamushko@gmail.com>
Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
Acked-by: Ingo Molnar <mingo@elte.hu>
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

parent 96a8e13e

mm/slub.c

+3 −1

Original line number	Diff line number	Diff line
		@@ -1628,9 +1628,11 @@ static __always_inline void slab_alloc(struct kmem_cache s,
		void **object;
		struct kmem_cache_cpu *c;
		unsigned long flags;
		unsigned int objsize;

		local_irq_save(flags);
		c = get_cpu_slab(s, smp_processor_id());
		objsize = c->objsize;
		if (unlikely(!c->freelist \|\| !node_match(c, node)))

		object = __slab_alloc(s, gfpflags, node, addr, c);
		@@ -1643,7 +1645,7 @@ static __always_inline void slab_alloc(struct kmem_cache s,
		local_irq_restore(flags);

		if (unlikely((gfpflags & __GFP_ZERO) && object))
		memset(object, 0, c->objsize);
		memset(object, 0, objsize);

		return object;
		}

Admin message