"Aneesh Kumar K.V" <aneesh.ku...@linux.ibm.com> writes: > After we ALIGN up the address we need to make sure we didn't overflow > and resulted in zero address. In that case, we need to make sure that > the returned address is greater than mmap_min_addr. > > Also when doing top-down search the low_limit is not PAGE_SIZE but rather > max(PAGE_SIZE, mmap_min_addr). This handle cases in which mmap_min_addr > > PAGE_SIZE. > > This fixes selftest va_128TBswitch --run-hugetlb reporting failures when > run as non root user for > > mmap(-1, MAP_HUGETLB) > mmap(-1, MAP_HUGETLB) > > We also avoid the first mmap(-1, MAP_HUGETLB) returning NULL address as mmap > address > with this change
So we think this is not a security issue, because it only affects whether we choose an address below mmap_min_addr, not whether we actually allow that address to be mapped. ie. there are existing capability checks to prevent a user mapping below mmap_min_addr and those will still be honoured even without this fix. However there is a bug in that a non-root user requesting address -1 will be given address 0 which will then fail, whereas they should have been given something else that would have succeeded. Did I get that all right? > CC: Laurent Dufour <lduf...@linux.vnet.ibm.com> > Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.ibm.com> Seems like this should have a Fixes: tag? cheers > --- > arch/powerpc/mm/hugetlbpage-radix.c | 5 +++-- > arch/powerpc/mm/slice.c | 10 ++++++---- > 2 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/mm/hugetlbpage-radix.c > b/arch/powerpc/mm/hugetlbpage-radix.c > index 2486bee0f93e..97c7a39ebc00 100644 > --- a/arch/powerpc/mm/hugetlbpage-radix.c > +++ b/arch/powerpc/mm/hugetlbpage-radix.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <linux/mm.h> > #include <linux/hugetlb.h> > +#include <linux/security.h> > #include <asm/pgtable.h> > #include <asm/pgalloc.h> > #include <asm/cacheflush.h> > @@ -73,7 +74,7 @@ radix__hugetlb_get_unmapped_area(struct file *file, > unsigned long addr, > if (addr) { > addr = ALIGN(addr, huge_page_size(h)); > vma = find_vma(mm, addr); > - if (high_limit - len >= addr && > + if (high_limit - len >= addr && addr >= mmap_min_addr && > (!vma || addr + len <= vm_start_gap(vma))) > return addr; > } > @@ -83,7 +84,7 @@ radix__hugetlb_get_unmapped_area(struct file *file, > unsigned long addr, > */ > info.flags = VM_UNMAPPED_AREA_TOPDOWN; > info.length = len; > - info.low_limit = PAGE_SIZE; > + info.low_limit = max(PAGE_SIZE, mmap_min_addr); > info.high_limit = mm->mmap_base + (high_limit - DEFAULT_MAP_WINDOW); > info.align_mask = PAGE_MASK & ~huge_page_mask(h); > info.align_offset = 0; > diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c > index 06898c13901d..aec91dbcdc0b 100644 > --- a/arch/powerpc/mm/slice.c > +++ b/arch/powerpc/mm/slice.c > @@ -32,6 +32,7 @@ > #include <linux/export.h> > #include <linux/hugetlb.h> > #include <linux/sched/mm.h> > +#include <linux/security.h> > #include <asm/mman.h> > #include <asm/mmu.h> > #include <asm/copro.h> > @@ -377,6 +378,7 @@ static unsigned long slice_find_area_topdown(struct > mm_struct *mm, > int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT); > unsigned long addr, found, prev; > struct vm_unmapped_area_info info; > + unsigned long min_addr = max(PAGE_SIZE, mmap_min_addr); > > info.flags = VM_UNMAPPED_AREA_TOPDOWN; > info.length = len; > @@ -393,7 +395,7 @@ static unsigned long slice_find_area_topdown(struct > mm_struct *mm, > if (high_limit > DEFAULT_MAP_WINDOW) > addr += mm->context.slb_addr_limit - DEFAULT_MAP_WINDOW; > > - while (addr > PAGE_SIZE) { > + while (addr > min_addr) { > info.high_limit = addr; > if (!slice_scan_available(addr - 1, available, 0, &addr)) > continue; > @@ -405,8 +407,8 @@ static unsigned long slice_find_area_topdown(struct > mm_struct *mm, > * Check if we need to reduce the range, or if we can > * extend it to cover the previous available slice. > */ > - if (addr < PAGE_SIZE) > - addr = PAGE_SIZE; > + if (addr < min_addr) > + addr = min_addr; > else if (slice_scan_available(addr - 1, available, 0, &prev)) { > addr = prev; > goto prev_slice; > @@ -528,7 +530,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, > unsigned long len, > addr = _ALIGN_UP(addr, page_size); > slice_dbg(" aligned addr=%lx\n", addr); > /* Ignore hint if it's too large or overlaps a VMA */ > - if (addr > high_limit - len || > + if (addr > high_limit - len || addr < mmap_min_addr || > !slice_area_is_free(mm, addr, len)) > addr = 0; > } > -- > 2.20.1