Thanks for your comments. Thank, Inki Dae
2013/9/26 Al Viro <v...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel