On 9/21/22 16:57, Mike Kravetz wrote: > On 09/21/22 10:48, Mike Kravetz wrote: >> On 09/21/22 16:34, Liu Shixin wrote: >>> The vma_lock and hugetlb_fault_mutex are dropped before handling >>> userfault and reacquire them again after handle_userfault(), but >>> reacquire the vma_lock could lead to UAF[1] due to the following >>> race, >>> >>> hugetlb_fault >>> hugetlb_no_page >>> /*unlock vma_lock */ >>> hugetlb_handle_userfault >>> handle_userfault >>> /* unlock mm->mmap_lock*/ >>> vm_mmap_pgoff >>> do_mmap >>> mmap_region >>> munmap_vma_range >>> /* clean old vma */ >>> /* lock vma_lock again <--- UAF */ >>> /* unlock vma_lock */ >>> >>> Since the vma_lock will unlock immediately after hugetlb_handle_userfault(), >>> let's drop the unneeded lock and unlock in hugetlb_handle_userfault() to fix >>> the issue. >> >> Thank you very much! >> >> When I saw this report, the obvious fix was to do something like what you have >> done below. That looks fine with a few minor comments. >> >> One question I have not yet answered is, "Does this same issue apply to >> follow_hugetlb_page()?". I believe it does. follow_hugetlb_page calls >> hugetlb_fault which could result in the fault being processed by userfaultfd. >> If we experience the race above, then the associated vma could no longer be >> valid when returning from hugetlb_fault. follow_hugetlb_page and callers >> have a flag (locked) to deal with dropping mmap lock. However, I am not sure >> if it is handled correctly WRT userfaultfd. I think this needs to be answered >> before fixing. And, if the follow_hugetlb_page code needs to be fixed it >> should be done at the same time. >> > > To at least verify this code path, I added userfaultfd handling to the gup_test > program in kernel selftests. When doing basic gup test on a hugetlb page in Just for those of us who are easily confused by userfaultfd cases, can you show what that patch is? It would help me understand this a little faster. Actually I'm expecting that Peter can easily answer this whole thing. :) thanks, -- John Hubbard NVIDIA
