Rebased on top of Nic's series "powerpc: Make hash MMU code build configurable" in today's powerpc/merge-test on github
This series converts powerpc to default topdown mmap layout.
powerpc requires its own arch_get_unmapped_area() only when
slices are needed, which is only for book3s/64. First part of
the series moves slices into book3s/64 specific directories
and cleans up other subarchitectures.
Last part converts to default topdown mmap layout.
A small modification is done to core mm to allow
powerpc to still provide its own arch_randomize_brk()
Another modification is done to core mm to allow powerpc
to use generic versions of get_unmapped_area functions for Radix
while still providing its own implementation for Hash, the
selection between Radix and Hash being doing at runtime.
Signed-off-by: Christophe Leroy <redacted>
Changes in v4:
- Move arch_randomize_brk() simplification out of this series
- Add a change to core mm to enable using generic implementation
while providing arch specific one at the same time.
- Reworked radix get_unmapped_area to use generic implementation
- Rebase on top of Nic's series v6
Changes in v3:
- Fixed missing <linux/elf-randomize.h> in last patch
- Added a patch to move SZ_1T out of drivers/pci/controller/pci-xgene.c
Changes in v2:
- Moved patch 4 before patch 2
- Make generic arch_randomize_brk() __weak
- Added patch 9
Christophe Leroy (10):
mm: Allow arch specific arch_randomize_brk() with
CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
mm, hugetlbfs: Allow an arch to always use generic versions of
get_unmapped_area functions
powerpc/mm: Move vma_mmu_pagesize()
powerpc/mm: Make slice specific to book3s/64
powerpc/mm: Remove CONFIG_PPC_MM_SLICES
powerpc/mm: Use generic_get_unmapped_area() and call it from
arch_get_unmapped_area()
powerpc/mm: Use generic_hugetlb_get_unmapped_area()
powerpc/mm: Move get_unmapped_area functions to slice.c
powerpc/mm: Convert to default topdown mmap layout
powerpc/mm: Properly randomise mmap with slices
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/book3s/64/hugetlb.h | 4 -
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 1 +
arch/powerpc/include/asm/book3s/64/mmu.h | 6 -
arch/powerpc/include/asm/book3s/64/slice.h | 24 ++
arch/powerpc/include/asm/hugetlb.h | 2 +-
arch/powerpc/include/asm/paca.h | 7 -
arch/powerpc/include/asm/page.h | 1 -
arch/powerpc/include/asm/processor.h | 2 -
arch/powerpc/include/asm/slice.h | 46 ----
arch/powerpc/kernel/paca.c | 5 -
arch/powerpc/mm/Makefile | 3 +-
arch/powerpc/mm/book3s64/Makefile | 2 +-
arch/powerpc/mm/book3s64/hash_utils.c | 14 -
arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 ----
arch/powerpc/mm/{ => book3s64}/slice.c | 71 ++++-
arch/powerpc/mm/hugetlbpage.c | 34 ---
arch/powerpc/mm/mmap.c | 256 ------------------
arch/powerpc/mm/nohash/mmu_context.c | 9 -
arch/powerpc/mm/nohash/tlb.c | 4 -
arch/powerpc/platforms/Kconfig.cputype | 4 -
fs/hugetlbfs/inode.c | 17 +-
include/linux/hugetlb.h | 5 +
include/linux/sched/mm.h | 9 +
mm/mmap.c | 31 ++-
mm/util.c | 2 +-
26 files changed, 139 insertions(+), 477 deletions(-)
delete mode 100644 arch/powerpc/include/asm/slice.h
rename arch/powerpc/mm/{ => book3s64}/slice.c (91%)
delete mode 100644 arch/powerpc/mm/mmap.c
--
2.33.1
Commit e7142bf5d231 ("arm64, mm: make randomization selected by
generic topdown mmap layout") introduced a default version of
arch_randomize_brk() provided when
CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT is selected.
powerpc could select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
but needs to provide its own arch_randomize_brk().
In order to allow that, define generic version of arch_randomize_brk()
as a __weak symbol.
Cc: Alexandre Ghiti <alex@ghiti.fr>
Signed-off-by: Christophe Leroy <redacted>
---
mm/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -344,7 +344,7 @@ unsigned long randomize_stack_top(unsigned long stack_top)}#ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT-unsignedlongarch_randomize_brk(structmm_struct*mm)+unsignedlong__weakarch_randomize_brk(structmm_struct*mm){/* Is the current task 32bit ? */if(!IS_ENABLED(CONFIG_64BIT)||is_compat_task())
Use the generic version of arch_get_unmapped_area() which
is now available at all time instead of its copy
radix__arch_get_unmapped_area()
Instead of setting mm->get_unmapped_area() to either
arch_get_unmapped_area() or generic_get_unmapped_area(),
always set it to arch_get_unmapped_area() and call
generic_get_unmapped_area() from there when radix is enabled.
Do the same with radix__arch_get_unmapped_area_topdown()
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/mm/mmap.c | 127 ++---------------------------------------
1 file changed, 6 insertions(+), 121 deletions(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-12-09 09:50:17
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted hunk
Use the generic version of arch_get_unmapped_area() which
is now available at all time instead of its copy
radix__arch_get_unmapped_area()
Instead of setting mm->get_unmapped_area() to either
arch_get_unmapped_area() or generic_get_unmapped_area(),
always set it to arch_get_unmapped_area() and call
generic_get_unmapped_area() from there when radix is enabled.
Do the same with radix__arch_get_unmapped_area_topdown()
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/mm/mmap.c | 127 ++---------------------------------------
1 file changed, 6 insertions(+), 121 deletions(-)
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Use the generic version of arch_get_unmapped_area() which
is now available at all time instead of its copy
radix__arch_get_unmapped_area()
Instead of setting mm->get_unmapped_area() to either
arch_get_unmapped_area() or generic_get_unmapped_area(),
always set it to arch_get_unmapped_area() and call
generic_get_unmapped_area() from there when radix is enabled.
Do the same with radix__arch_get_unmapped_area_topdown()
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/mm/mmap.c | 127 ++---------------------------------------
1 file changed, 6 insertions(+), 121 deletions(-)
@@ -81,115 +81,15 @@ static inline unsigned long mmap_base(unsigned long rnd,}#ifdef HAVE_ARCH_UNMAPPED_AREA-#ifdef CONFIG_PPC_RADIX_MMU-/*-*Samefunctionasgenericcodeusedonlyforradix,becausewedon'tneedtooverload-*thegenericone.Butwewillhavetoduplicate,becausehashselect-*HAVE_ARCH_UNMAPPED_AREA-*/-staticunsignedlong-radix__arch_get_unmapped_area(structfile*filp,unsignedlongaddr,-unsignedlonglen,unsignedlongpgoff,-unsignedlongflags)-{-structmm_struct*mm=current->mm;-structvm_area_struct*vma;-intfixed=(flags&MAP_FIXED);-unsignedlonghigh_limit;-structvm_unmapped_area_infoinfo;--high_limit=DEFAULT_MAP_WINDOW;-if(addr>=high_limit||(fixed&&(addr+len>high_limit)))-high_limit=TASK_SIZE;
Does 64s radix need to define arch_get_mmap_end() to do the above now?
Sure, good point.
Seems like I got hypnotised by the comment "- * Same function as generic
code used only for radix", I didn't catch that little difference.
Otherwise great to consolidate this with core code, nice patch.
Thanks,
Nick
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Use the generic version of arch_get_unmapped_area() which
is now available at all time instead of its copy
radix__arch_get_unmapped_area()
Instead of setting mm->get_unmapped_area() to either
arch_get_unmapped_area() or generic_get_unmapped_area(),
always set it to arch_get_unmapped_area() and call
generic_get_unmapped_area() from there when radix is enabled.
Do the same with radix__arch_get_unmapped_area_topdown()
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/mm/mmap.c | 127 ++---------------------------------------
1 file changed, 6 insertions(+), 121 deletions(-)
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2021-12-13 13:11:59
Christophe Leroy [off-list ref] writes:
quoted hunk
Use the generic version of arch_get_unmapped_area() which
is now available at all time instead of its copy
radix__arch_get_unmapped_area()
Instead of setting mm->get_unmapped_area() to either
arch_get_unmapped_area() or generic_get_unmapped_area(),
always set it to arch_get_unmapped_area() and call
generic_get_unmapped_area() from there when radix is enabled.
Do the same with radix__arch_get_unmapped_area_topdown()
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/mm/mmap.c | 127 ++---------------------------------------
1 file changed, 6 insertions(+), 121 deletions(-)
@@ -81,115 +81,15 @@ static inline unsigned long mmap_base(unsigned long rnd,}#ifdef HAVE_ARCH_UNMAPPED_AREA-#ifdef CONFIG_PPC_RADIX_MMU-/*-*Samefunctionasgenericcodeusedonlyforradix,becausewedon'tneedtooverload-*thegenericone.Butwewillhavetoduplicate,becausehashselect-*HAVE_ARCH_UNMAPPED_AREA-*/-staticunsignedlong-radix__arch_get_unmapped_area(structfile*filp,unsignedlongaddr,-unsignedlonglen,unsignedlongpgoff,-unsignedlongflags)-{-structmm_struct*mm=current->mm;-structvm_area_struct*vma;-intfixed=(flags&MAP_FIXED);-unsignedlonghigh_limit;-structvm_unmapped_area_infoinfo;--high_limit=DEFAULT_MAP_WINDOW;-if(addr>=high_limit||(fixed&&(addr+len>high_limit)))-high_limit=TASK_SIZE;--if(len>high_limit)-return-ENOMEM;
There are some differences in the above vs the generic code, the generic
arch_get_unmapped_area_topdown() in mm/mmap.c does:
const unsigned long mmap_end = arch_get_mmap_end(addr);
if (len > mmap_end - mmap_min_addr)
return -ENOMEM;
if (flags & MAP_FIXED)
return addr;
Our current code adjusts high_limit for fixed mappings that span above
the default map window. We added that logic in:
35602f82d0c7 ("powerpc/64s/hash: Allow MAP_FIXED allocations to cross 128TB boundary")
That means a fixed mapping that crosses the 128T boundary will be
allowed by our code.
On the other hand the generic code will allow a fixed mapping to cross
the 128T boundary, but only if the size of the mapping is < ~128T.
(The actual size limit is (128T - mmap_min_addr), which is usually 4K or
64K, but is adjustable.)
It's unlikely that any apps are doing fixed mappings larger than 128T
that cross the 128T boundary, but I think we need to allow it. 128T
seems like a lot, but is not compared to the entire 4PB address space.
So I think we need to fix that in the generic code.
The easiest option is probably to pass flags to arch_get_mmap_end(), and
then the arches can decide whether to adjust the return value based on
flags.
Then there's also the extra check we have here:
- if (fixed) {
- if (addr > high_limit - len)
- return -ENOMEM;
- return addr;
- }
I think we can drop that when converting to the generic version, the
only case in which it matters is when high_limit == TASK_SIZE, and
get_unmapped_area() already does that check after calling us:
if (addr > TASK_SIZE - len)
return -ENOMEM;
cheers
Unlike most architectures, powerpc can only define at runtime
if it is going to use the generic arch_get_unmapped_area() or not.
Today, powerpc has a copy of the generic arch_get_unmapped_area()
because when selection HAVE_ARCH_UNMAPPED_AREA the generic
arch_get_unmapped_area() is not available.
Rename it generic_get_unmapped_area() and make it independent of
HAVE_ARCH_UNMAPPED_AREA.
Do the same for arch_get_unmapped_area_topdown() versus
HAVE_ARCH_UNMAPPED_AREA_TOPDOWN.
Do the same for hugetlb_get_unmapped_area() versus
HAVE_ARCH_HUGETLB_UNMAPPED_AREA.
Signed-off-by: Christophe Leroy <redacted>
---
fs/hugetlbfs/inode.c | 17 +++++++++++++----
include/linux/hugetlb.h | 5 +++++
include/linux/sched/mm.h | 9 +++++++++
mm/mmap.c | 31 ++++++++++++++++++++++++-------
4 files changed, 51 insertions(+), 11 deletions(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-12-09 09:40:28
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
Unlike most architectures, powerpc can only define at runtime
if it is going to use the generic arch_get_unmapped_area() or not.
Today, powerpc has a copy of the generic arch_get_unmapped_area()
because when selection HAVE_ARCH_UNMAPPED_AREA the generic
arch_get_unmapped_area() is not available.
Rename it generic_get_unmapped_area() and make it independent of
HAVE_ARCH_UNMAPPED_AREA.
Do the same for arch_get_unmapped_area_topdown() versus
HAVE_ARCH_UNMAPPED_AREA_TOPDOWN.
Do the same for hugetlb_get_unmapped_area() versus
HAVE_ARCH_HUGETLB_UNMAPPED_AREA.
Use the generic version of arch_hugetlb_get_unmapped_area()
which is now available at all time.
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/include/asm/book3s/64/hugetlb.h | 4 --
arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 --------------------
arch/powerpc/mm/hugetlbpage.c | 4 +-
3 files changed, 1 insertion(+), 62 deletions(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-12-09 10:02:12
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted hunk
Use the generic version of arch_hugetlb_get_unmapped_area()
which is now available at all time.
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/include/asm/book3s/64/hugetlb.h | 4 --
arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 --------------------
arch/powerpc/mm/hugetlbpage.c | 4 +-
3 files changed, 1 insertion(+), 62 deletions(-)
I wonder if generic hugetlb_get_unmapped_area needs to have the
arch_get_mmap_end() added.
arm64 has arch_get_mmap_end() and !HAVE_ARCH_HUGETLB_UNMAPPED_AREA so
it looks like it has broken large address hint logic for hugetlbfs
mappings? x86-64 defines their own and does the same hinting for
normal and hugetlbfs mmap.
If we had that and defied arch_get_mmap_end(), then this patch should
work.
Thanks,
Nick
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Use the generic version of arch_hugetlb_get_unmapped_area()
which is now available at all time.
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/include/asm/book3s/64/hugetlb.h | 4 --
arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 --------------------
arch/powerpc/mm/hugetlbpage.c | 4 +-
3 files changed, 1 insertion(+), 62 deletions(-)
I wonder if generic hugetlb_get_unmapped_area needs to have the
arch_get_mmap_end() added.
arm64 has arch_get_mmap_end() and !HAVE_ARCH_HUGETLB_UNMAPPED_AREA so
it looks like it has broken large address hint logic for hugetlbfs
mappings? x86-64 defines their own and does the same hinting for
normal and hugetlbfs mmap.
If we had that and defied arch_get_mmap_end(), then this patch should
work.
As far as I can see, hugetlb_get_unmapped_area() variants used to be
very similar to get_unmapped_area() until commit 1be7107fbe18 ("mm:
larger stack guard gap, between vmas") and commit f6795053dac8 ("mm:
mmap: Allow for "high" userspace addresses")
I see no reason why those changes couldn't apply to
hugetlb_get_unmapped_area() as well.
Need to know what ARM64 think about it thought. Will, Catalin, any opinion ?
Christophe
On Thu, Dec 16, 2021 at 05:13:47PM +0000, Christophe Leroy wrote:
Le 09/12/2021 à 11:02, Nicholas Piggin a écrit :
quoted
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Use the generic version of arch_hugetlb_get_unmapped_area()
which is now available at all time.
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/include/asm/book3s/64/hugetlb.h | 4 --
arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 --------------------
arch/powerpc/mm/hugetlbpage.c | 4 +-
3 files changed, 1 insertion(+), 62 deletions(-)
I wonder if generic hugetlb_get_unmapped_area needs to have the
arch_get_mmap_end() added.
arm64 has arch_get_mmap_end() and !HAVE_ARCH_HUGETLB_UNMAPPED_AREA so
it looks like it has broken large address hint logic for hugetlbfs
mappings? x86-64 defines their own and does the same hinting for
normal and hugetlbfs mmap.
If we had that and defied arch_get_mmap_end(), then this patch should
work.
As far as I can see, hugetlb_get_unmapped_area() variants used to be
very similar to get_unmapped_area() until commit 1be7107fbe18 ("mm:
larger stack guard gap, between vmas") and commit f6795053dac8 ("mm:
mmap: Allow for "high" userspace addresses")
I see no reason why those changes couldn't apply to
hugetlb_get_unmapped_area() as well.
Need to know what ARM64 think about it thought. Will, Catalin, any opinion ?
I think we should have fixed hugetlb_get_unmapped_area() as well when we
added support for 52-bit VA. The reason for commit f6795053dac8 was to
prevent normal mmap() from returning addresses above 48-bit by default
as some user-space had hard assumptions about this.
It's a slight ABI change if you do this for hugetlb_get_unmapped_area()
but I doubt anyone would notice. It's more likely that the current
behaviour would cause issues, so I'd rather have them consistent.
--
Catalin
vma_mmu_pagesize() is only required for slices,
otherwise there is a generic weak version doing the
exact same thing.
Move it to slice.c
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/mm/hugetlbpage.c | 11 -----------
arch/powerpc/mm/slice.c | 9 +++++++++
2 files changed, 9 insertions(+), 11 deletions(-)
@@ -565,17 +565,6 @@ unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,}#endif-unsignedlongvma_mmu_pagesize(structvm_area_struct*vma)-{-/* With radix we don't use slice, so derive it from vma*/-if(IS_ENABLED(CONFIG_PPC_MM_SLICES)&&!radix_enabled()){-unsignedintpsize=get_slice_psize(vma->vm_mm,vma->vm_start);--return1UL<<mmu_psize_to_shift(psize);-}-returnvma_kernel_pagesize(vma);-}-bool__initarch_hugetlb_valid_size(unsignedlongsize){intshift=__ffs(size);
@@ -759,4 +759,13 @@ int slice_is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,return!slice_check_range_fits(mm,maskp,addr,len);}++unsignedlongvma_mmu_pagesize(structvm_area_struct*vma)+{+/* With radix we don't use slice, so derive it from vma*/+if(radix_enabled())+returnvma_kernel_pagesize(vma);++return1UL<<mmu_psize_to_shift(get_slice_psize(vma->vm_mm,vma->vm_start));+}#endif
@@ -565,17 +565,6 @@ unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,}#endif-unsignedlongvma_mmu_pagesize(structvm_area_struct*vma)-{-/* With radix we don't use slice, so derive it from vma*/-if(IS_ENABLED(CONFIG_PPC_MM_SLICES)&&!radix_enabled()){-unsignedintpsize=get_slice_psize(vma->vm_mm,vma->vm_start);--return1UL<<mmu_psize_to_shift(psize);-}-returnvma_kernel_pagesize(vma);-}-bool__initarch_hugetlb_valid_size(unsignedlongsize){intshift=__ffs(size);
@@ -759,4 +759,13 @@ int slice_is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,return!slice_check_range_fits(mm,maskp,addr,len);}++unsignedlongvma_mmu_pagesize(structvm_area_struct*vma)+{+/* With radix we don't use slice, so derive it from vma*/+if(radix_enabled())+returnvma_kernel_pagesize(vma);++return1UL<<mmu_psize_to_shift(get_slice_psize(vma->vm_mm,vma->vm_start));+}#endif
@@ -21,6 +21,7 @@ obj-$(CONFIG_PPC_RADIX_MMU) += radix_hugetlbpage.oendifobj-$(CONFIG_SPAPR_TCE_IOMMU)+=iommu_api.oobj-$(CONFIG_PPC_PKEY)+=pkeys.o+obj-$(CONFIG_PPC_MM_SLICES)+=slice.o# Instrumenting the SLB fault path can lead to duplicate SLB entriesKCOV_INSTRUMENT_slb.o:=n
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/book3s64/slice.csimilarity index 99%rename from arch/powerpc/mm/slice.crename to arch/powerpc/mm/book3s64/slice.cindex 8a3ac062b71e..e4382713746d 100644--- a/arch/powerpc/mm/slice.c+++ b/arch/powerpc/mm/book3s64/slice.c
hugetlb_get_unmapped_area() is now identical to the
generic version if only RADIX is enabled, so move it
to slice.c and let it fallback on the generic one
when HASH MMU is not compiled in.
Do the same with arch_get_unmapped_area() and
arch_get_unmapped_area_topdown().
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 6 ----
arch/powerpc/include/asm/book3s/64/slice.h | 6 ++++
arch/powerpc/mm/book3s64/slice.c | 42 ++++++++++++++++++++++
arch/powerpc/mm/hugetlbpage.c | 21 -----------
arch/powerpc/mm/mmap.c | 36 -------------------
5 files changed, 48 insertions(+), 63 deletions(-)
@@ -639,6 +639,32 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,}EXPORT_SYMBOL_GPL(slice_get_unmapped_area);+unsignedlongarch_get_unmapped_area(structfile*filp,+unsignedlongaddr,+unsignedlonglen,+unsignedlongpgoff,+unsignedlongflags)+{+if(radix_enabled())+returngeneric_get_unmapped_area(filp,addr,len,pgoff,flags);++returnslice_get_unmapped_area(addr,len,flags,+mm_ctx_user_psize(¤t->mm->context),0);+}++unsignedlongarch_get_unmapped_area_topdown(structfile*filp,+constunsignedlongaddr0,+constunsignedlonglen,+constunsignedlongpgoff,+constunsignedlongflags)+{+if(radix_enabled())+returngeneric_get_unmapped_area_topdown(filp,addr0,len,pgoff,flags);++returnslice_get_unmapped_area(addr0,len,flags,+mm_ctx_user_psize(¤t->mm->context),1);+}+unsignedintnotraceget_slice_psize(structmm_struct*mm,unsignedlongaddr){unsignedchar*psizes;
@@ -766,4 +792,20 @@ unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)return1UL<<mmu_psize_to_shift(get_slice_psize(vma->vm_mm,vma->vm_start));}++staticintfile_to_psize(structfile*file)+{+structhstate*hstate=hstate_file(file);+returnshift_to_mmu_psize(huge_page_shift(hstate));+}++unsignedlonghugetlb_get_unmapped_area(structfile*file,unsignedlongaddr,+unsignedlonglen,unsignedlongpgoff,+unsignedlongflags)+{+if(radix_enabled())+returngeneric_hugetlb_get_unmapped_area(file,addr,len,pgoff,flags);++returnslice_get_unmapped_area(addr,len,flags,file_to_psize(file),1);+}#endif
@@ -21,7 +21,6 @@ obj-$(CONFIG_PPC_RADIX_MMU) += radix_hugetlbpage.oendifobj-$(CONFIG_SPAPR_TCE_IOMMU)+=iommu_api.oobj-$(CONFIG_PPC_PKEY)+=pkeys.o-obj-$(CONFIG_PPC_MM_SLICES)+=slice.o# Instrumenting the SLB fault path can lead to duplicate SLB entriesKCOV_INSTRUMENT_slb.o:=n
@@ -1264,7 +1264,6 @@ unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap)returnpp;}-#ifdef CONFIG_PPC_MM_SLICESstaticunsignedintget_paca_psize(unsignedlongaddr){unsignedchar*psizes;
@@ -1281,12 +1280,6 @@ static unsigned int get_paca_psize(unsigned long addr)return(psizes[index>>1]>>(mask_index*4))&0xF;}-#else-unsignedintget_paca_psize(unsignedlongaddr)-{-returnget_paca()->mm_ctx_user_psize;-}-#endif/**Demoteasegmenttousing4kpages.
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/processor.h | 2 -
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/mmap.c | 105 ---------------------------
4 files changed, 2 insertions(+), 109 deletions(-)
delete mode 100644 arch/powerpc/mm/mmap.c
@@ -1,105 +0,0 @@-// SPDX-License-Identifier: GPL-2.0-or-later-/*- * flexible mmap layout support- *- * Copyright 2003-2004 Red Hat Inc., Durham, North Carolina.- * All Rights Reserved.- *- * Started by Ingo Molnar <mingo@elte.hu>- */--#include <linux/personality.h>-#include <linux/mm.h>-#include <linux/random.h>-#include <linux/sched/signal.h>-#include <linux/sched/mm.h>-#include <linux/elf-randomize.h>-#include <linux/security.h>-#include <linux/mman.h>--/*- * Top of mmap area (just below the process stack).- *- * Leave at least a ~128 MB hole.- */-#define MIN_GAP (128*1024*1024)-#define MAX_GAP (TASK_SIZE/6*5)--static inline int mmap_is_legacy(struct rlimit *rlim_stack)-{- if (current->personality & ADDR_COMPAT_LAYOUT)- return 1;-- if (rlim_stack->rlim_cur == RLIM_INFINITY)- return 1;-- return sysctl_legacy_va_layout;-}--unsigned long arch_mmap_rnd(void)-{- unsigned long shift, rnd;-- shift = mmap_rnd_bits;-#ifdef CONFIG_COMPAT- if (is_32bit_task())- shift = mmap_rnd_compat_bits;-#endif- rnd = get_random_long() % (1ul << shift);-- return rnd << PAGE_SHIFT;-}--static inline unsigned long stack_maxrandom_size(void)-{- if (!(current->flags & PF_RANDOMIZE))- return 0;-- /* 8MB for 32bit, 1GB for 64bit */- if (is_32bit_task())- return (1<<23);- else- return (1<<30);-}--static inline unsigned long mmap_base(unsigned long rnd,- struct rlimit *rlim_stack)-{- unsigned long gap = rlim_stack->rlim_cur;- unsigned long pad = stack_maxrandom_size() + stack_guard_gap;-- /* Values close to RLIM_INFINITY can overflow. */- if (gap + pad > gap)- gap += pad;-- if (gap < MIN_GAP)- gap = MIN_GAP;- else if (gap > MAX_GAP)- gap = MAX_GAP;-- return PAGE_ALIGN(DEFAULT_MAP_WINDOW - gap - rnd);-}--/*- * This function, called very early during the creation of a new- * process VM image, sets up which VM layout function to use:- */-void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)-{- unsigned long random_factor = 0UL;-- if (current->flags & PF_RANDOMIZE)- random_factor = arch_mmap_rnd();-- /*- * Fall back to the standard layout if the personality- * bit is set, or if the expected stack growth is unlimited:- */- if (mmap_is_legacy(rlim_stack)) {- mm->mmap_base = TASK_UNMAPPED_BASE;- mm->get_unmapped_area = arch_get_unmapped_area;- } else {- mm->mmap_base = mmap_base(random_factor, rlim_stack);- mm->get_unmapped_area = arch_get_unmapped_area_topdown;- }-}
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-12-09 10:15:51
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
The justification seems pretty reasonable.
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Thanks for describing it. Could you add random_factor to mmap_base for
the legacy path for powerpc as a 2-line change that adds the legacy
randomisation. And then this bigger patch would be closer to a no-op.
Thanks,
Nick
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
The justification seems pretty reasonable.
quoted
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Thanks for describing it. Could you add random_factor to mmap_base for
the legacy path for powerpc as a 2-line change that adds the legacy
randomisation. And then this bigger patch would be closer to a no-op.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-12-09 10:43:41
Excerpts from Christophe Leroy's message of December 9, 2021 8:22 pm:
Le 09/12/2021 à 11:15, Nicholas Piggin a écrit :
quoted
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
The justification seems pretty reasonable.
quoted
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Thanks for describing it. Could you add random_factor to mmap_base for
the legacy path for powerpc as a 2-line change that adds the legacy
randomisation. And then this bigger patch would be closer to a no-op.
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2021-12-09 11:22:35
Nicholas Piggin [off-list ref] writes:
Excerpts from Christophe Leroy's message of December 9, 2021 8:22 pm:
quoted
Le 09/12/2021 à 11:15, Nicholas Piggin a écrit :
quoted
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
The justification seems pretty reasonable.
quoted
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Thanks for describing it. Could you add random_factor to mmap_base for
the legacy path for powerpc as a 2-line change that adds the legacy
randomisation. And then this bigger patch would be closer to a no-op.
My comment at the time was:
Basically mmap_is_legacy() tells you if any of these is true:
- process has the ADDR_COMPAT_LAYOUT personality
- global legacy_va_layout sysctl is enabled
- stack is unlimited
And we only want to change the behaviour for the stack. Or at least the
change log of your patch only talks about the stack limit, not the
others.
Possibly we should just enable randomisation for all three of those
cases, but if so we must spell it out in the patch.
It'd also be good to see the output of /proc/x/maps for some processes
before and after, to show what actually changes.
From: https://github.com/linuxppc/issues/issues/59#issuecomment-502066947
So I think at least the change log on that patch still needs updating to
be clear that it's changing behaviour for all mmap_is_legacy() cases,
not just the stack unlimited case.
There's also a risk changing the mmap legacy behaviour breaks something.
But we are at least matching the behaviour of other architectures, and
there is also an escape hatch in the form of `setarch -R`.
cheers
Excerpts from Christophe Leroy's message of December 9, 2021 8:22 pm:
quoted
Le 09/12/2021 à 11:15, Nicholas Piggin a écrit :
quoted
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
The justification seems pretty reasonable.
quoted
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Thanks for describing it. Could you add random_factor to mmap_base for
the legacy path for powerpc as a 2-line change that adds the legacy
randomisation. And then this bigger patch would be closer to a no-op.
My comment at the time was:
Basically mmap_is_legacy() tells you if any of these is true:
- process has the ADDR_COMPAT_LAYOUT personality
- global legacy_va_layout sysctl is enabled
- stack is unlimited
And we only want to change the behaviour for the stack. Or at least the
change log of your patch only talks about the stack limit, not the
others.
Possibly we should just enable randomisation for all three of those
cases, but if so we must spell it out in the patch.
It'd also be good to see the output of /proc/x/maps for some processes
before and after, to show what actually changes.
From: https://github.com/linuxppc/issues/issues/59#issuecomment-502066947
So I think at least the change log on that patch still needs updating to
be clear that it's changing behaviour for all mmap_is_legacy() cases,
not just the stack unlimited case.
There's also a risk changing the mmap legacy behaviour breaks something.
But we are at least matching the behaviour of other architectures, and
there is also an escape hatch in the form of `setarch -R`.
That was the purpose of adding in the change log a reference to commit
8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32")
All this applies to powerpc as well.
But I can copy paste the changelog of that commit into mine if you think
it is more explicit.
I agree that old patch was only refering to stack limit, I had no clue
of everything else at that time.
Christophe
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2021-12-09 23:56:17
Christophe Leroy [off-list ref] writes:
Le 09/12/2021 à 12:22, Michael Ellerman a écrit :
quoted
Nicholas Piggin [off-list ref] writes:
quoted
Excerpts from Christophe Leroy's message of December 9, 2021 8:22 pm:
quoted
Le 09/12/2021 à 11:15, Nicholas Piggin a écrit :
quoted
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
The justification seems pretty reasonable.
quoted
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Thanks for describing it. Could you add random_factor to mmap_base for
the legacy path for powerpc as a 2-line change that adds the legacy
randomisation. And then this bigger patch would be closer to a no-op.
My comment at the time was:
Basically mmap_is_legacy() tells you if any of these is true:
- process has the ADDR_COMPAT_LAYOUT personality
- global legacy_va_layout sysctl is enabled
- stack is unlimited
And we only want to change the behaviour for the stack. Or at least the
change log of your patch only talks about the stack limit, not the
others.
Possibly we should just enable randomisation for all three of those
cases, but if so we must spell it out in the patch.
It'd also be good to see the output of /proc/x/maps for some processes
before and after, to show what actually changes.
From: https://github.com/linuxppc/issues/issues/59#issuecomment-502066947
So I think at least the change log on that patch still needs updating to
be clear that it's changing behaviour for all mmap_is_legacy() cases,
not just the stack unlimited case.
There's also a risk changing the mmap legacy behaviour breaks something.
But we are at least matching the behaviour of other architectures, and
there is also an escape hatch in the form of `setarch -R`.
That was the purpose of adding in the change log a reference to commit
8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32")
All this applies to powerpc as well.
Yeah, I'm just a pessimist :) So although the security benefit is nice,
I'm more worried that the layout change will break some mission critical
legacy app somewhere. So I just like to have that spelled out in the
change log, or at least in the discussion like here.
But I can copy paste the changelog of that commit into mine if you think
it is more explicit.
Just referring to it is probably fine.
I agree that old patch was only refering to stack limit, I had no clue
of everything else at that time.
Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
quoted
Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
remove arch/powerpc/mm/mmap.c
This change provides standard randomisation of mmaps.
See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32") for all the benefits of mmap randomisation.
The justification seems pretty reasonable.
quoted
Comparison between powerpc implementation and the generic one:
- mmap_is_legacy() is identical.
- arch_mmap_rnd() does exactly the same allthough it's written
slightly differently.
- MIN_GAP and MAX_GAP are identical.
- mmap_base() does the same but uses STACK_RND_MASK which provides
the same values as stack_maxrandom_size().
- arch_pick_mmap_layout() is almost identical. The only difference
is that it also adds the random factor to mm->mmap_base in legacy mode.
That last point is what provides the standard randomisation of mmaps.
Thanks for describing it. Could you add random_factor to mmap_base for
the legacy path for powerpc as a 2-line change that adds the legacy
randomisation. And then this bigger patch would be closer to a no-op.
Ok, in v5 I added that change in patch 10 then switched this patch with
that patch.
Christophe
Now that powerpc switched to default topdown mmap layout,
mm->mmap_base is properly randomised. However
slice_find_area_bottomup() doesn't use mm->mmap_base but
uses the fixed TASK_UNMAPPED_BASE instead.
slice_find_area_bottomup() being used as a fallback to
slice_find_area_topdown(), it can't use mm->mmap_base
directly.
Instead of always using TASK_UNMAPPED_BASE as base address, leave
it to the caller. When called from slice_find_area_topdown()
TASK_UNMAPPED_BASE is used. Otherwise mm->mmap_base is used.
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/mm/book3s64/slice.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)