diff mbox series

[RFC,4/8] thp: Add thp_get_unmapped_area_vmflags()

Message ID 20240215231332.1556787-5-rick.p.edgecombe@intel.com (mailing list archive)
State New
Headers show
Series Cover a guard gap corner case | expand

Commit Message

Edgecombe, Rick P Feb. 15, 2024, 11:13 p.m. UTC
When memory is being placed, mmap() will take care to respect the guard
gaps of certain types of memory (VM_SHADOWSTACK, VM_GROWSUP and
VM_GROWSDOWN). In order to ensure guard gaps between mappings, mmap()
needs to consider two things:
 1. That the new mapping isn’t placed in an any existing mappings guard
    gaps.
 2. That the new mapping isn’t placed such that any existing mappings
    are not in *its* guard gaps.

The long standing behavior of mmap() is to ensure 1, but not take any care
around 2. So for example, if there is a PAGE_SIZE free area, and a
mmap() with a PAGE_SIZE size, and a type that has a guard gap is being
placed, mmap() may place the shadow stack in the PAGE_SIZE free area. Then
the mapping that is supposed to have a guard gap will not have a gap to
the adjacent VMA.

Add a THP implementations of the vm_flags variant of get_unmapped_area().
Future changes will call this from mmap.c in the do_mmap() path to allow
shadow stacks to be placed with consideration taken for the start guard
gap. Shadow stack memory is always private and anonymous and so special
guard gap logic is not needed in a lot of caseis, but it can be mapped by
THP, so needs to be handled.

Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
 include/linux/huge_mm.h | 11 +++++++++++
 mm/huge_memory.c        | 23 ++++++++++++++++-------
 mm/mmap.c               | 12 +++++++-----
 3 files changed, 34 insertions(+), 12 deletions(-)

Comments

Kirill A . Shutemov Feb. 16, 2024, 12:59 p.m. UTC | #1
On Thu, Feb 15, 2024 at 03:13:28PM -0800, Rick Edgecombe wrote:
> +unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
> +		unsigned long len, unsigned long pgoff, unsigned long flags)
> +{
> +	return thp_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, 0);
>  }
>  EXPORT_SYMBOL_GPL(thp_get_unmapped_area);

Again, static inline should be fine.
Edgecombe, Rick P Feb. 16, 2024, 10:21 p.m. UTC | #2
On Fri, 2024-02-16 at 14:59 +0200, Kirill A. Shutemov wrote:
> On Thu, Feb 15, 2024 at 03:13:28PM -0800, Rick Edgecombe wrote:
> > +unsigned long thp_get_unmapped_area(struct file *filp, unsigned
> > long addr,
> > +               unsigned long len, unsigned long pgoff, unsigned
> > long flags)
> > +{
> > +       return thp_get_unmapped_area_vmflags(filp, addr, len,
> > pgoff, flags, 0);
> >   }
> >   EXPORT_SYMBOL_GPL(thp_get_unmapped_area);
> 
> Again, static inline should be fine.

Not sure if the diff might be a bit misleading here, the signature of
thp_get_unmapped_area() didn't actually change.

If thp_get_unmapped_area() is made into a static inline, then
thp_get_unmapped_area_vmflags() would need to be exported instead so it
could be used in some modules. Unlike get_unmapped_area()
thp_get_unmapped_area is actually is used that way.

Better to export the more limited version?
Kirill A . Shutemov Feb. 17, 2024, 12:57 p.m. UTC | #3
On Fri, Feb 16, 2024 at 10:21:13PM +0000, Edgecombe, Rick P wrote:
> On Fri, 2024-02-16 at 14:59 +0200, Kirill A. Shutemov wrote:
> > On Thu, Feb 15, 2024 at 03:13:28PM -0800, Rick Edgecombe wrote:
> > > +unsigned long thp_get_unmapped_area(struct file *filp, unsigned
> > > long addr,
> > > +               unsigned long len, unsigned long pgoff, unsigned
> > > long flags)
> > > +{
> > > +       return thp_get_unmapped_area_vmflags(filp, addr, len,
> > > pgoff, flags, 0);
> > >   }
> > >   EXPORT_SYMBOL_GPL(thp_get_unmapped_area);
> > 
> > Again, static inline should be fine.
> 
> Not sure if the diff might be a bit misleading here, the signature of
> thp_get_unmapped_area() didn't actually change.
> 
> If thp_get_unmapped_area() is made into a static inline, then
> thp_get_unmapped_area_vmflags() would need to be exported instead so it
> could be used in some modules. Unlike get_unmapped_area()
> thp_get_unmapped_area is actually is used that way.
> 
> Better to export the more limited version?

