* [PATCH 0/3] mm: Care about shadow stack guard gap when getting an unmapped area
@ 2024-09-02 19:08 Mark Brown
2024-09-02 19:08 ` [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default Mark Brown
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Mark Brown @ 2024-09-02 19:08 UTC (permalink / raw)
To: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes
Cc: Catalin Marinas, Will Deacon, Deepak Gupta, linux-arm-kernel,
linux-alpha, linux-kernel, linux-snps-arc, linux-arm-kernel,
linux-csky, loongarch, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-mm, Mark Brown, Rick Edgecombe
As covered in the commit log for c44357c2e76b ("x86/mm: care about shadow
stack guard gap during placement") our current mmap() implementation does
not take care to ensure that a new mapping isn't placed with existing
mappings inside it's own guard gaps. This is particularly important for
shadow stacks since if two shadow stacks end up getting placed adjacent to
each other then they can overflow into each other which weakens the
protection offered by the feature.
On x86 there is a custom arch_get_unmapped_area() which was updated by the
above commit to cover this case by specifying a start_gap for allocations
with VM_SHADOW_STACK. Both arm64 and RISC-V have equivalent features and
use the generic implementation of arch_get_unmapped_area() so let's make
the equivalent change there so they also don't get shadow stack pages
placed without guard pages. The arm64 and RISC-V shadow stack
implementations are currently on the list:
https://lore.kernel.org/r/20240829-arm64-gcs-v12-0-42fec94743
https://lore.kernel.org/lkml/20240403234054.2020347-1-debug@rivosinc.com/
Given the addition of the use of vm_flags in the generic implementation
we also simplify the set of possibilities that have to be dealt with in
the core code by making arch_get_unmapped_area() take vm_flags as
standard. This is a bit invasive since the prototype change touches
quite a few architectures but since the parameter is ignored the change
is straightforward, the simplification for the generic code seems worth
it.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Mark Brown (3):
mm: Make arch_get_unmapped_area() take vm_flags by default
mm: Pass vm_flags to generic_get_unmapped_area()
mm: Care about shadow stack guard gap when getting an unmapped area
arch/alpha/kernel/osf_sys.c | 2 +-
arch/arc/mm/mmap.c | 3 ++-
arch/arm/mm/mmap.c | 7 +++---
arch/csky/abiv1/mmap.c | 3 ++-
arch/loongarch/mm/mmap.c | 5 ++--
arch/mips/mm/mmap.c | 2 +-
arch/parisc/kernel/sys_parisc.c | 5 ++--
arch/parisc/mm/hugetlbpage.c | 2 +-
arch/powerpc/mm/book3s64/slice.c | 10 +++++---
arch/s390/mm/mmap.c | 4 +--
arch/sh/mm/mmap.c | 5 ++--
arch/sparc/kernel/sys_sparc_32.c | 2 +-
arch/sparc/kernel/sys_sparc_64.c | 4 +--
arch/x86/include/asm/pgtable_64.h | 1 -
arch/x86/kernel/sys_x86_64.c | 21 +++-------------
arch/xtensa/kernel/syscall.c | 3 ++-
include/linux/sched/mm.h | 27 ++++++++-------------
mm/mmap.c | 51 ++++++++++++++++++---------------------
18 files changed, 69 insertions(+), 88 deletions(-)
---
base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
change-id: 20240830-mm-generic-shadow-stack-guard-5bc5b8d0e95d
Best regards,
--
Mark Brown <broonie@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default
2024-09-02 19:08 [PATCH 0/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
@ 2024-09-02 19:08 ` Mark Brown
2024-09-03 17:43 ` Lorenzo Stoakes
` (2 more replies)
2024-09-02 19:08 ` [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area() Mark Brown
2024-09-02 19:08 ` [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2 siblings, 3 replies; 18+ messages in thread
From: Mark Brown @ 2024-09-02 19:08 UTC (permalink / raw)
To: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes
Cc: Catalin Marinas, Will Deacon, Deepak Gupta, linux-arm-kernel,
linux-alpha, linux-kernel, linux-snps-arc, linux-arm-kernel,
linux-csky, loongarch, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-mm, Mark Brown
When we introduced arch_get_unmapped_area_vmflags() in 961148704acd
("mm: introduce arch_get_unmapped_area_vmflags()") we did so as part of
properly supporting guard pages for shadow stacks on x86_64, which uses
a custom arch_get_unmapped_area(). Equivalent features are also present
on both arm64 and RISC-V, both of which use the generic implementation
of arch_get_unmapped_area() and will require equivalent modification
there. Rather than continue to deal with having two versions of the
functions let's bite the bullet and have all implementations of
arch_get_unmapped_area() take vm_flags as a parameter.
The new parameter is currently ignored by all implementations other than
x86. The only caller that doesn't have a vm_flags available is
mm_get_unmapped_area(), as for the x86 implementation and the wrapper used
on other architectures this is modified to supply no flags.
No functional changes.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/alpha/kernel/osf_sys.c | 2 +-
arch/arc/mm/mmap.c | 3 ++-
arch/arm/mm/mmap.c | 7 ++++---
arch/csky/abiv1/mmap.c | 3 ++-
arch/loongarch/mm/mmap.c | 5 +++--
arch/mips/mm/mmap.c | 2 +-
arch/parisc/kernel/sys_parisc.c | 5 +++--
arch/parisc/mm/hugetlbpage.c | 2 +-
arch/powerpc/mm/book3s64/slice.c | 6 ++++--
arch/s390/mm/mmap.c | 4 ++--
arch/sh/mm/mmap.c | 5 +++--
arch/sparc/kernel/sys_sparc_32.c | 2 +-
arch/sparc/kernel/sys_sparc_64.c | 4 ++--
arch/x86/include/asm/pgtable_64.h | 1 -
arch/x86/kernel/sys_x86_64.c | 21 +++------------------
arch/xtensa/kernel/syscall.c | 3 ++-
include/linux/sched/mm.h | 23 ++++++++---------------
mm/mmap.c | 31 +++++++------------------------
18 files changed, 49 insertions(+), 80 deletions(-)
diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index e5f881bc8288..8886ab539273 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -1229,7 +1229,7 @@ arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
unsigned long
arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
unsigned long limit;
diff --git a/arch/arc/mm/mmap.c b/arch/arc/mm/mmap.c
index 69a915297155..2185afe8d59f 100644
--- a/arch/arc/mm/mmap.c
+++ b/arch/arc/mm/mmap.c
@@ -23,7 +23,8 @@
*/
unsigned long
arch_get_unmapped_area(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff, unsigned long flags)
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags, vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index d65d0e6ed10a..3dbb383c26d5 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -28,7 +28,8 @@
*/
unsigned long
arch_get_unmapped_area(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff, unsigned long flags)
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags, vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
@@ -78,8 +79,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long
arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
- const unsigned long len, const unsigned long pgoff,
- const unsigned long flags)
+ const unsigned long len, const unsigned long pgoff,
+ const unsigned long flags, vm_flags_t vm_flags)
{
struct vm_area_struct *vma;
struct mm_struct *mm = current->mm;
diff --git a/arch/csky/abiv1/mmap.c b/arch/csky/abiv1/mmap.c
index 7f826331d409..1047865e82a9 100644
--- a/arch/csky/abiv1/mmap.c
+++ b/arch/csky/abiv1/mmap.c
@@ -23,7 +23,8 @@
*/
unsigned long
arch_get_unmapped_area(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff, unsigned long flags)
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags, vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
diff --git a/arch/loongarch/mm/mmap.c b/arch/loongarch/mm/mmap.c
index 889030985135..914e82ff3f65 100644
--- a/arch/loongarch/mm/mmap.c
+++ b/arch/loongarch/mm/mmap.c
@@ -89,7 +89,8 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
}
unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
- unsigned long len, unsigned long pgoff, unsigned long flags)
+ unsigned long len, unsigned long pgoff, unsigned long flags,
+ vm_flags_t vm_flags)
{
return arch_get_unmapped_area_common(filp,
addr0, len, pgoff, flags, UP);
@@ -101,7 +102,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
*/
unsigned long arch_get_unmapped_area_topdown(struct file *filp,
unsigned long addr0, unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
return arch_get_unmapped_area_common(filp,
addr0, len, pgoff, flags, DOWN);
diff --git a/arch/mips/mm/mmap.c b/arch/mips/mm/mmap.c
index 7e11d7b58761..02bf5353efbd 100644
--- a/arch/mips/mm/mmap.c
+++ b/arch/mips/mm/mmap.c
@@ -110,7 +110,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
*/
unsigned long arch_get_unmapped_area_topdown(struct file *filp,
unsigned long addr0, unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
return arch_get_unmapped_area_common(filp,
addr0, len, pgoff, flags, DOWN);
diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index f7722451276e..f852fe274abe 100644
--- a/arch/parisc/kernel/sys_parisc.c
+++ b/arch/parisc/kernel/sys_parisc.c
@@ -167,7 +167,8 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
}
unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff, unsigned long flags)
+ unsigned long len, unsigned long pgoff, unsigned long flags,
+ vm_flags_t vm_flags)
{
return arch_get_unmapped_area_common(filp,
addr, len, pgoff, flags, UP);
@@ -175,7 +176,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long arch_get_unmapped_area_topdown(struct file *filp,
unsigned long addr, unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
return arch_get_unmapped_area_common(filp,
addr, len, pgoff, flags, DOWN);
diff --git a/arch/parisc/mm/hugetlbpage.c b/arch/parisc/mm/hugetlbpage.c
index 0356199bd9e7..aa664f7ddb63 100644
--- a/arch/parisc/mm/hugetlbpage.c
+++ b/arch/parisc/mm/hugetlbpage.c
@@ -40,7 +40,7 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
addr = ALIGN(addr, huge_page_size(h));
/* we need to make sure the colouring is OK */
- return arch_get_unmapped_area(file, addr, len, pgoff, flags);
+ return arch_get_unmapped_area(file, addr, len, pgoff, flags, 0);
}
diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
index ef3ce37f1bb3..ada6bf896ef8 100644
--- a/arch/powerpc/mm/book3s64/slice.c
+++ b/arch/powerpc/mm/book3s64/slice.c
@@ -637,7 +637,8 @@ unsigned long arch_get_unmapped_area(struct file *filp,
unsigned long addr,
unsigned long len,
unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags,
+ vm_flags_t vm_flags)
{
if (radix_enabled())
return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
@@ -650,7 +651,8 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,
const unsigned long addr0,
const unsigned long len,
const unsigned long pgoff,
- const unsigned long flags)
+ const unsigned long flags,
+ vm_flags_t vm_flags)
{
if (radix_enabled())
return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
index 206756946589..96efa061ce01 100644
--- a/arch/s390/mm/mmap.c
+++ b/arch/s390/mm/mmap.c
@@ -82,7 +82,7 @@ static int get_align_mask(struct file *filp, unsigned long flags)
unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
@@ -117,7 +117,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
struct vm_area_struct *vma;
struct mm_struct *mm = current->mm;
diff --git a/arch/sh/mm/mmap.c b/arch/sh/mm/mmap.c
index bee329d4149a..c442734d9b0c 100644
--- a/arch/sh/mm/mmap.c
+++ b/arch/sh/mm/mmap.c
@@ -52,7 +52,8 @@ static inline unsigned long COLOUR_ALIGN(unsigned long addr,
}
unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff, unsigned long flags)
+ unsigned long len, unsigned long pgoff, unsigned long flags,
+ vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
@@ -99,7 +100,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long
arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
const unsigned long len, const unsigned long pgoff,
- const unsigned long flags)
+ const unsigned long flags, vm_flags_t vm_flags)
{
struct vm_area_struct *vma;
struct mm_struct *mm = current->mm;
diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c
index 08a19727795c..80822f922e76 100644
--- a/arch/sparc/kernel/sys_sparc_32.c
+++ b/arch/sparc/kernel/sys_sparc_32.c
@@ -39,7 +39,7 @@ SYSCALL_DEFINE0(getpagesize)
return PAGE_SIZE; /* Possibly older binaries want 8192 on sun4's? */
}
-unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags)
+unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
{
struct vm_unmapped_area_info info = {};
diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index d9c3b34ca744..acade309dc2f 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -87,7 +87,7 @@ static inline unsigned long COLOR_ALIGN(unsigned long addr,
return base + off;
}
-unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags)
+unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct * vma;
@@ -146,7 +146,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsi
unsigned long
arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
const unsigned long len, const unsigned long pgoff,
- const unsigned long flags)
+ const unsigned long flags, vm_flags_t vm_flags)
{
struct vm_area_struct *vma;
struct mm_struct *mm = current->mm;
diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 3c4407271d08..7e9db77231ac 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -245,7 +245,6 @@ extern void cleanup_highmap(void);
#define HAVE_ARCH_UNMAPPED_AREA
#define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
-#define HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
#define PAGE_AGP PAGE_KERNEL_NOCACHE
#define HAVE_PAGE_AGP 1
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 01d7cd85ef97..87f8c9a71c49 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -121,7 +121,7 @@ static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
}
unsigned long
-arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, unsigned long len,
+arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
@@ -158,7 +158,7 @@ arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, unsigned l
}
unsigned long
-arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr0,
+arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr0,
unsigned long len, unsigned long pgoff,
unsigned long flags, vm_flags_t vm_flags)
{
@@ -228,20 +228,5 @@ arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr0,
* can happen with large stack limits and large mmap()
* allocations.
*/
- return arch_get_unmapped_area(filp, addr0, len, pgoff, flags);
-}
-
-unsigned long
-arch_get_unmapped_area(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff, unsigned long flags)
-{
- return arch_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, 0);
-}
-
-unsigned long
-arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr,
- const unsigned long len, const unsigned long pgoff,
- const unsigned long flags)
-{
- return arch_get_unmapped_area_topdown_vmflags(filp, addr, len, pgoff, flags, 0);
+ return arch_get_unmapped_area(filp, addr0, len, pgoff, flags, 0);
}
diff --git a/arch/xtensa/kernel/syscall.c b/arch/xtensa/kernel/syscall.c
index b3c2450d6f23..7f048d368dba 100644
--- a/arch/xtensa/kernel/syscall.c
+++ b/arch/xtensa/kernel/syscall.c
@@ -55,7 +55,8 @@ asmlinkage long xtensa_fadvise64_64(int fd, int advice,
#ifdef CONFIG_MMU
unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff, unsigned long flags)
+ unsigned long len, unsigned long pgoff, unsigned long flags,
+ vm_flgs_t vm_flags)
{
struct vm_area_struct *vmm;
struct vma_iterator vmi;
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 91546493c43d..c4d34abc45d4 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -179,27 +179,20 @@ static inline void mm_update_next_owner(struct mm_struct *mm)
extern void arch_pick_mmap_layout(struct mm_struct *mm,
struct rlimit *rlim_stack);
-extern unsigned long
-arch_get_unmapped_area(struct file *, unsigned long, unsigned long,
- unsigned long, unsigned long);
-extern unsigned long
+
+unsigned long
+arch_get_unmapped_area(struct file *filp, unsigned long addr,
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags, vm_flags_t vm_flags);
+unsigned long
arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff,
- unsigned long flags);
+ unsigned long len, unsigned long pgoff,
+ unsigned long flags, vm_flags_t);
unsigned long mm_get_unmapped_area(struct mm_struct *mm, struct file *filp,
unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags);
-unsigned long
-arch_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
-arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff,
- unsigned long flags, vm_flags_t);
-
unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
struct file *filp,
unsigned long addr,
diff --git a/mm/mmap.c b/mm/mmap.c
index d0dfc85b209b..7528146f886f 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1821,7 +1821,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long
arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
}
@@ -1885,38 +1885,21 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
unsigned long
arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
}
#endif
-#ifndef HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
-unsigned long
-arch_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 arch_get_unmapped_area(filp, addr, len, pgoff, flags);
-}
-
-unsigned long
-arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
- unsigned long len, unsigned long pgoff,
- unsigned long flags, vm_flags_t vm_flags)
-{
- return arch_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
-}
-#endif
-
unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm, struct file *filp,
unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags,
vm_flags_t vm_flags)
{
if (test_bit(MMF_TOPDOWN, &mm->flags))
- return arch_get_unmapped_area_topdown_vmflags(filp, addr, len, pgoff,
- flags, vm_flags);
- return arch_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, vm_flags);
+ return arch_get_unmapped_area_topdown(filp, addr, len, pgoff,
+ flags, vm_flags);
+ return arch_get_unmapped_area(filp, addr, len, pgoff, flags, vm_flags);
}
unsigned long
@@ -1978,8 +1961,8 @@ mm_get_unmapped_area(struct mm_struct *mm, struct file *file,
unsigned long pgoff, unsigned long flags)
{
if (test_bit(MMF_TOPDOWN, &mm->flags))
- return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags);
- return arch_get_unmapped_area(file, addr, len, pgoff, flags);
+ return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags, 0);
+ return arch_get_unmapped_area(file, addr, len, pgoff, flags, 0);
}
EXPORT_SYMBOL(mm_get_unmapped_area);
--
2.39.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area()
2024-09-02 19:08 [PATCH 0/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2024-09-02 19:08 ` [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default Mark Brown
@ 2024-09-02 19:08 ` Mark Brown
2024-09-03 17:44 ` Lorenzo Stoakes
` (3 more replies)
2024-09-02 19:08 ` [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2 siblings, 4 replies; 18+ messages in thread
From: Mark Brown @ 2024-09-02 19:08 UTC (permalink / raw)
To: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes
Cc: Catalin Marinas, Will Deacon, Deepak Gupta, linux-arm-kernel,
linux-alpha, linux-kernel, linux-snps-arc, linux-arm-kernel,
linux-csky, loongarch, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-mm, Mark Brown
In preparation for using vm_flags to ensure guard pages for shadow stacks
supply them as an argument to generic_get_unmapped_area(). The only user
outside of the core code is the PowerPC book3s64 implementation which is
trivially wrapping the generic implementation in the radix_enabled() case.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/powerpc/mm/book3s64/slice.c | 4 ++--
include/linux/sched/mm.h | 4 ++--
mm/mmap.c | 10 ++++++----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
index ada6bf896ef8..87307d0fc3b8 100644
--- a/arch/powerpc/mm/book3s64/slice.c
+++ b/arch/powerpc/mm/book3s64/slice.c
@@ -641,7 +641,7 @@ unsigned long arch_get_unmapped_area(struct file *filp,
vm_flags_t vm_flags)
{
if (radix_enabled())
- return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
+ return generic_get_unmapped_area(filp, addr, len, pgoff, flags, vm_flags);
return slice_get_unmapped_area(addr, len, flags,
mm_ctx_user_psize(¤t->mm->context), 0);
@@ -655,7 +655,7 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,
vm_flags_t vm_flags)
{
if (radix_enabled())
- return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
+ return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags, vm_flags);
return slice_get_unmapped_area(addr0, len, flags,
mm_ctx_user_psize(¤t->mm->context), 1);
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index c4d34abc45d4..07bb8d4181d7 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -204,11 +204,11 @@ unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
unsigned long
generic_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags);
+ unsigned long flags, vm_flags_t vm_flags);
unsigned long
generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags);
+ unsigned long flags, vm_flags_t vm_flags);
#else
static inline void arch_pick_mmap_layout(struct mm_struct *mm,
struct rlimit *rlim_stack) {}
diff --git a/mm/mmap.c b/mm/mmap.c
index 7528146f886f..b06ba847c96e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1789,7 +1789,7 @@ unsigned long vm_unmapped_area(struct vm_unmapped_area_info *info)
unsigned long
generic_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma, *prev;
@@ -1823,7 +1823,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
unsigned long flags, vm_flags_t vm_flags)
{
- return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
+ return generic_get_unmapped_area(filp, addr, len, pgoff, flags,
+ vm_flags);
}
#endif
@@ -1834,7 +1835,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
unsigned long
generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
- unsigned long flags)
+ unsigned long flags, vm_flags_t vm_flags)
{
struct vm_area_struct *vma, *prev;
struct mm_struct *mm = current->mm;
@@ -1887,7 +1888,8 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
unsigned long len, unsigned long pgoff,
unsigned long flags, vm_flags_t vm_flags)
{
- return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
+ return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags,
+ vm_flags);
}
#endif
--
2.39.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-02 19:08 [PATCH 0/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2024-09-02 19:08 ` [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default Mark Brown
2024-09-02 19:08 ` [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area() Mark Brown
@ 2024-09-02 19:08 ` Mark Brown
2024-09-03 17:49 ` Lorenzo Stoakes
` (2 more replies)
2 siblings, 3 replies; 18+ messages in thread
From: Mark Brown @ 2024-09-02 19:08 UTC (permalink / raw)
To: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes
Cc: Catalin Marinas, Will Deacon, Deepak Gupta, linux-arm-kernel,
linux-alpha, linux-kernel, linux-snps-arc, linux-arm-kernel,
linux-csky, loongarch, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-mm, Mark Brown, Rick Edgecombe
As covered in the commit log for c44357c2e76b ("x86/mm: care about shadow
stack guard gap during placement") our current mmap() implementation does
not take care to ensure that a new mapping isn't placed with existing
mappings inside it's own guard gaps. This is particularly important for
shadow stacks since if two shadow stacks end up getting placed adjacent to
each other then they can overflow into each other which weakens the
protection offered by the feature.
On x86 there is a custom arch_get_unmapped_area() which was updated by the
above commit to cover this case by specifying a start_gap for allocations
with VM_SHADOW_STACK. Both arm64 and RISC-V have equivalent features and
use the generic implementation of arch_get_unmapped_area() so let's make
the equivalent change there so they also don't get shadow stack pages
placed without guard pages.
Architectures which do not have this feature will define VM_SHADOW_STACK
to VM_NONE and hence be unaffected.
Suggested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
mm/mmap.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/mm/mmap.c b/mm/mmap.c
index b06ba847c96e..902c482b6084 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1753,6 +1753,14 @@ static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
return gap;
}
+static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
+{
+ if (vm_flags & VM_SHADOW_STACK)
+ return PAGE_SIZE;
+
+ return 0;
+}
+
/*
* Search for an unmapped address range.
*
@@ -1814,6 +1822,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
info.length = len;
info.low_limit = mm->mmap_base;
info.high_limit = mmap_end;
+ info.start_gap = stack_guard_placement(vm_flags);
return vm_unmapped_area(&info);
}
@@ -1863,6 +1872,7 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
info.length = len;
info.low_limit = PAGE_SIZE;
info.high_limit = arch_get_mmap_base(addr, mm->mmap_base);
+ info.start_gap = stack_guard_placement(vm_flags);
addr = vm_unmapped_area(&info);
/*
--
2.39.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default
2024-09-02 19:08 ` [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default Mark Brown
@ 2024-09-03 17:43 ` Lorenzo Stoakes
2024-09-03 19:35 ` Liam R. Howlett
2024-09-03 19:50 ` Helge Deller
2 siblings, 0 replies; 18+ messages in thread
From: Lorenzo Stoakes @ 2024-09-03 17:43 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm
On Mon, Sep 02, 2024 at 08:08:13PM GMT, Mark Brown wrote:
> When we introduced arch_get_unmapped_area_vmflags() in 961148704acd
> ("mm: introduce arch_get_unmapped_area_vmflags()") we did so as part of
> properly supporting guard pages for shadow stacks on x86_64, which uses
> a custom arch_get_unmapped_area(). Equivalent features are also present
> on both arm64 and RISC-V, both of which use the generic implementation
> of arch_get_unmapped_area() and will require equivalent modification
> there. Rather than continue to deal with having two versions of the
> functions let's bite the bullet and have all implementations of
> arch_get_unmapped_area() take vm_flags as a parameter.
>
> The new parameter is currently ignored by all implementations other than
> x86. The only caller that doesn't have a vm_flags available is
> mm_get_unmapped_area(), as for the x86 implementation and the wrapper used
> on other architectures this is modified to supply no flags.
>
> No functional changes.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
mm/mmap.c bit looks reasonable to me! Nice cleanup! :)
Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> (for mm/mmap.c part)
> ---
> arch/alpha/kernel/osf_sys.c | 2 +-
> arch/arc/mm/mmap.c | 3 ++-
> arch/arm/mm/mmap.c | 7 ++++---
> arch/csky/abiv1/mmap.c | 3 ++-
> arch/loongarch/mm/mmap.c | 5 +++--
> arch/mips/mm/mmap.c | 2 +-
> arch/parisc/kernel/sys_parisc.c | 5 +++--
> arch/parisc/mm/hugetlbpage.c | 2 +-
> arch/powerpc/mm/book3s64/slice.c | 6 ++++--
> arch/s390/mm/mmap.c | 4 ++--
> arch/sh/mm/mmap.c | 5 +++--
> arch/sparc/kernel/sys_sparc_32.c | 2 +-
> arch/sparc/kernel/sys_sparc_64.c | 4 ++--
> arch/x86/include/asm/pgtable_64.h | 1 -
> arch/x86/kernel/sys_x86_64.c | 21 +++------------------
> arch/xtensa/kernel/syscall.c | 3 ++-
> include/linux/sched/mm.h | 23 ++++++++---------------
> mm/mmap.c | 31 +++++++------------------------
> 18 files changed, 49 insertions(+), 80 deletions(-)
>
> diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> index e5f881bc8288..8886ab539273 100644
> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1229,7 +1229,7 @@ arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> unsigned long limit;
>
> diff --git a/arch/arc/mm/mmap.c b/arch/arc/mm/mmap.c
> index 69a915297155..2185afe8d59f 100644
> --- a/arch/arc/mm/mmap.c
> +++ b/arch/arc/mm/mmap.c
> @@ -23,7 +23,8 @@
> */
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
> index d65d0e6ed10a..3dbb383c26d5 100644
> --- a/arch/arm/mm/mmap.c
> +++ b/arch/arm/mm/mmap.c
> @@ -28,7 +28,8 @@
> */
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> @@ -78,8 +79,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> - const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long len, const unsigned long pgoff,
> + const unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/csky/abiv1/mmap.c b/arch/csky/abiv1/mmap.c
> index 7f826331d409..1047865e82a9 100644
> --- a/arch/csky/abiv1/mmap.c
> +++ b/arch/csky/abiv1/mmap.c
> @@ -23,7 +23,8 @@
> */
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> diff --git a/arch/loongarch/mm/mmap.c b/arch/loongarch/mm/mmap.c
> index 889030985135..914e82ff3f65 100644
> --- a/arch/loongarch/mm/mmap.c
> +++ b/arch/loongarch/mm/mmap.c
> @@ -89,7 +89,8 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
> }
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr0, len, pgoff, flags, UP);
> @@ -101,7 +102,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
> */
> unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> unsigned long addr0, unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr0, len, pgoff, flags, DOWN);
> diff --git a/arch/mips/mm/mmap.c b/arch/mips/mm/mmap.c
> index 7e11d7b58761..02bf5353efbd 100644
> --- a/arch/mips/mm/mmap.c
> +++ b/arch/mips/mm/mmap.c
> @@ -110,7 +110,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
> */
> unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> unsigned long addr0, unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr0, len, pgoff, flags, DOWN);
> diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
> index f7722451276e..f852fe274abe 100644
> --- a/arch/parisc/kernel/sys_parisc.c
> +++ b/arch/parisc/kernel/sys_parisc.c
> @@ -167,7 +167,8 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
> }
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr, len, pgoff, flags, UP);
> @@ -175,7 +176,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
>
> unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> unsigned long addr, unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr, len, pgoff, flags, DOWN);
> diff --git a/arch/parisc/mm/hugetlbpage.c b/arch/parisc/mm/hugetlbpage.c
> index 0356199bd9e7..aa664f7ddb63 100644
> --- a/arch/parisc/mm/hugetlbpage.c
> +++ b/arch/parisc/mm/hugetlbpage.c
> @@ -40,7 +40,7 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> addr = ALIGN(addr, huge_page_size(h));
>
> /* we need to make sure the colouring is OK */
> - return arch_get_unmapped_area(file, addr, len, pgoff, flags);
> + return arch_get_unmapped_area(file, addr, len, pgoff, flags, 0);
> }
>
>
> diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
> index ef3ce37f1bb3..ada6bf896ef8 100644
> --- a/arch/powerpc/mm/book3s64/slice.c
> +++ b/arch/powerpc/mm/book3s64/slice.c
> @@ -637,7 +637,8 @@ unsigned long arch_get_unmapped_area(struct file *filp,
> unsigned long addr,
> unsigned long len,
> unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags,
> + vm_flags_t vm_flags)
> {
> if (radix_enabled())
> return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> @@ -650,7 +651,8 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> const unsigned long addr0,
> const unsigned long len,
> const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long flags,
> + vm_flags_t vm_flags)
> {
> if (radix_enabled())
> return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
> diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
> index 206756946589..96efa061ce01 100644
> --- a/arch/s390/mm/mmap.c
> +++ b/arch/s390/mm/mmap.c
> @@ -82,7 +82,7 @@ static int get_align_mask(struct file *filp, unsigned long flags)
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> @@ -117,7 +117,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
>
> unsigned long arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/sh/mm/mmap.c b/arch/sh/mm/mmap.c
> index bee329d4149a..c442734d9b0c 100644
> --- a/arch/sh/mm/mmap.c
> +++ b/arch/sh/mm/mmap.c
> @@ -52,7 +52,8 @@ static inline unsigned long COLOUR_ALIGN(unsigned long addr,
> }
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> @@ -99,7 +100,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c
> index 08a19727795c..80822f922e76 100644
> --- a/arch/sparc/kernel/sys_sparc_32.c
> +++ b/arch/sparc/kernel/sys_sparc_32.c
> @@ -39,7 +39,7 @@ SYSCALL_DEFINE0(getpagesize)
> return PAGE_SIZE; /* Possibly older binaries want 8192 on sun4's? */
> }
>
> -unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags)
> +unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_unmapped_area_info info = {};
>
> diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
> index d9c3b34ca744..acade309dc2f 100644
> --- a/arch/sparc/kernel/sys_sparc_64.c
> +++ b/arch/sparc/kernel/sys_sparc_64.c
> @@ -87,7 +87,7 @@ static inline unsigned long COLOR_ALIGN(unsigned long addr,
> return base + off;
> }
>
> -unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags)
> +unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct * vma;
> @@ -146,7 +146,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsi
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
> index 3c4407271d08..7e9db77231ac 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -245,7 +245,6 @@ extern void cleanup_highmap(void);
>
> #define HAVE_ARCH_UNMAPPED_AREA
> #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
> -#define HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
>
> #define PAGE_AGP PAGE_KERNEL_NOCACHE
> #define HAVE_PAGE_AGP 1
> diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> index 01d7cd85ef97..87f8c9a71c49 100644
> --- a/arch/x86/kernel/sys_x86_64.c
> +++ b/arch/x86/kernel/sys_x86_64.c
> @@ -121,7 +121,7 @@ static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
> }
>
> unsigned long
> -arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, unsigned long len,
> +arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> @@ -158,7 +158,7 @@ arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, unsigned l
> }
>
> unsigned long
> -arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr0,
> +arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr0,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> @@ -228,20 +228,5 @@ arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr0,
> * can happen with large stack limits and large mmap()
> * allocations.
> */
> - return arch_get_unmapped_area(filp, addr0, len, pgoff, flags);
> -}
> -
> -unsigned long
> -arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> -{
> - return arch_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, 0);
> -}
> -
> -unsigned long
> -arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr,
> - const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> -{
> - return arch_get_unmapped_area_topdown_vmflags(filp, addr, len, pgoff, flags, 0);
> + return arch_get_unmapped_area(filp, addr0, len, pgoff, flags, 0);
> }
> diff --git a/arch/xtensa/kernel/syscall.c b/arch/xtensa/kernel/syscall.c
> index b3c2450d6f23..7f048d368dba 100644
> --- a/arch/xtensa/kernel/syscall.c
> +++ b/arch/xtensa/kernel/syscall.c
> @@ -55,7 +55,8 @@ asmlinkage long xtensa_fadvise64_64(int fd, int advice,
>
> #ifdef CONFIG_MMU
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flgs_t vm_flags)
> {
> struct vm_area_struct *vmm;
> struct vma_iterator vmi;
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 91546493c43d..c4d34abc45d4 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -179,27 +179,20 @@ static inline void mm_update_next_owner(struct mm_struct *mm)
>
> extern void arch_pick_mmap_layout(struct mm_struct *mm,
> struct rlimit *rlim_stack);
> -extern unsigned long
> -arch_get_unmapped_area(struct file *, unsigned long, unsigned long,
> - unsigned long, unsigned long);
> -extern unsigned long
> +
> +unsigned long
> +arch_get_unmapped_area(struct file *filp, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags);
> +unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t);
>
> unsigned long mm_get_unmapped_area(struct mm_struct *mm, struct file *filp,
> unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags);
>
> -unsigned long
> -arch_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
> -arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff,
> - unsigned long flags, vm_flags_t);
> -
> unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
> struct file *filp,
> unsigned long addr,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d0dfc85b209b..7528146f886f 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1821,7 +1821,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> }
> @@ -1885,38 +1885,21 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> }
> #endif
>
> -#ifndef HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
> -unsigned long
> -arch_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 arch_get_unmapped_area(filp, addr, len, pgoff, flags);
> -}
> -
> -unsigned long
> -arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff,
> - unsigned long flags, vm_flags_t vm_flags)
> -{
> - return arch_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> -}
> -#endif
> -
> unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm, struct file *filp,
> unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags,
> vm_flags_t vm_flags)
> {
> if (test_bit(MMF_TOPDOWN, &mm->flags))
> - return arch_get_unmapped_area_topdown_vmflags(filp, addr, len, pgoff,
> - flags, vm_flags);
> - return arch_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, vm_flags);
> + return arch_get_unmapped_area_topdown(filp, addr, len, pgoff,
> + flags, vm_flags);
> + return arch_get_unmapped_area(filp, addr, len, pgoff, flags, vm_flags);
> }
Kind of a pity to keep the _vmflags() variants when in similarly-named
arch_get_unmapped...() functions we drop it, but I guess it would get churny to
try to change mm_get_unmapped_area() as I see that being called in various places.
>
> unsigned long
> @@ -1978,8 +1961,8 @@ mm_get_unmapped_area(struct mm_struct *mm, struct file *file,
> unsigned long pgoff, unsigned long flags)
> {
> if (test_bit(MMF_TOPDOWN, &mm->flags))
> - return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags);
> - return arch_get_unmapped_area(file, addr, len, pgoff, flags);
> + return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags, 0);
> + return arch_get_unmapped_area(file, addr, len, pgoff, flags, 0);
> }
> EXPORT_SYMBOL(mm_get_unmapped_area);
>
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area()
2024-09-02 19:08 ` [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area() Mark Brown
@ 2024-09-03 17:44 ` Lorenzo Stoakes
2024-09-03 19:37 ` Liam R. Howlett
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Lorenzo Stoakes @ 2024-09-03 17:44 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm
On Mon, Sep 02, 2024 at 08:08:14PM GMT, Mark Brown wrote:
> In preparation for using vm_flags to ensure guard pages for shadow stacks
> supply them as an argument to generic_get_unmapped_area(). The only user
> outside of the core code is the PowerPC book3s64 implementation which is
> trivially wrapping the generic implementation in the radix_enabled() case.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> (for mm/mmap.c part)
> ---
> arch/powerpc/mm/book3s64/slice.c | 4 ++--
> include/linux/sched/mm.h | 4 ++--
> mm/mmap.c | 10 ++++++----
> 3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
> index ada6bf896ef8..87307d0fc3b8 100644
> --- a/arch/powerpc/mm/book3s64/slice.c
> +++ b/arch/powerpc/mm/book3s64/slice.c
> @@ -641,7 +641,7 @@ unsigned long arch_get_unmapped_area(struct file *filp,
> vm_flags_t vm_flags)
> {
> if (radix_enabled())
> - return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area(filp, addr, len, pgoff, flags, vm_flags);
>
> return slice_get_unmapped_area(addr, len, flags,
> mm_ctx_user_psize(¤t->mm->context), 0);
> @@ -655,7 +655,7 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> vm_flags_t vm_flags)
> {
> if (radix_enabled())
> - return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
> + return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags, vm_flags);
>
> return slice_get_unmapped_area(addr0, len, flags,
> mm_ctx_user_psize(¤t->mm->context), 1);
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index c4d34abc45d4..07bb8d4181d7 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -204,11 +204,11 @@ unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
> unsigned long
> generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long flags, vm_flags_t vm_flags);
> unsigned long
> generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long flags, vm_flags_t vm_flags);
> #else
> static inline void arch_pick_mmap_layout(struct mm_struct *mm,
> struct rlimit *rlim_stack) {}
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 7528146f886f..b06ba847c96e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1789,7 +1789,7 @@ unsigned long vm_unmapped_area(struct vm_unmapped_area_info *info)
> unsigned long
> generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma, *prev;
> @@ -1823,7 +1823,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> - return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area(filp, addr, len, pgoff, flags,
> + vm_flags);
> }
> #endif
>
> @@ -1834,7 +1835,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long
> generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma, *prev;
> struct mm_struct *mm = current->mm;
> @@ -1887,7 +1888,8 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> - return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags,
> + vm_flags);
> }
> #endif
>
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-02 19:08 ` [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
@ 2024-09-03 17:49 ` Lorenzo Stoakes
2024-09-03 18:20 ` Mark Brown
2024-09-03 19:41 ` Liam R. Howlett
2024-09-04 18:51 ` Deepak Gupta
2 siblings, 1 reply; 18+ messages in thread
From: Lorenzo Stoakes @ 2024-09-03 17:49 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm, Rick Edgecombe
On Mon, Sep 02, 2024 at 08:08:15PM GMT, Mark Brown wrote:
> As covered in the commit log for c44357c2e76b ("x86/mm: care about shadow
> stack guard gap during placement") our current mmap() implementation does
> not take care to ensure that a new mapping isn't placed with existing
> mappings inside it's own guard gaps. This is particularly important for
> shadow stacks since if two shadow stacks end up getting placed adjacent to
> each other then they can overflow into each other which weakens the
> protection offered by the feature.
>
> On x86 there is a custom arch_get_unmapped_area() which was updated by the
> above commit to cover this case by specifying a start_gap for allocations
> with VM_SHADOW_STACK. Both arm64 and RISC-V have equivalent features and
> use the generic implementation of arch_get_unmapped_area() so let's make
> the equivalent change there so they also don't get shadow stack pages
> placed without guard pages.
Don't you need to unwind that change in x86 now you're doing it in generic code?
>
> Architectures which do not have this feature will define VM_SHADOW_STACK
> to VM_NONE and hence be unaffected.
Nice.
>
> Suggested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> mm/mmap.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index b06ba847c96e..902c482b6084 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1753,6 +1753,14 @@ static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
> return gap;
> }
>
Would be nice to take some of the context in commit message as a short comment
describing the function. I mean it's kinda trivially self-documenting obviously,
but it's useful context for somebody wanting to understand _why_ we are doing
this at a glance.
> +static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
> +{
> + if (vm_flags & VM_SHADOW_STACK)
> + return PAGE_SIZE;
> +
> + return 0;
> +}
> +
> /*
> * Search for an unmapped address range.
> *
> @@ -1814,6 +1822,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
> info.length = len;
> info.low_limit = mm->mmap_base;
> info.high_limit = mmap_end;
> + info.start_gap = stack_guard_placement(vm_flags);
> return vm_unmapped_area(&info);
> }
>
> @@ -1863,6 +1872,7 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> info.length = len;
> info.low_limit = PAGE_SIZE;
> info.high_limit = arch_get_mmap_base(addr, mm->mmap_base);
> + info.start_gap = stack_guard_placement(vm_flags);
> addr = vm_unmapped_area(&info);
>
> /*
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-03 17:49 ` Lorenzo Stoakes
@ 2024-09-03 18:20 ` Mark Brown
2024-09-03 19:24 ` Lorenzo Stoakes
0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2024-09-03 18:20 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm, Rick Edgecombe
[-- Attachment #1: Type: text/plain, Size: 865 bytes --]
On Tue, Sep 03, 2024 at 06:49:46PM +0100, Lorenzo Stoakes wrote:
> On Mon, Sep 02, 2024 at 08:08:15PM GMT, Mark Brown wrote:
> > On x86 there is a custom arch_get_unmapped_area() which was updated by the
> > above commit to cover this case by specifying a start_gap for allocations
> > with VM_SHADOW_STACK. Both arm64 and RISC-V have equivalent features and
> > use the generic implementation of arch_get_unmapped_area() so let's make
> > the equivalent change there so they also don't get shadow stack pages
> > placed without guard pages.
> Don't you need to unwind that change in x86 now you're doing it in generic code?
No, x86 had a preexisting custom implementation for some other reason
(hence the "updated by the above commit" part above) - the shadow stack
support would most likely have been added in the core in the first place
were it not for that.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-03 18:20 ` Mark Brown
@ 2024-09-03 19:24 ` Lorenzo Stoakes
0 siblings, 0 replies; 18+ messages in thread
From: Lorenzo Stoakes @ 2024-09-03 19:24 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm, Rick Edgecombe
On Tue, Sep 03, 2024 at 07:20:02PM GMT, Mark Brown wrote:
> On Tue, Sep 03, 2024 at 06:49:46PM +0100, Lorenzo Stoakes wrote:
> > On Mon, Sep 02, 2024 at 08:08:15PM GMT, Mark Brown wrote:
>
> > > On x86 there is a custom arch_get_unmapped_area() which was updated by the
> > > above commit to cover this case by specifying a start_gap for allocations
> > > with VM_SHADOW_STACK. Both arm64 and RISC-V have equivalent features and
> > > use the generic implementation of arch_get_unmapped_area() so let's make
> > > the equivalent change there so they also don't get shadow stack pages
> > > placed without guard pages.
>
> > Don't you need to unwind that change in x86 now you're doing it in generic code?
>
> No, x86 had a preexisting custom implementation for some other reason
> (hence the "updated by the above commit" part above) - the shadow stack
> support would most likely have been added in the core in the first place
> were it not for that.
Oh yeah missed that!
Other than comment nice-to-have this seems fine:
Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default
2024-09-02 19:08 ` [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default Mark Brown
2024-09-03 17:43 ` Lorenzo Stoakes
@ 2024-09-03 19:35 ` Liam R. Howlett
2024-09-03 19:50 ` Helge Deller
2 siblings, 0 replies; 18+ messages in thread
From: Liam R. Howlett @ 2024-09-03 19:35 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Vlastimil Babka, Lorenzo Stoakes, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm
* Mark Brown <broonie@kernel.org> [240902 15:09]:
> When we introduced arch_get_unmapped_area_vmflags() in 961148704acd
> ("mm: introduce arch_get_unmapped_area_vmflags()") we did so as part of
> properly supporting guard pages for shadow stacks on x86_64, which uses
> a custom arch_get_unmapped_area(). Equivalent features are also present
> on both arm64 and RISC-V, both of which use the generic implementation
> of arch_get_unmapped_area() and will require equivalent modification
> there. Rather than continue to deal with having two versions of the
> functions let's bite the bullet and have all implementations of
> arch_get_unmapped_area() take vm_flags as a parameter.
>
> The new parameter is currently ignored by all implementations other than
> x86. The only caller that doesn't have a vm_flags available is
> mm_get_unmapped_area(), as for the x86 implementation and the wrapper used
> on other architectures this is modified to supply no flags.
>
> No functional changes.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
I don't love sparc32/sparc64 requires a wide screen monitor, but it
already broke the 80 char limit.
Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
> ---
> arch/alpha/kernel/osf_sys.c | 2 +-
> arch/arc/mm/mmap.c | 3 ++-
> arch/arm/mm/mmap.c | 7 ++++---
> arch/csky/abiv1/mmap.c | 3 ++-
> arch/loongarch/mm/mmap.c | 5 +++--
> arch/mips/mm/mmap.c | 2 +-
> arch/parisc/kernel/sys_parisc.c | 5 +++--
> arch/parisc/mm/hugetlbpage.c | 2 +-
> arch/powerpc/mm/book3s64/slice.c | 6 ++++--
> arch/s390/mm/mmap.c | 4 ++--
> arch/sh/mm/mmap.c | 5 +++--
> arch/sparc/kernel/sys_sparc_32.c | 2 +-
> arch/sparc/kernel/sys_sparc_64.c | 4 ++--
> arch/x86/include/asm/pgtable_64.h | 1 -
> arch/x86/kernel/sys_x86_64.c | 21 +++------------------
> arch/xtensa/kernel/syscall.c | 3 ++-
> include/linux/sched/mm.h | 23 ++++++++---------------
> mm/mmap.c | 31 +++++++------------------------
> 18 files changed, 49 insertions(+), 80 deletions(-)
>
> diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
> index e5f881bc8288..8886ab539273 100644
> --- a/arch/alpha/kernel/osf_sys.c
> +++ b/arch/alpha/kernel/osf_sys.c
> @@ -1229,7 +1229,7 @@ arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> unsigned long limit;
>
> diff --git a/arch/arc/mm/mmap.c b/arch/arc/mm/mmap.c
> index 69a915297155..2185afe8d59f 100644
> --- a/arch/arc/mm/mmap.c
> +++ b/arch/arc/mm/mmap.c
> @@ -23,7 +23,8 @@
> */
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
> index d65d0e6ed10a..3dbb383c26d5 100644
> --- a/arch/arm/mm/mmap.c
> +++ b/arch/arm/mm/mmap.c
> @@ -28,7 +28,8 @@
> */
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> @@ -78,8 +79,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
>
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> - const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long len, const unsigned long pgoff,
> + const unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/csky/abiv1/mmap.c b/arch/csky/abiv1/mmap.c
> index 7f826331d409..1047865e82a9 100644
> --- a/arch/csky/abiv1/mmap.c
> +++ b/arch/csky/abiv1/mmap.c
> @@ -23,7 +23,8 @@
> */
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> diff --git a/arch/loongarch/mm/mmap.c b/arch/loongarch/mm/mmap.c
> index 889030985135..914e82ff3f65 100644
> --- a/arch/loongarch/mm/mmap.c
> +++ b/arch/loongarch/mm/mmap.c
> @@ -89,7 +89,8 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
> }
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr0, len, pgoff, flags, UP);
> @@ -101,7 +102,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
> */
> unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> unsigned long addr0, unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr0, len, pgoff, flags, DOWN);
> diff --git a/arch/mips/mm/mmap.c b/arch/mips/mm/mmap.c
> index 7e11d7b58761..02bf5353efbd 100644
> --- a/arch/mips/mm/mmap.c
> +++ b/arch/mips/mm/mmap.c
> @@ -110,7 +110,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr0,
> */
> unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> unsigned long addr0, unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr0, len, pgoff, flags, DOWN);
> diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
> index f7722451276e..f852fe274abe 100644
> --- a/arch/parisc/kernel/sys_parisc.c
> +++ b/arch/parisc/kernel/sys_parisc.c
> @@ -167,7 +167,8 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
> }
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr, len, pgoff, flags, UP);
> @@ -175,7 +176,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
>
> unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> unsigned long addr, unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return arch_get_unmapped_area_common(filp,
> addr, len, pgoff, flags, DOWN);
> diff --git a/arch/parisc/mm/hugetlbpage.c b/arch/parisc/mm/hugetlbpage.c
> index 0356199bd9e7..aa664f7ddb63 100644
> --- a/arch/parisc/mm/hugetlbpage.c
> +++ b/arch/parisc/mm/hugetlbpage.c
> @@ -40,7 +40,7 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> addr = ALIGN(addr, huge_page_size(h));
>
> /* we need to make sure the colouring is OK */
> - return arch_get_unmapped_area(file, addr, len, pgoff, flags);
> + return arch_get_unmapped_area(file, addr, len, pgoff, flags, 0);
> }
>
>
> diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
> index ef3ce37f1bb3..ada6bf896ef8 100644
> --- a/arch/powerpc/mm/book3s64/slice.c
> +++ b/arch/powerpc/mm/book3s64/slice.c
> @@ -637,7 +637,8 @@ unsigned long arch_get_unmapped_area(struct file *filp,
> unsigned long addr,
> unsigned long len,
> unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags,
> + vm_flags_t vm_flags)
> {
> if (radix_enabled())
> return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> @@ -650,7 +651,8 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> const unsigned long addr0,
> const unsigned long len,
> const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long flags,
> + vm_flags_t vm_flags)
> {
> if (radix_enabled())
> return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
> diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
> index 206756946589..96efa061ce01 100644
> --- a/arch/s390/mm/mmap.c
> +++ b/arch/s390/mm/mmap.c
> @@ -82,7 +82,7 @@ static int get_align_mask(struct file *filp, unsigned long flags)
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> @@ -117,7 +117,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
>
> unsigned long arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/sh/mm/mmap.c b/arch/sh/mm/mmap.c
> index bee329d4149a..c442734d9b0c 100644
> --- a/arch/sh/mm/mmap.c
> +++ b/arch/sh/mm/mmap.c
> @@ -52,7 +52,8 @@ static inline unsigned long COLOUR_ALIGN(unsigned long addr,
> }
>
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> @@ -99,7 +100,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c
> index 08a19727795c..80822f922e76 100644
> --- a/arch/sparc/kernel/sys_sparc_32.c
> +++ b/arch/sparc/kernel/sys_sparc_32.c
> @@ -39,7 +39,7 @@ SYSCALL_DEFINE0(getpagesize)
> return PAGE_SIZE; /* Possibly older binaries want 8192 on sun4's? */
> }
>
> -unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags)
> +unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_unmapped_area_info info = {};
>
> diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
> index d9c3b34ca744..acade309dc2f 100644
> --- a/arch/sparc/kernel/sys_sparc_64.c
> +++ b/arch/sparc/kernel/sys_sparc_64.c
> @@ -87,7 +87,7 @@ static inline unsigned long COLOR_ALIGN(unsigned long addr,
> return base + off;
> }
>
> -unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags)
> +unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct * vma;
> @@ -146,7 +146,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsi
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
> const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> + const unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma;
> struct mm_struct *mm = current->mm;
> diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
> index 3c4407271d08..7e9db77231ac 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -245,7 +245,6 @@ extern void cleanup_highmap(void);
>
> #define HAVE_ARCH_UNMAPPED_AREA
> #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
> -#define HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
>
> #define PAGE_AGP PAGE_KERNEL_NOCACHE
> #define HAVE_PAGE_AGP 1
> diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> index 01d7cd85ef97..87f8c9a71c49 100644
> --- a/arch/x86/kernel/sys_x86_64.c
> +++ b/arch/x86/kernel/sys_x86_64.c
> @@ -121,7 +121,7 @@ static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
> }
>
> unsigned long
> -arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, unsigned long len,
> +arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> @@ -158,7 +158,7 @@ arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, unsigned l
> }
>
> unsigned long
> -arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr0,
> +arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr0,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> @@ -228,20 +228,5 @@ arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr0,
> * can happen with large stack limits and large mmap()
> * allocations.
> */
> - return arch_get_unmapped_area(filp, addr0, len, pgoff, flags);
> -}
> -
> -unsigned long
> -arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> -{
> - return arch_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, 0);
> -}
> -
> -unsigned long
> -arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr,
> - const unsigned long len, const unsigned long pgoff,
> - const unsigned long flags)
> -{
> - return arch_get_unmapped_area_topdown_vmflags(filp, addr, len, pgoff, flags, 0);
> + return arch_get_unmapped_area(filp, addr0, len, pgoff, flags, 0);
> }
> diff --git a/arch/xtensa/kernel/syscall.c b/arch/xtensa/kernel/syscall.c
> index b3c2450d6f23..7f048d368dba 100644
> --- a/arch/xtensa/kernel/syscall.c
> +++ b/arch/xtensa/kernel/syscall.c
> @@ -55,7 +55,8 @@ asmlinkage long xtensa_fadvise64_64(int fd, int advice,
>
> #ifdef CONFIG_MMU
> unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff, unsigned long flags)
> + unsigned long len, unsigned long pgoff, unsigned long flags,
> + vm_flgs_t vm_flags)
> {
> struct vm_area_struct *vmm;
> struct vma_iterator vmi;
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 91546493c43d..c4d34abc45d4 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -179,27 +179,20 @@ static inline void mm_update_next_owner(struct mm_struct *mm)
>
> extern void arch_pick_mmap_layout(struct mm_struct *mm,
> struct rlimit *rlim_stack);
> -extern unsigned long
> -arch_get_unmapped_area(struct file *, unsigned long, unsigned long,
> - unsigned long, unsigned long);
> -extern unsigned long
> +
> +unsigned long
> +arch_get_unmapped_area(struct file *filp, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags);
> +unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t);
>
> unsigned long mm_get_unmapped_area(struct mm_struct *mm, struct file *filp,
> unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags);
>
> -unsigned long
> -arch_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
> -arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff,
> - unsigned long flags, vm_flags_t);
> -
> unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
> struct file *filp,
> unsigned long addr,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d0dfc85b209b..7528146f886f 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1821,7 +1821,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long
> arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> }
> @@ -1885,38 +1885,21 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long
> arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> }
> #endif
>
> -#ifndef HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
> -unsigned long
> -arch_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 arch_get_unmapped_area(filp, addr, len, pgoff, flags);
> -}
> -
> -unsigned long
> -arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
> - unsigned long len, unsigned long pgoff,
> - unsigned long flags, vm_flags_t vm_flags)
> -{
> - return arch_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> -}
> -#endif
> -
> unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm, struct file *filp,
> unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags,
> vm_flags_t vm_flags)
> {
> if (test_bit(MMF_TOPDOWN, &mm->flags))
> - return arch_get_unmapped_area_topdown_vmflags(filp, addr, len, pgoff,
> - flags, vm_flags);
> - return arch_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, vm_flags);
> + return arch_get_unmapped_area_topdown(filp, addr, len, pgoff,
> + flags, vm_flags);
> + return arch_get_unmapped_area(filp, addr, len, pgoff, flags, vm_flags);
> }
>
> unsigned long
> @@ -1978,8 +1961,8 @@ mm_get_unmapped_area(struct mm_struct *mm, struct file *file,
> unsigned long pgoff, unsigned long flags)
> {
> if (test_bit(MMF_TOPDOWN, &mm->flags))
> - return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags);
> - return arch_get_unmapped_area(file, addr, len, pgoff, flags);
> + return arch_get_unmapped_area_topdown(file, addr, len, pgoff, flags, 0);
> + return arch_get_unmapped_area(file, addr, len, pgoff, flags, 0);
> }
> EXPORT_SYMBOL(mm_get_unmapped_area);
>
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area()
2024-09-02 19:08 ` [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area() Mark Brown
2024-09-03 17:44 ` Lorenzo Stoakes
@ 2024-09-03 19:37 ` Liam R. Howlett
2024-09-04 4:13 ` Michael Ellerman
2024-09-04 18:53 ` Deepak Gupta
3 siblings, 0 replies; 18+ messages in thread
From: Liam R. Howlett @ 2024-09-03 19:37 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Vlastimil Babka, Lorenzo Stoakes, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm
* Mark Brown <broonie@kernel.org> [240902 15:09]:
> In preparation for using vm_flags to ensure guard pages for shadow stacks
> supply them as an argument to generic_get_unmapped_area(). The only user
> outside of the core code is the PowerPC book3s64 implementation which is
> trivially wrapping the generic implementation in the radix_enabled() case.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
It is interesting that book3s64 ppc is special in this regard.
Reviewed-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
> ---
> arch/powerpc/mm/book3s64/slice.c | 4 ++--
> include/linux/sched/mm.h | 4 ++--
> mm/mmap.c | 10 ++++++----
> 3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
> index ada6bf896ef8..87307d0fc3b8 100644
> --- a/arch/powerpc/mm/book3s64/slice.c
> +++ b/arch/powerpc/mm/book3s64/slice.c
> @@ -641,7 +641,7 @@ unsigned long arch_get_unmapped_area(struct file *filp,
> vm_flags_t vm_flags)
> {
> if (radix_enabled())
> - return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area(filp, addr, len, pgoff, flags, vm_flags);
>
> return slice_get_unmapped_area(addr, len, flags,
> mm_ctx_user_psize(¤t->mm->context), 0);
> @@ -655,7 +655,7 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> vm_flags_t vm_flags)
> {
> if (radix_enabled())
> - return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
> + return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags, vm_flags);
>
> return slice_get_unmapped_area(addr0, len, flags,
> mm_ctx_user_psize(¤t->mm->context), 1);
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index c4d34abc45d4..07bb8d4181d7 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -204,11 +204,11 @@ unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
> unsigned long
> generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long flags, vm_flags_t vm_flags);
> unsigned long
> generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long flags, vm_flags_t vm_flags);
> #else
> static inline void arch_pick_mmap_layout(struct mm_struct *mm,
> struct rlimit *rlim_stack) {}
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 7528146f886f..b06ba847c96e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1789,7 +1789,7 @@ unsigned long vm_unmapped_area(struct vm_unmapped_area_info *info)
> unsigned long
> generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma, *prev;
> @@ -1823,7 +1823,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> - return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area(filp, addr, len, pgoff, flags,
> + vm_flags);
> }
> #endif
>
> @@ -1834,7 +1835,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long
> generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma, *prev;
> struct mm_struct *mm = current->mm;
> @@ -1887,7 +1888,8 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> - return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags,
> + vm_flags);
> }
> #endif
>
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-02 19:08 ` [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2024-09-03 17:49 ` Lorenzo Stoakes
@ 2024-09-03 19:41 ` Liam R. Howlett
2024-09-03 19:57 ` Mark Brown
2024-09-04 18:51 ` Deepak Gupta
2 siblings, 1 reply; 18+ messages in thread
From: Liam R. Howlett @ 2024-09-03 19:41 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Vlastimil Babka, Lorenzo Stoakes, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm, Rick Edgecombe
* Mark Brown <broonie@kernel.org> [240902 15:09]:
> As covered in the commit log for c44357c2e76b ("x86/mm: care about shadow
> stack guard gap during placement") our current mmap() implementation does
> not take care to ensure that a new mapping isn't placed with existing
> mappings inside it's own guard gaps. This is particularly important for
> shadow stacks since if two shadow stacks end up getting placed adjacent to
> each other then they can overflow into each other which weakens the
> protection offered by the feature.
>
> On x86 there is a custom arch_get_unmapped_area() which was updated by the
> above commit to cover this case by specifying a start_gap for allocations
> with VM_SHADOW_STACK. Both arm64 and RISC-V have equivalent features and
> use the generic implementation of arch_get_unmapped_area() so let's make
> the equivalent change there so they also don't get shadow stack pages
> placed without guard pages.
>
> Architectures which do not have this feature will define VM_SHADOW_STACK
> to VM_NONE and hence be unaffected.
>
> Suggested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> mm/mmap.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index b06ba847c96e..902c482b6084 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1753,6 +1753,14 @@ static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
> return gap;
> }
>
> +static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
> +{
> + if (vm_flags & VM_SHADOW_STACK)
> + return PAGE_SIZE;
Is PAGE_SIZE is enough?
> +
> + return 0;
> +}
> +
> /*
> * Search for an unmapped address range.
> *
> @@ -1814,6 +1822,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
> info.length = len;
> info.low_limit = mm->mmap_base;
> info.high_limit = mmap_end;
> + info.start_gap = stack_guard_placement(vm_flags);
> return vm_unmapped_area(&info);
> }
>
> @@ -1863,6 +1872,7 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> info.length = len;
> info.low_limit = PAGE_SIZE;
> info.high_limit = arch_get_mmap_base(addr, mm->mmap_base);
> + info.start_gap = stack_guard_placement(vm_flags);
> addr = vm_unmapped_area(&info);
>
> /*
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default
2024-09-02 19:08 ` [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default Mark Brown
2024-09-03 17:43 ` Lorenzo Stoakes
2024-09-03 19:35 ` Liam R. Howlett
@ 2024-09-03 19:50 ` Helge Deller
2 siblings, 0 replies; 18+ messages in thread
From: Helge Deller @ 2024-09-03 19:50 UTC (permalink / raw)
To: Mark Brown, Richard Henderson, Ivan Kokshaysky, Matt Turner,
Vineet Gupta, Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes
Cc: Catalin Marinas, Will Deacon, Deepak Gupta, linux-arm-kernel,
linux-alpha, linux-kernel, linux-snps-arc, linux-csky, loongarch,
linux-parisc, linuxppc-dev, linux-s390, linux-sh, sparclinux,
linux-mm
On 9/2/24 21:08, Mark Brown wrote:
> When we introduced arch_get_unmapped_area_vmflags() in 961148704acd
> ("mm: introduce arch_get_unmapped_area_vmflags()") we did so as part of
> properly supporting guard pages for shadow stacks on x86_64, which uses
> a custom arch_get_unmapped_area(). Equivalent features are also present
> on both arm64 and RISC-V, both of which use the generic implementation
> of arch_get_unmapped_area() and will require equivalent modification
> there. Rather than continue to deal with having two versions of the
> functions let's bite the bullet and have all implementations of
> arch_get_unmapped_area() take vm_flags as a parameter.
>
> The new parameter is currently ignored by all implementations other than
> x86. The only caller that doesn't have a vm_flags available is
> mm_get_unmapped_area(), as for the x86 implementation and the wrapper used
> on other architectures this is modified to supply no flags.
>
> No functional changes.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> arch/alpha/kernel/osf_sys.c | 2 +-
> arch/arc/mm/mmap.c | 3 ++-
> arch/arm/mm/mmap.c | 7 ++++---
> arch/csky/abiv1/mmap.c | 3 ++-
> arch/loongarch/mm/mmap.c | 5 +++--
> arch/mips/mm/mmap.c | 2 +-
> arch/parisc/kernel/sys_parisc.c | 5 +++--
> arch/parisc/mm/hugetlbpage.c | 2 +-
Acked-by: Helge Deller <deller@gmx.de> # parisc
Helge
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-03 19:41 ` Liam R. Howlett
@ 2024-09-03 19:57 ` Mark Brown
2024-09-04 19:07 ` Deepak Gupta
0 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2024-09-03 19:57 UTC (permalink / raw)
To: Liam R. Howlett, Richard Henderson, Ivan Kokshaysky, Matt Turner,
Vineet Gupta, Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Vlastimil Babka, Lorenzo Stoakes, Catalin Marinas, Will Deacon,
Deepak Gupta, linux-arm-kernel, linux-alpha, linux-kernel,
linux-snps-arc, linux-csky, loongarch, linux-parisc, linuxppc-dev,
linux-s390, linux-sh, sparclinux, linux-mm, Rick Edgecombe
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
On Tue, Sep 03, 2024 at 03:41:49PM -0400, Liam R. Howlett wrote:
> * Mark Brown <broonie@kernel.org> [240902 15:09]:
> > +static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
> > +{
> > + if (vm_flags & VM_SHADOW_STACK)
> > + return PAGE_SIZE;
> Is PAGE_SIZE is enough?
It's what x86 currently uses so it'll be no worse off if it gets moved
to the generic code (there's a comment in the arch code explaing what's
needed there) and it's enough for arm64, we only do single record
pushes/pops or (optionally) writes to unconstrained addresses.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area()
2024-09-02 19:08 ` [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area() Mark Brown
2024-09-03 17:44 ` Lorenzo Stoakes
2024-09-03 19:37 ` Liam R. Howlett
@ 2024-09-04 4:13 ` Michael Ellerman
2024-09-04 18:53 ` Deepak Gupta
3 siblings, 0 replies; 18+ messages in thread
From: Michael Ellerman @ 2024-09-04 4:13 UTC (permalink / raw)
To: Mark Brown, Richard Henderson, Ivan Kokshaysky, Matt Turner,
Vineet Gupta, Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes
Cc: Catalin Marinas, Will Deacon, Deepak Gupta, linux-arm-kernel,
linux-alpha, linux-kernel, linux-snps-arc, linux-arm-kernel,
linux-csky, loongarch, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-mm, Mark Brown
Mark Brown <broonie@kernel.org> writes:
> In preparation for using vm_flags to ensure guard pages for shadow stacks
> supply them as an argument to generic_get_unmapped_area(). The only user
> outside of the core code is the PowerPC book3s64 implementation which is
> trivially wrapping the generic implementation in the radix_enabled() case.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> arch/powerpc/mm/book3s64/slice.c | 4 ++--
> include/linux/sched/mm.h | 4 ++--
> mm/mmap.c | 10 ++++++----
> 3 files changed, 10 insertions(+), 8 deletions(-)
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
> diff --git a/arch/powerpc/mm/book3s64/slice.c b/arch/powerpc/mm/book3s64/slice.c
> index ada6bf896ef8..87307d0fc3b8 100644
> --- a/arch/powerpc/mm/book3s64/slice.c
> +++ b/arch/powerpc/mm/book3s64/slice.c
> @@ -641,7 +641,7 @@ unsigned long arch_get_unmapped_area(struct file *filp,
> vm_flags_t vm_flags)
> {
> if (radix_enabled())
> - return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area(filp, addr, len, pgoff, flags, vm_flags);
>
> return slice_get_unmapped_area(addr, len, flags,
> mm_ctx_user_psize(¤t->mm->context), 0);
> @@ -655,7 +655,7 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp,
> vm_flags_t vm_flags)
> {
> if (radix_enabled())
> - return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags);
> + return generic_get_unmapped_area_topdown(filp, addr0, len, pgoff, flags, vm_flags);
>
> return slice_get_unmapped_area(addr0, len, flags,
> mm_ctx_user_psize(¤t->mm->context), 1);
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index c4d34abc45d4..07bb8d4181d7 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -204,11 +204,11 @@ unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
> unsigned long
> generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long flags, vm_flags_t vm_flags);
> unsigned long
> generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags);
> + unsigned long flags, vm_flags_t vm_flags);
> #else
> static inline void arch_pick_mmap_layout(struct mm_struct *mm,
> struct rlimit *rlim_stack) {}
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 7528146f886f..b06ba847c96e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1789,7 +1789,7 @@ unsigned long vm_unmapped_area(struct vm_unmapped_area_info *info)
> unsigned long
> generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma, *prev;
> @@ -1823,7 +1823,8 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> - return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area(filp, addr, len, pgoff, flags,
> + vm_flags);
> }
> #endif
>
> @@ -1834,7 +1835,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long
> generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> - unsigned long flags)
> + unsigned long flags, vm_flags_t vm_flags)
> {
> struct vm_area_struct *vma, *prev;
> struct mm_struct *mm = current->mm;
> @@ -1887,7 +1888,8 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> unsigned long flags, vm_flags_t vm_flags)
> {
> - return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> + return generic_get_unmapped_area_topdown(filp, addr, len, pgoff, flags,
> + vm_flags);
> }
> #endif
>
>
> --
> 2.39.2
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-02 19:08 ` [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2024-09-03 17:49 ` Lorenzo Stoakes
2024-09-03 19:41 ` Liam R. Howlett
@ 2024-09-04 18:51 ` Deepak Gupta
2 siblings, 0 replies; 18+ messages in thread
From: Deepak Gupta @ 2024-09-04 18:51 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes,
Catalin Marinas, Will Deacon, linux-arm-kernel, linux-alpha,
linux-kernel, linux-snps-arc, linux-csky, loongarch, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, linux-mm,
Rick Edgecombe
On Mon, Sep 02, 2024 at 08:08:15PM +0100, Mark Brown wrote:
>As covered in the commit log for c44357c2e76b ("x86/mm: care about shadow
>stack guard gap during placement") our current mmap() implementation does
>not take care to ensure that a new mapping isn't placed with existing
>mappings inside it's own guard gaps. This is particularly important for
>shadow stacks since if two shadow stacks end up getting placed adjacent to
>each other then they can overflow into each other which weakens the
>protection offered by the feature.
>
>On x86 there is a custom arch_get_unmapped_area() which was updated by the
>above commit to cover this case by specifying a start_gap for allocations
>with VM_SHADOW_STACK. Both arm64 and RISC-V have equivalent features and
>use the generic implementation of arch_get_unmapped_area() so let's make
>the equivalent change there so they also don't get shadow stack pages
>placed without guard pages.
>
>Architectures which do not have this feature will define VM_SHADOW_STACK
>to VM_NONE and hence be unaffected.
>
>Suggested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>Signed-off-by: Mark Brown <broonie@kernel.org>
>---
> mm/mmap.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/mm/mmap.c b/mm/mmap.c
>index b06ba847c96e..902c482b6084 100644
>--- a/mm/mmap.c
>+++ b/mm/mmap.c
>@@ -1753,6 +1753,14 @@ static unsigned long unmapped_area_topdown(struct vm_unmapped_area_info *info)
> return gap;
> }
>
>+static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
>+{
>+ if (vm_flags & VM_SHADOW_STACK)
>+ return PAGE_SIZE;
>+
>+ return 0;
>+}
>+
> /*
> * Search for an unmapped address range.
> *
>@@ -1814,6 +1822,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
> info.length = len;
> info.low_limit = mm->mmap_base;
> info.high_limit = mmap_end;
>+ info.start_gap = stack_guard_placement(vm_flags);
> return vm_unmapped_area(&info);
> }
>
>@@ -1863,6 +1872,7 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> info.length = len;
> info.low_limit = PAGE_SIZE;
> info.high_limit = arch_get_mmap_base(addr, mm->mmap_base);
>+ info.start_gap = stack_guard_placement(vm_flags);
> addr = vm_unmapped_area(&info);
>
> /*
>
lgtm
Reviewed-by: Deepak Gupta <debug@rivosinc.com>
>--
>2.39.2
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area()
2024-09-02 19:08 ` [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area() Mark Brown
` (2 preceding siblings ...)
2024-09-04 4:13 ` Michael Ellerman
@ 2024-09-04 18:53 ` Deepak Gupta
3 siblings, 0 replies; 18+ messages in thread
From: Deepak Gupta @ 2024-09-04 18:53 UTC (permalink / raw)
To: Mark Brown
Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Liam R. Howlett, Vlastimil Babka, Lorenzo Stoakes,
Catalin Marinas, Will Deacon, linux-arm-kernel, linux-alpha,
linux-kernel, linux-snps-arc, linux-csky, loongarch, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, linux-mm
On Mon, Sep 02, 2024 at 08:08:14PM +0100, Mark Brown wrote:
>In preparation for using vm_flags to ensure guard pages for shadow stacks
>supply them as an argument to generic_get_unmapped_area(). The only user
>outside of the core code is the PowerPC book3s64 implementation which is
>trivially wrapping the generic implementation in the radix_enabled() case.
>
>Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Deepak Gupta <debug@rivosinc.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area
2024-09-03 19:57 ` Mark Brown
@ 2024-09-04 19:07 ` Deepak Gupta
0 siblings, 0 replies; 18+ messages in thread
From: Deepak Gupta @ 2024-09-04 19:07 UTC (permalink / raw)
To: Mark Brown
Cc: Liam R. Howlett, Richard Henderson, Ivan Kokshaysky, Matt Turner,
Vineet Gupta, Russell King, Guo Ren, Huacai Chen, WANG Xuerui,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S. Miller, Andreas Larsson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Andrew Morton,
Vlastimil Babka, Lorenzo Stoakes, Catalin Marinas, Will Deacon,
linux-arm-kernel, linux-alpha, linux-kernel, linux-snps-arc,
linux-csky, loongarch, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-mm, Rick Edgecombe
On Tue, Sep 03, 2024 at 08:57:20PM +0100, Mark Brown wrote:
>On Tue, Sep 03, 2024 at 03:41:49PM -0400, Liam R. Howlett wrote:
>> * Mark Brown <broonie@kernel.org> [240902 15:09]:
>
>> > +static inline unsigned long stack_guard_placement(vm_flags_t vm_flags)
>> > +{
>> > + if (vm_flags & VM_SHADOW_STACK)
>> > + return PAGE_SIZE;
>
>> Is PAGE_SIZE is enough?
>
>It's what x86 currently uses so it'll be no worse off if it gets moved
>to the generic code (there's a comment in the arch code explaing what's
>needed there) and it's enough for arm64, we only do single record
>pushes/pops or (optionally) writes to unconstrained addresses.
It's enough for RISC-V too.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2024-09-04 19:07 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-02 19:08 [PATCH 0/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2024-09-02 19:08 ` [PATCH 1/3] mm: Make arch_get_unmapped_area() take vm_flags by default Mark Brown
2024-09-03 17:43 ` Lorenzo Stoakes
2024-09-03 19:35 ` Liam R. Howlett
2024-09-03 19:50 ` Helge Deller
2024-09-02 19:08 ` [PATCH 2/3] mm: Pass vm_flags to generic_get_unmapped_area() Mark Brown
2024-09-03 17:44 ` Lorenzo Stoakes
2024-09-03 19:37 ` Liam R. Howlett
2024-09-04 4:13 ` Michael Ellerman
2024-09-04 18:53 ` Deepak Gupta
2024-09-02 19:08 ` [PATCH 3/3] mm: Care about shadow stack guard gap when getting an unmapped area Mark Brown
2024-09-03 17:49 ` Lorenzo Stoakes
2024-09-03 18:20 ` Mark Brown
2024-09-03 19:24 ` Lorenzo Stoakes
2024-09-03 19:41 ` Liam R. Howlett
2024-09-03 19:57 ` Mark Brown
2024-09-04 19:07 ` Deepak Gupta
2024-09-04 18:51 ` Deepak Gupta
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).