Bug 15723 - race condition between __purge_vmap_area_lazy() and free_unmap_vmap_area_noflush()
Summary: race condition between __purge_vmap_area_lazy() and free_unmap_vmap_area_nofl...
Status: RESOLVED OBSOLETE
Alias: None
Product: Memory Management
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 high
Assignee: Andrew Morton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-08 05:31 UTC by Leifu Zhao
Modified: 2013-12-10 18:22 UTC (History)
2 users (show)

See Also:
Kernel Version: 2.6.28
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Leifu Zhao 2010-04-08 05:31:32 UTC
This is a bug found in kernel 2.6.28 and exists at least in kernel 2.6.32


Bug description:

There is race condition between function __purge_vmap_area_lazy() and free_unmap_vmap_area_noflush() in vmalloc.c of kernel. In function free_unmap_vmap_area_noflush(), if  va->flags is updated and then is preempted to run __purge_vmap_area_lazy() before updating vmap_lazy_nr, then vmap_lazy_nr used in function __purge_vmap_area_lazy() has incorrect value, therefore cause the crash(due to run BUG_ON function). So the updating va->flags and vmap_lazy_nr must execute atomicly, the solution is to use spinlock to protect updating va->flags and vmap_lazy_nr.


Captured log information:

kernel BUG at mm/vmalloc.c:507!
invalid opcode: 0000 [#1] PREEMPT 
last sysfs file: /sys/class/sound/controlC0/dev
Modules linked in: avcap_nxp(P) avcap_synthetic(P) avcap_core(P) pvrsrvkm
alsa_shim(P) snd_usb_audio ]

Pid: 1022, comm: Audio_Timing Tainted: P           (2.6.28 #1) 
EIP: 0060:[<c01537d5>] EFLAGS: 00010297 CPU: 0
EIP is at __purge_vmap_area_lazy+0x1b1/0x1b5
EAX: c4e1e000 EBX: c04e0cec ECX: c4e1fc9c EDX: c04e0d04
ESI: 000001e2 EDI: c4e1fcc4 EBP: c4e1fcc0 ESP: c4e1fc98



Patch:

diff -urN linux-2.6.28.orig/mm/vmalloc.c linux-2.6.28.mod/mm/vmalloc.c
--- linux-2.6.28.orig/mm/vmalloc.c  2010-04-07 11:09:52.000000000 +0800
+++ linux-2.6.28.mod/mm/vmalloc.c        2010-04-07 11:12:40.000000000 +0800
@@ -467,6 +467,7 @@
 
 static atomic_t vmap_lazy_nr = ATOMIC_INIT(0);
 
+static DEFINE_SPINLOCK(purge_lock);
 /*
  * Purges all lazily-freed vmap areas.
  *
@@ -480,7 +481,6 @@
 static void __purge_vmap_area_lazy(unsigned long *start, unsigned long *end,
                                                int sync, int force_flush)
 {
-        static DEFINE_SPINLOCK(purge_lock);
         LIST_HEAD(valist);
         struct vmap_area *va;
         int nr = 0;
@@ -556,8 +556,10 @@
  */
 static void free_unmap_vmap_area_noflush(struct vmap_area *va)
 {
+       spin_lock(&purge_lock);
         va->flags |= VM_LAZY_FREE;
         atomic_add((va->va_end - va->va_start) >> PAGE_SHIFT, &vmap_lazy_nr);
+       spin_unlock(&purge_lock);
         if (unlikely(atomic_read(&vmap_lazy_nr) > lazy_max_pages()))
                  try_purge_vmap_area_lazy();
 }
Comment 1 Andrew Morton 2010-04-08 05:37:56 UTC
Please don't send patches via bugzilla - it causes lots of problems with
our usual patch management and review processes.

Please send this patch via email as per Documentation/SubmittingPatches. 
Suitable recipients may be found via scripts/get_maintainer.pl.  Please
also cc myself on the email.

Thanks.

Note You need to log in before you can comment on or make changes to this bug.