RE: [hugepage] Fix unmap_and_free_vma backout path

Hugh Dickins
2006-11-13T20:41:49Z

On Mon, 13 Nov 2006, Adam Litke wrote:

> Looks good to me, notwithstanding the nano-nit below.
>
> On Mon, 2006-11-13 at 17:00 +0000, Hugh Dickins wrote:
> > + /*
> > + * vma alignment has already been checked by prepare_hugepage_range.
> > + * If you add any error returns here, do so after setting VM_HUGETLB,
> > + * so is_vm_huge_tlb_page tests below unmap_region go the right way
> > + * when do_mmap_pgoff unwinds (may be important on powerpc and ia64).
> > + */
>
> Sorry. This is hardly worth it, but the function referred to by this
> comment is actually is_vm_hugetlb_page() :-/

Indeed, thanks!

[PATCH] hugetlb: prepare_hugepage_range check offset too

prepare_hugepage_range should check file offset alignment when it checks
virtual address and length, to stop MAP_FIXED with a bad huge offset from
unmapping before it fails further down. PowerPC should apply the same
prepare_hugepage_range alignment checks as ia64 and all the others do.

Then none of the alignment checks in hugetlbfs_file_mmap are required
(nor is the check for too small a mapping); but even so, move up setting
of VM_HUGETLB and add a comment to warn of what David Gibson discovered -
if hugetlbfs_file_mmap fails before setting it, do_mmap_pgoff's unmap_region
when unwinding from error will go the non-huge way, which may cause bad
behaviour on architectures (powerpc and ia64) which segregate their huge
mappings into a separate region of the address space.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

arch/ia64/mm/hugetlbpage.c | 4 +++-
arch/powerpc/mm/hugetlbpage.c | 8 ++++++--
fs/hugetlbfs/inode.c | 21 ++++++++-------------
include/linux/hugetlb.h | 10 +++++++---
mm/mmap.c | 2 +-
5 files changed, 25 insertions(+), 20 deletions(-)

--- 2.6.19-rc5/arch/ia64/mm/hugetlbpage.c 2006-09-20 04:42:06.000000000 +0100
+++ linux/arch/ia64/mm/hugetlbpage.c 2006-11-13 15:37:04.000000000 +0000
@@ -70,8 +70,10 @@ huge_pte_offset (struct mm_struct *mm, u
* Don't actually need to do any preparation, but need to make sure
* the address is in the right region.
*/
-int prepare_hugepage_range(unsigned long addr, unsigned long len)
+int prepare_hugepage_range(unsigned long addr, unsigned long len, pgoff_t pgoff)
{
+ if (pgoff & (~HPAGE_MASK >> PAGE_SHIFT))
+ return -EINVAL;
if (len & ~HPAGE_MASK)
return -EINVAL;
if (addr & ~HPAGE_MASK)
--- 2.6.19-rc5/arch/powerpc/mm/hugetlbpage.c 2006-11-08 08:30:56.000000000 +0000
+++ linux/arch/powerpc/mm/hugetlbpage.c 2006-11-13 15:37:04.000000000 +0000
@@ -491,11 +491,15 @@ static int open_high_hpage_areas(struct
return 0;
}

-int prepare_hugepage_range(unsigned long addr, unsigned long len)
+int prepare_hugepage_range(unsigned long addr, unsigned long len, pgoff_t pgoff)
{
int err = 0;

- if ( (addr+len) < addr )
+ if (pgoff & (~HPAGE_MASK >> PAGE_SHIFT))
+ return -EINVAL;
+ if (len & ~HPAGE_MASK)
+ return -EINVAL;
+ if (addr & ~HPAGE_MASK)
return -EINVAL;

if (addr < 0x100000000UL)
--- 2.6.19-rc5/fs/hugetlbfs/inode.c 2006-11-08 08:31:14.000000000 +0000
+++ linux/fs/hugetlbfs/inode.c 2006-11-13 15:37:04.000000000 +0000
@@ -62,24 +62,19 @@ static int hugetlbfs_file_mmap(struct fi
loff_t len, vma_len;
int ret;

- if (vma->vm_pgoff & (HPAGE_SIZE / PAGE_SIZE - 1))
- return -EINVAL;
-
- if (vma->vm_start & ~HPAGE_MASK)
- return -EINVAL;
-
- if (vma->vm_end & ~HPAGE_MASK)
- return -EINVAL;
-
- if (vma->vm_end - vma->vm_start < HPAGE_SIZE)
- return -EINVAL;
+ /*
+ * vma alignment has already been checked by prepare_hugepage_range.
+ * If you add any error returns here, do so after setting VM_HUGETLB,
+ * so is_vm_hugetlb_page tests below unmap_region go the right way
+ * when do_mmap_pgoff unwinds (may be important on powerpc and ia64).
+ */
+ vma->vm_flags |= VM_HUGETLB | VM_RESERVED;
+ vma->vm_ops = &hugetlb_vm_ops;

vma_len = (loff_t)(vma->vm_end - vma->vm_start);

mutex_lock(&inode->i_mutex);
file_accessed(file);
- vma->vm_flags |= VM_HUGETLB | VM_RESERVED;
- vma->vm_ops = &hugetlb_vm_ops;

ret = -ENOMEM;
len = vma_len + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
--- 2.6.19-rc5/include/linux/hugetlb.h 2006-11-08 08:31:21.000000000 +0000
+++ linux/include/linux/hugetlb.h 2006-11-13 15:37:04.000000000 +0000
@@ -60,8 +60,11 @@ void hugetlb_free_pgd_range(struct mmu_g
* If the arch doesn't supply something else, assume that hugepage
* size aligned regions are ok without further preparation.
*/
-static inline int prepare_hugepage_range(unsigned long addr, unsigned long len)
+static inline int prepare_hugepage_range(unsigned long addr, unsigned long len,
+ pgoff_t pgoff)
{
+ if (pgoff & (~HPAGE_MASK >> PAGE_SHIFT))
+ return -EINVAL;
if (len & ~HPAGE_MASK)
return -EINVAL;
if (addr & ~HPAGE_MASK)
@@ -69,7 +72,8 @@ static inline int prepare_hugepage_range
return 0;
}
#else
-int prepare_hugepage_range(unsigned long addr, unsigned long len);
+int prepare_hugepage_range(unsigned long addr, unsigned long len,
+ pgoff_t pgoff);
#endif

#ifndef ARCH_HAS_SETCLEAR_HUGE_PTE
@@ -107,7 +111,7 @@ static inline unsigned long hugetlb_tota
#define hugetlb_report_meminfo(buf) 0
#define hugetlb_report_node_meminfo(n, buf) 0
#define follow_huge_pmd(mm, addr, pmd, write) NULL
-#define prepare_hugepage_range(addr, len) (-EINVAL)
+#define prepare_hugepage_range(addr,len,pgoff) (-EINVAL)
#define pmd_huge(x) 0
#define is_hugepage_only_range(mm, addr, len) 0
#define hugetlb_free_pgd_range(tlb, addr, end, floor, ceiling) ({BUG(); 0; })
--- 2.6.19-rc5/mm/mmap.c 2006-11-08 08:31:23.000000000 +0000
+++ linux/mm/mmap.c 2006-11-13 15:37:04.000000000 +0000
@@ -1379,7 +1379,7 @@ get_unmapped_area(struct file *file, uns
* Check if the given range is hugepage aligned, and
* can be made suitable for hugepages.
*/
- ret = prepare_hugepage_range(addr, len);
+ ret = prepare_hugepage_range(addr, len, pgoff);
} else {
/*
* Ensure that a normal request is not falling in a