Just a quick ping on this, let me know if you need anything more from me! Thanks, Lorenzo
On Sun, Sep 11, 2016 at 11:54:25PM +0100, Lorenzo Stoakes wrote: > The NUMA balancing logic uses an arch-specific PROT_NONE page table flag > defined > by pte_protnone() or pmd_protnone() to mark PTEs or huge page PMDs > respectively > as requiring balancing upon a subsequent page fault. User-defined PROT_NONE > memory regions which also have this flag set will not normally invoke the NUMA > balancing code as do_page_fault() will send a segfault to the process before > handle_mm_fault() is even called. > > However if access_remote_vm() is invoked to access a PROT_NONE region of > memory, > handle_mm_fault() is called via faultin_page() and __get_user_pages() without > any access checks being performed, meaning the NUMA balancing logic is > incorrectly invoked on a non-NUMA memory region. > > A simple means of triggering this problem is to access PROT_NONE mmap'd memory > using /proc/self/mem which reliably results in the NUMA handling functions > being > invoked when CONFIG_NUMA_BALANCING is set. > > This issue was reported in bugzilla (issue 99101) which includes some simple > repro code. > > There are BUG_ON() checks in do_numa_page() and do_huge_pmd_numa_page() added > at > commit c0e7cad to avoid accidentally provoking strange behaviour by attempting > to apply NUMA balancing to pages that are in fact PROT_NONE. The BUG_ON()'s > are > consistently triggered by the repro. > > This patch moves the PROT_NONE check into mm/memory.c rather than invoking > BUG_ON() as faulting in these pages via faultin_page() is a valid reason for > reaching the NUMA check with the PROT_NONE page table flag set and is > therefore > not always a bug. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=99101 > Reported-by: Trevor Saunders <tbsau...@tbsaunde.org> > Signed-off-by: Lorenzo Stoakes <lstoa...@gmail.com> > --- > mm/huge_memory.c | 3 --- > mm/memory.c | 12 +++++++----- > 2 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index d76700d..954be55 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -1198,9 +1198,6 @@ int do_huge_pmd_numa_page(struct fault_env *fe, pmd_t > pmd) > bool was_writable; > int flags = 0; > > - /* A PROT_NONE fault should not end up here */ > - BUG_ON(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))); > - > fe->ptl = pmd_lock(vma->vm_mm, fe->pmd); > if (unlikely(!pmd_same(pmd, *fe->pmd))) > goto out_unlock; > diff --git a/mm/memory.c b/mm/memory.c > index 020226b..aebc04f 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3351,9 +3351,6 @@ static int do_numa_page(struct fault_env *fe, pte_t pte) > bool was_writable = pte_write(pte); > int flags = 0; > > - /* A PROT_NONE fault should not end up here */ > - BUG_ON(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))); > - > /* > * The "pte" at this point cannot be used safely without > * validation through pte_unmap_same(). It's of NUMA type but > @@ -3458,6 +3455,11 @@ static int wp_huge_pmd(struct fault_env *fe, pmd_t > orig_pmd) > return VM_FAULT_FALLBACK; > } > > +static inline bool vma_is_accessible(struct vm_area_struct *vma) > +{ > + return vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE); > +} > + > /* > * These routines also need to handle stuff like marking pages dirty > * and/or accessed for architectures that don't do it in hardware (most > @@ -3524,7 +3526,7 @@ static int handle_pte_fault(struct fault_env *fe) > if (!pte_present(entry)) > return do_swap_page(fe, entry); > > - if (pte_protnone(entry)) > + if (pte_protnone(entry) && vma_is_accessible(fe->vma)) > return do_numa_page(fe, entry); > > fe->ptl = pte_lockptr(fe->vma->vm_mm, fe->pmd); > @@ -3590,7 +3592,7 @@ static int __handle_mm_fault(struct vm_area_struct > *vma, unsigned long address, > > barrier(); > if (pmd_trans_huge(orig_pmd) || pmd_devmap(orig_pmd)) { > - if (pmd_protnone(orig_pmd)) > + if (pmd_protnone(orig_pmd) && vma_is_accessible(vma)) > return do_huge_pmd_numa_page(&fe, orig_pmd); > > if ((fe.flags & FAULT_FLAG_WRITE) && > -- > 2.9.3
