Messages in this thread | | | Date | Wed, 29 Jul 2009 20:53:51 +0900 | From | KAMEZAWA Hiroyuki <> | Subject | Re: [RFC][PATCH 1/2] vmalloc: reorder unmap and removal entry |
| |
On Wed, 29 Jul 2009 17:40:34 +0800 Amerigo Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Jul 29, 2009 at 05:36:00PM +0900, KAMEZAWA Hiroyuki wrote: > >> I wonder fs/proc/kcore.c's vmalloc area access needs some fix. let me try. > >> > >> Thanks, > >Finally, I wrote 2 patches. maybe 2/2 is a fix for the bug. > >but needs comments. > > > >Comaparing 2 calls, vfree() and vread() > >== > >vfree() > > -> __vumap() > > -> remove_vm_area() > > -> free_unmap_vmap_area # > > -> try_purge_vmap_area_lazy() # > > ->__purge_vmap_area_lazy(); # > > -> unmap_vmap_area() # unmap memory here. > > -> write_lock(&vmlist_lock) > > remvoe vm_struct from list > > write_unlock(&vmlist_lock). > >== > >vread() > > -> read_lock(&vmlist_lock); > > get vm_struct -> do memcpy > > read_unlock(&vmlist_lock); > >== > > > I think this is a very good catch! > > Your patch looks reasonable for me. > > > > >Hmm, maybe not related to original bug but above order should be fixed. > > > >From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > > >vmap area should be purged after vm_struct is removed from the list > >because vread/vwrite etc...believes the range is valid while it's on > >vm_struct list. > > > >Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > > Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com> > > Aside, would you like to replace vmlist, a single list, with our > generic list API? :) Just as you did for kcore. Pls send it in > a seprate patch. >
I'll schedule it up after investigation. (I'm not sure why private list handle code is used.) and some pending patchs.
Thanks, -Kame
> Thanks! > > > >--- > > mm/vmalloc.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > >Index: linux-2.6.31-rc4/mm/vmalloc.c > >=================================================================== > >--- linux-2.6.31-rc4.orig/mm/vmalloc.c > >+++ linux-2.6.31-rc4/mm/vmalloc.c > >@@ -1256,17 +1256,21 @@ struct vm_struct *remove_vm_area(const v > > if (va && va->flags & VM_VM_AREA) { > > struct vm_struct *vm = va->private; > > struct vm_struct *tmp, **p; > >- > >- vmap_debug_free_range(va->va_start, va->va_end); > >- free_unmap_vmap_area(va); > >- vm->size -= PAGE_SIZE; > >- > >+ /* > >+ * remove from list and disallow access to this vm_struct > >+ * before unmap. (address range confliction is maintained by > >+ * vmap.) > >+ */ > > write_lock(&vmlist_lock); > > for (p = &vmlist; (tmp = *p) != vm; p = &tmp->next) > > ; > > *p = tmp->next; > > write_unlock(&vmlist_lock); > > > >+ vmap_debug_free_range(va->va_start, va->va_end); > >+ free_unmap_vmap_area(va); > >+ vm->size -= PAGE_SIZE; > >+ > > return vm; > > } > > return NULL; > > >
| |