LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* hugepage: Fix broken check for offset alignment in hugepage mappings
@ 2007-08-27  4:36 David Gibson
  2007-08-27  6:05 ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 2+ messages in thread
From: David Gibson @ 2007-08-27  4:36 UTC (permalink / raw)
  To: Andrew Morton
  Cc: William Lee Irwin, Benjamin Herrenschmidt, Hugh Dickins, linux-kernel

For hugepage mappings, the file offset, like the address and size,
needs to be aligned to the size of a hugepage.

In commit 68589bc353037f233fe510ad9ff432338c95db66, the check for this
was moved into prepare_hugepage_range() along with the address and
size checks.  But since BenH's rework of the get_unmapped_area() paths
leading up to commit 4b1d89290b62bb2db476c94c82cf7442aab440c8,
prepare_hugepage_range() is only called for MAP_FIXED mappings, not
for other mappings.  This means we're no longer ever checking for an
aligned offset - I've confirmed that mmap() will (apparently) succeed
with a misaligned offset on both powerpc and i386 at least.

This patch restores the check, removing it from
prepare_hugepage_range() and putting it back into
hugetlbfs_file_mmap().  I'm putting it there, rather than in the
get_unmapped_area() path so it only needs to go in one place, than
separately in the half-dozen or so arch-specific implementations of
hugetlb_get_unmapped_area().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

---
 arch/i386/mm/hugetlbpage.c    |    2 +-
 arch/ia64/mm/hugetlbpage.c    |    6 ++----
 arch/sparc64/mm/hugetlbpage.c |    2 +-
 fs/hugetlbfs/inode.c          |   15 ++++++++++-----
 include/linux/hugetlb.h       |   10 +++-------
 5 files changed, 17 insertions(+), 18 deletions(-)

Andrew, please apply.  I think this bugfix should go into 2.6.23.

