On 10/27/22 15:34, Peter Xu wrote: > On Wed, Oct 26, 2022 at 05:34:04PM -0700, Mike Kravetz wrote: > > On 10/26/22 17:59, Peter Xu wrote: > > If we want to use the vma read lock to protect here as the slow gup path, > then please check again with below [1] - I think we'll also need to protect > it with fast-gup (probably with trylock only, because fast-gup cannot > sleep) or it'll encounter the same race, iiuc. > > Actually, instead of using vma lock, I really think this is another problem > and needs standalone fixing. The problem is we allows huge_pte_offset() to > walk the process pgtable without any protection, while pmd unsharing can > drop a page anytime. huge_pte_offset() is always facing use-after-free > when walking the PUD page. > > We may want RCU lock to protect the pgtable pages from getting away when > huge_pte_offset() is walking it, it'll be safe then because pgtable pages > are released in RCU fashion only (e.g. in above example, process [2] will > munmap() and release the last ref to the "used to be shared" pmd and the > PUD that maps the shared pmds will be released only after a RCU grace > period), and afaict that's also what's protecting fast-gup from accessing > freed pgtable pages. > > If with all huge_pte_offset() callers becoming RCU-safe, then IIUC we can > drop the vma lock in all GUP code, aka, in hugetlb_follow_page_mask() here, > because both slow and fast gup should be safe too in the same manner. > > Thanks, > > > > IIUC it's also the same as fast-gup - afaiu we don't take the read vma lock > > > in fast-gup too but I also think it's safe. But I hope I didn't miss > > > something. > > [1] Thanks Peter! I think the best thing would be to eliminate the vma_lock calls in this patch. The code it is replacing/simplifying does not do any locking, so no real regression. I think a scheme like you describe above is going to require some more thought/work. It might be better as a follow on patch. -- Mike Kravetz
