mbox series

[v2,0/9] Cover a guard gap corner case

Message ID 20240226190951.3240433-1-rick.p.edgecombe@intel.com
Headers
Series Cover a guard gap corner case |

Message

Edgecombe, Rick P Feb. 26, 2024, 7:09 p.m. UTC 
  Hi,

For v2, the notable change is a bug fix to not clobber the MMF_TOPDOWN 
during fork. In the RFC this resulted in fork() children that didn't exec 
getting the map up behavior, which included the stress-ng bigheap test. It 
turns out much of the 4% improvement seen was due to the bottomup mapping 
direction. With the fix, the performance benefit was a less surprising 
~1%. Other than that, Kirill's style feedback was addressed. There were no 
functional changes (other than the bug).

Link to v1:
https://lore.kernel.org/lkml/20240215231332.1556787-1-rick.p.edgecombe@intel.com/

=======

In working on x86’s shadow stack feature, I came across some limitations 
around the kernel’s handling of guard gaps. AFAICT these limitations are 
not too important for the traditional stack usage of guard gaps, but have 
bigger impact on shadow stack’s usage. And now in addition to x86, we have 
two other architectures implementing shadow stack like features that plan 
to use guard gaps. I wanted to see about addressing them, but I have not 
worked on mmap() placement related code before, so would greatly 
appreciate if people could take a look and point me in the right 
direction.

The nature of the limitations of concern is as follows. In order to ensure 
guard gaps between mappings, mmap() would need to consider two things:
 1. That the new mapping isn’t placed in an any existing mapping’s guard
    gap.
 2. That the new mapping isn’t placed such that any existing mappings are
    not in *its* guard gaps
Currently mmap never considers (2), and (1) is not considered in some 
situations.

When not passing an address hint, or passing one without 
MAP_FIXED_NOREPLACE, (1) is enforced. With MAP_FIXED_NOREPLACE, (1) is not 
enforced. With MAP_FIXED, (1) is not considered, but this seems to be 
expected since MAP_FIXED can already clobber existing mappings. For 
MAP_FIXED_NOREPLACE I would have guessed it should respect the guard gaps 
of existing mappings, but it is probably a little ambiguous.

In this series I just tried to add enforcement of (2) for the normal (no 
address hint) case and only for the newer shadow stack memory (not 
stacks). The reason is that with the no-address-hint situation, landing 
next to a guard gap could come up naturally and so be more influencable by 
attackers such that two shadow stacks could be adjacent without a guard 
gap. Where as the address-hint scenarios would require more control - 
being able to call mmap() with specific arguments. As for why not just fix 
the other corner cases anyway, I thought it might have some greater 
possibility of affecting existing apps.

Thanks,

Rick

Rick Edgecombe (9):
  mm: Switch mm->get_unmapped_area() to a flag
  mm: Introduce arch_get_unmapped_area_vmflags()
  mm: Use get_unmapped_area_vmflags()
  thp: Add thp_get_unmapped_area_vmflags()
  mm: Initialize struct vm_unmapped_area_info
  mm: Take placement mappings gap into account
  x86/mm: Implement HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
  x86/mm: Care about shadow stack guard gap during placement
  selftests/x86: Add placement guard gap test for shstk

 arch/alpha/kernel/osf_sys.c                   |   2 +-
 arch/arc/mm/mmap.c                            |   2 +-
 arch/arm/mm/mmap.c                            |   4 +-
 arch/csky/abiv1/mmap.c                        |   2 +-
 arch/loongarch/mm/mmap.c                      |   2 +-
 arch/mips/mm/mmap.c                           |   2 +-
 arch/parisc/kernel/sys_parisc.c               |   2 +-
 arch/powerpc/mm/book3s64/slice.c              |   4 +-
 arch/s390/mm/hugetlbpage.c                    |   6 +-
 arch/s390/mm/mmap.c                           |   8 +-
 arch/sh/mm/mmap.c                             |   4 +-
 arch/sparc/kernel/sys_sparc_32.c              |   2 +-
 arch/sparc/kernel/sys_sparc_64.c              |  19 ++--
 arch/sparc/mm/hugetlbpage.c                   |   6 +-
 arch/x86/include/asm/pgtable_64.h             |   1 +
 arch/x86/kernel/cpu/sgx/driver.c              |   2 +-
 arch/x86/kernel/sys_x86_64.c                  |  39 +++++--
 arch/x86/mm/hugetlbpage.c                     |   6 +-
 arch/x86/mm/mmap.c                            |   4 +-
 drivers/char/mem.c                            |   2 +-
 drivers/dax/device.c                          |   6 +-
 fs/hugetlbfs/inode.c                          |   6 +-
 fs/proc/inode.c                               |  15 +--
 fs/ramfs/file-mmu.c                           |   2 +-
 include/linux/huge_mm.h                       |  11 ++
 include/linux/mm.h                            |  12 ++-
 include/linux/mm_types.h                      |   6 +-
 include/linux/sched/coredump.h                |   5 +-
 include/linux/sched/mm.h                      |  22 ++++
 io_uring/io_uring.c                           |   2 +-
 mm/debug.c                                    |   6 --
 mm/huge_memory.c                              |  23 ++--
 mm/mmap.c                                     | 101 +++++++++++++-----
 mm/shmem.c                                    |  11 +-
 mm/util.c                                     |   6 +-
 .../testing/selftests/x86/test_shadow_stack.c |  67 +++++++++++-
 36 files changed, 298 insertions(+), 122 deletions(-)