Re: [PATCH] mm: do_shared_fault: fix potential NULL pointer dereference

[Andrew Morton <akpm@linux-foundation.org>]

On Sat, 1 Mar 2014 11:14:17 +0800 Bob Liu <lliubbo@gmail.com> wrote:

> >
> > --- a/mm/memory.c~mm-introduce-do_shared_fault-and-drop-do_fault-fix-fix
> > +++ a/mm/memory.c
> > @@ -3476,6 +3476,12 @@ set_pte:
> >
> >         if (set_page_dirty(fault_page))
> >                 dirtied = 1;
> > +       /*
> > +        * Take a local copy of the address_space - page.mapping may be zeroed
> > +        * by truncate after unlock_page().   The address_space itself remains
> > +        * pinned by vma->vm_file's reference.  We rely on unlock_page()'s
> > +        * release semantics to prevent the compiler from undoing this copying.
> > +        */
> >         mapping = fault_page->mapping;
> >         unlock_page(fault_page);
> >         if ((dirtied || vma->vm_ops->page_mkwrite) && mapping) {
> >
> > I don't actually know if that's true.  What *does* protect ->mapping
> > from reclaim, drop_caches, etc?
> >
> 
> I also puzzled what can protect ->mapping.

Yes, ->mapping is pinned by the reference from vma->vm_file. 
vma->vm_file (and the vma itself) are protected by mmap_sem (held for
rear or for write).

I'll stick this in there, see what happens..

--- a/mm/memory.c~a
+++ a/mm/memory.c
@@ -3422,6 +3422,8 @@ static int do_shared_fault(struct mm_str
 	struct vm_fault vmf;
 	int ret, tmp;
 
+	WARN_ON_ONCE(!rwsem_is_locked(&mm->mmap_sem));
+
 	ret = __do_fault(vma, address, pgoff, flags, &fault_page);
 	if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY)))
 		return ret;
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>