[RFC] deadlock in "drm/exynos: fix wrong pointer access at vm close"

[Inki Dae]

Thanks for your comments.

Thank,
Inki Dae

2013/9/26 Al Viro <viro at zeniv.linux.org.uk>

> On Tue, Sep 24, 2013 at 01:41:00PM +0900, Inki Dae wrote:
>
> > I can't see to hold ->mmap_sem when it calls find_vma() anywhere else.
>
> Er...  What, in your opinion, would protect the result of find_vma(), if
> not that?  E.g. if another thread does munmap() on that area...


Right. the vm_area_struct object could be removed from rb tree;
mm->mm_rb->rb_node.


>  vma isn't
> refcounted; there are only two things that can prevent its freeing -
> mmap_sem being held and the lack of anybody else seeing the address of
> mm_struct it belongs to (basically, when we are killing mm_struct off
> or when we are populating a fresh mm_struct; in the latter case the
> parent is locked, but the child doesn't need to).
>

Ok, will add down_write(&mm->mmap_sem) and up_write(&mm->mmap_sem) between
and after find_vma() call.


>
> Look at page fault handlers - they grab mmap_sem (shared) before looking
> for
> vma.


Yes, and do_munmap also.


>  And anything modifying the list of vmas (vm_mmap(), etc.) grabs it
> exclusive...
>
> > > to caller) and clones it manually, regardless of whether that vma
> allows
> > > to clone itself or not.  Quite a few drivers rely on that not
> happening...
> > >
> >
> > I think that has no any problem because exynos_gem_get_vma() takes a
> > reference to vma, and also v4l2 side is using same way. I and v4l2 guys
> > might be missing something what you are concerning. So Could you give us
> > more comments?
>
> vb2_get_vma()/vb2_put_userptr() is also buggy.  At the very least, it
> should refuse to handle ones with VM_DONTCOPY in flags.  Again, there
> are drivers that seriously rely on VM_DONTCOPY being honoured.
>

Got it. will check the VM_DONTCOPY flag before copying the vma.


>
> BTW, what do you expect exynos_gem_get_pages_from_userptr() to do if
> the area has been unmapped in the meanwhile?


Yes, that function would try to access a invaild vma.


>  Or, for that matter,
> if that has been followed by mmap() of something with VM_IO on the
> same range of addresses...  ->open() does *NOT* prevent any of that.
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130929/5aaaf3fb/attachment.html>