Okay, it is a valid point.
diff mbox series

Patch

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index fa0350b0812a..ef7251dfd9f9 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -139,6 +139,9 @@  bool hugepage_vma_check(struct vm_area_struct *vma, unsigned long vm_flags,
 
 unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
 		unsigned long len, unsigned long pgoff, unsigned long flags);
+unsigned long thp_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
+		unsigned long len, unsigned long pgoff, unsigned long flags,
+		vm_flags_t vm_flags);
 
 void folio_prep_large_rmappable(struct folio *folio);
 bool can_split_folio(struct folio *folio, int *pextra_pins);
@@ -286,6 +289,14 @@  static inline void folio_prep_large_rmappable(struct folio *folio) {}
 
 #define thp_get_unmapped_area	NULL
 
+static inline unsigned long
+thp_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
+			      unsigned long len, unsigned long pgoff,
+			       unsigned long flags, vm_flags_t vm_flags)
+{
+	return 0;
+}
+
 static inline bool
 can_split_folio(struct folio *folio, int *pextra_pins)
 {
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index e9ef43a719a5..f235f6d3ff62 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -628,7 +628,8 @@  static inline bool is_transparent_hugepage(struct folio *folio)
 
 static unsigned long __thp_get_unmapped_area(struct file *filp,
 		unsigned long addr, unsigned long len,
-		loff_t off, unsigned long flags, unsigned long size)
+		loff_t off, unsigned long flags, unsigned long size,
+		vm_flags_t vm_flags)
 {
 	loff_t off_end = off + len;
 	loff_t off_align = round_up(off, size);
@@ -641,8 +642,8 @@  static unsigned long __thp_get_unmapped_area(struct file *filp,
 	if (len_pad < len || (off + len_pad) < off)
 		return 0;
 
-	ret = mm_get_unmapped_area(current->mm, filp, addr, len_pad,
-				   off >> PAGE_SHIFT, flags);
+	ret = mm_get_unmapped_area_vmflags(current->mm, filp, addr, len_pad,
+					   off >> PAGE_SHIFT, flags, vm_flags);
 
 	/*
 	 * The failure might be due to length padding. The caller will retry
@@ -662,17 +663,25 @@  static unsigned long __thp_get_unmapped_area(struct file *filp,
 	return ret;
 }
 
-unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
-		unsigned long len, unsigned long pgoff, unsigned long flags)
+unsigned long thp_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
+		unsigned long len, unsigned long pgoff, unsigned long flags,
+		vm_flags_t vm_flags)
 {
 	unsigned long ret;
 	loff_t off = (loff_t)pgoff << PAGE_SHIFT;
 
-	ret = __thp_get_unmapped_area(filp, addr, len, off, flags, PMD_SIZE);
+	ret = __thp_get_unmapped_area(filp, addr, len, off, flags, PMD_SIZE, vm_flags);
 	if (ret)
 		return ret;
 
-	return mm_get_unmapped_area(current->mm, filp, addr, len, pgoff, flags);
+	return mm_get_unmapped_area_vmflags(current->mm, filp, addr, len, pgoff, flags,
+					    vm_flags);
+}
+
+unsigned long thp_get_unmapped_area(struct file *filp, unsigned long addr,
+		unsigned long len, unsigned long pgoff, unsigned long flags)
+{
+	return thp_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, 0);
 }
 EXPORT_SYMBOL_GPL(thp_get_unmapped_area);
 
diff --git a/mm/mmap.c b/mm/mmap.c
index 2723f26f7c62..936d728ba1ca 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1857,16 +1857,18 @@  __get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
 		 */
 		pgoff = 0;
 		get_area = shmem_get_unmapped_area;
-	} else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
-		/* Ensures that larger anonymous mappings are THP aligned. */
-		get_area = thp_get_unmapped_area;
 	}
 
-	if (get_area)
+	if (get_area) {
 		addr = get_area(file, addr, len, pgoff, flags);
-	else
+	} else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
+		/* Ensures that larger anonymous mappings are THP aligned. */
+		addr = thp_get_unmapped_area_vmflags(file, addr, len,
+						     pgoff, flags, vm_flags);
+	} else {
 		addr = mm_get_unmapped_area_vmflags(current->mm, file, addr, len,
 						    pgoff, flags, vm_flags);
+	}
 	if (IS_ERR_VALUE(addr))
 		return addr;