Index: working-2.6/fs/hugetlbfs/inode.c
===================================================================
--- working-2.6.orig/fs/hugetlbfs/inode.c	2007-08-27 13:37:02.000000000 +1000
+++ working-2.6/fs/hugetlbfs/inode.c	2007-08-27 13:37:11.000000000 +1000
@@ -82,14 +82,19 @@ static int hugetlbfs_file_mmap(struct fi
 	int ret;
 
 	/*
-	 * 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 address alignment (but not the pgoff 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;
 
+	if (vma->vm_pgoff & ~(HPAGE_MASK >> PAGE_SHIFT))
+		return -EINVAL;
+
 	vma_len = (loff_t)(vma->vm_end - vma->vm_start);
 
 	mutex_lock(&inode->i_mutex);
@@ -132,7 +137,7 @@ hugetlb_get_unmapped_area(struct file *f
 		return -ENOMEM;
 
 	if (flags & MAP_FIXED) {
-		if (prepare_hugepage_range(addr, len, pgoff))
+		if (prepare_hugepage_range(addr, len))
 			return -EINVAL;
 		return addr;
 	}
Index: working-2.6/include/linux/hugetlb.h
===================================================================
--- working-2.6.orig/include/linux/hugetlb.h	2007-08-27 13:37:02.000000000 +1000
+++ working-2.6/include/linux/hugetlb.h	2007-08-27 13:37:11.000000000 +1000
@@ -66,11 +66,8 @@ 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,
-						pgoff_t pgoff)
+static inline int prepare_hugepage_range(unsigned long addr, unsigned long len)
 {
-	if (pgoff & (~HPAGE_MASK >> PAGE_SHIFT))
-		return -EINVAL;
 	if (len & ~HPAGE_MASK)
 		return -EINVAL;
 	if (addr & ~HPAGE_MASK)
@@ -78,8 +75,7 @@ static inline int prepare_hugepage_range
 	return 0;
 }
 #else
-int prepare_hugepage_range(unsigned long addr, unsigned long len,
-						pgoff_t pgoff);
+int prepare_hugepage_range(unsigned long addr, unsigned long len);
 #endif
 
 #ifndef ARCH_HAS_SETCLEAR_HUGE_PTE
@@ -117,7 +113,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,pgoff)	(-EINVAL)
+#define prepare_hugepage_range(addr,len)	(-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; })
Index: working-2.6/arch/i386/mm/hugetlbpage.c
===================================================================
--- working-2.6.orig/arch/i386/mm/hugetlbpage.c	2007-08-27 13:37:02.000000000 +1000
+++ working-2.6/arch/i386/mm/hugetlbpage.c	2007-08-27 13:37:11.000000000 +1000
@@ -367,7 +367,7 @@ hugetlb_get_unmapped_area(struct file *f
 		return -ENOMEM;
 
 	if (flags & MAP_FIXED) {
-		if (prepare_hugepage_range(addr, len, pgoff))
+		if (prepare_hugepage_range(addr, len))
 			return -EINVAL;
 		return addr;
 	}
Index: working-2.6/arch/ia64/mm/hugetlbpage.c
===================================================================
--- working-2.6.orig/arch/ia64/mm/hugetlbpage.c	2007-08-27 13:37:02.000000000 +1000
+++ working-2.6/arch/ia64/mm/hugetlbpage.c	2007-08-27 13:37:12.000000000 +1000
@@ -75,10 +75,8 @@ int huge_pmd_unshare(struct mm_struct *m
  * 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, pgoff_t pgoff)
+int prepare_hugepage_range(unsigned long addr, unsigned long len)
 {
-	if (pgoff & (~HPAGE_MASK >> PAGE_SHIFT))
-		return -EINVAL;
 	if (len & ~HPAGE_MASK)
 		return -EINVAL;
 	if (addr & ~HPAGE_MASK)
@@ -151,7 +149,7 @@ unsigned long hugetlb_get_unmapped_area(
 
 	/* Handle MAP_FIXED */
 	if (flags & MAP_FIXED) {
-		if (prepare_hugepage_range(addr, len, pgoff))
+		if (prepare_hugepage_range(addr, len))
 			return -EINVAL;
 		return addr;
 	}
Index: working-2.6/arch/sparc64/mm/hugetlbpage.c
===================================================================
--- working-2.6.orig/arch/sparc64/mm/hugetlbpage.c	2007-08-27 13:37:02.000000000 +1000
+++ working-2.6/arch/sparc64/mm/hugetlbpage.c	2007-08-27 13:37:12.000000000 +1000
@@ -175,7 +175,7 @@ hugetlb_get_unmapped_area(struct file *f
 		return -ENOMEM;
 
 	if (flags & MAP_FIXED) {
-		if (prepare_hugepage_range(addr, len, pgoff))
+		if (prepare_hugepage_range(addr, len))
 			return -EINVAL;
 		return addr;
 	}

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: hugepage: Fix broken check for offset alignment in hugepage mappings
  2007-08-27  4:36 hugepage: Fix broken check for offset alignment in hugepage mappings David Gibson
@ 2007-08-27  6:05 ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 2+ messages in thread
From: Benjamin Herrenschmidt @ 2007-08-27  6:05 UTC (permalink / raw)
  To: David Gibson; +Cc: Andrew Morton, William Lee Irwin, Hugh Dickins, linux-kernel

On Mon, 2007-08-27 at 14:36 +1000, David Gibson wrote:
> This patch restores the check, removing it from
> prepare_hugepage_range() and putting it back into
> hugetlbfs_file_mmap().  I'm putting it there, rather than in the
> get_unmapped_area() path so it only needs to go in one place, than
> separately in the half-dozen or so arch-specific implementations of
> hugetlb_get_unmapped_area().

Yup, that's the right place to put it imho.

> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> 

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-08-27  6:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-27  4:36 hugepage: Fix broken check for offset alignment in hugepage mappings David Gibson
2007-08-27  6:05 ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).