On Wed 15-11-17 17:36:06, Kirill A. Shutemov wrote:
> In case of 5-level paging, the kernel does not place any mapping above
> 47-bit, unless userspace explicitly asks for it.
>
> Userspace can request an allocation from the full address space by
> specifying the mmap address hint above 47-bit.
>
> Nicholas noticed that the current implementation violates this interface:
>
> If user space requests a mapping at the end of the 47-bit address space
> with a length which causes the mapping to cross the 47-bit border
> (DEFAULT_MAP_WINDOW), then the vma is partially in the address space
> below and above.
>
> Sanity check the mmap address hint so that start and end of the resulting
> vma are on the same side of the 47-bit border. If that's not the case fall
> back to the code path which ignores the address hint and allocate from the
> regular address space below 47-bit.
>
> [ tglx: Moved the address check to a function and massaged comment and
> changelog ]
>
> Reported-by: Nicholas Piggin <npig...@gmail.com>
> Signed-off-by: Kirill A. Shutemov <kirill.shute...@linux.intel.com>
> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
FWIW
Acked-by: Michal Hocko <mho...@suse.com>
> ---
> arch/x86/include/asm/elf.h | 1 +
> arch/x86/kernel/sys_x86_64.c | 10 +++++++---
> arch/x86/mm/hugetlbpage.c | 11 ++++++++---
> arch/x86/mm/mmap.c | 46
> ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> index 3a091cea36c5..0d157d2a1e2a 100644
> --- a/arch/x86/include/asm/elf.h
> +++ b/arch/x86/include/asm/elf.h
> @@ -309,6 +309,7 @@ static inline int mmap_is_ia32(void)
> extern unsigned long task_size_32bit(void);
> extern unsigned long task_size_64bit(int full_addr_space);
> extern unsigned long get_mmap_base(int is_legacy);
> +extern bool mmap_address_hint_valid(unsigned long addr, unsigned long len);
>
> #ifdef CONFIG_X86_32
>
> diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
> index a63fe77b3217..676774b9bb8d 100644
> --- a/arch/x86/kernel/sys_x86_64.c
> +++ b/arch/x86/kernel/sys_x86_64.c
> @@ -188,6 +188,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const
> unsigned long addr0,
> if (len > TASK_SIZE)
> return -ENOMEM;
>
> + /* No address checking. See comment at mmap_address_hint_valid() */
> if (flags & MAP_FIXED)
> return addr;
>
> @@ -197,12 +198,15 @@ arch_get_unmapped_area_topdown(struct file *filp, const
> unsigned long addr0,
>
> /* requesting a specific address */
> if (addr) {
> - addr = PAGE_ALIGN(addr);
> + addr &= PAGE_MASK;
> + if (!mmap_address_hint_valid(addr, len))
> + goto get_unmapped_area;
> +
> vma = find_vma(mm, addr);
> - if (TASK_SIZE - len >= addr &&
> - (!vma || addr + len <= vm_start_gap(vma)))
> + if (!vma || addr + len <= vm_start_gap(vma))
> return addr;
> }
> +get_unmapped_area:
>
> info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> info.length = len;
> diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
> index 8ae0000cbdb3..00b296617ca4 100644
> --- a/arch/x86/mm/hugetlbpage.c
> +++ b/arch/x86/mm/hugetlbpage.c
> @@ -158,6 +158,7 @@ hugetlb_get_unmapped_area(struct file *file, unsigned
> long addr,
> if (len > TASK_SIZE)
> return -ENOMEM;
>
> + /* No address checking. See comment at mmap_address_hint_valid() */
> if (flags & MAP_FIXED) {
> if (prepare_hugepage_range(file, addr, len))
> return -EINVAL;
> @@ -165,12 +166,16 @@ hugetlb_get_unmapped_area(struct file *file, unsigned
> long addr,
> }
>
> if (addr) {
> - addr = ALIGN(addr, huge_page_size(h));
> + addr &= huge_page_mask(h);
> + if (!mmap_address_hint_valid(addr, len))
> + goto get_unmapped_area;
> +
> vma = find_vma(mm, addr);
> - if (TASK_SIZE - len >= addr &&
> - (!vma || addr + len <= vm_start_gap(vma)))
> + if (!vma || addr + len <= vm_start_gap(vma))
> return addr;
> }
> +
> +get_unmapped_area:
> if (mm->get_unmapped_area == arch_get_unmapped_area)
> return hugetlb_get_unmapped_area_bottomup(file, addr, len,
> pgoff, flags);
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index a99679826846..62285fe77b0f 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -174,3 +174,49 @@ const char *arch_vma_name(struct vm_area_struct *vma)
> return "[mpx]";
> return NULL;
> }
> +
> +/**
> + * mmap_address_hint_valid - Validate the address hint of mmap
> + * @addr: Address hint
> + * @len: Mapping length
> + *
> + * Check whether @addr and @addr + @len result in a valid mapping.
> + *
> + * On 32bit this only checks whether @addr + @len is <= TASK_SIZE.
> + *
> + * On 64bit with 5-level page tables another sanity check is required
> + * because mappings requested by mmap(@addr, 0) which cross the 47-bit
> + * virtual address boundary can cause the following theoretical issue:
> + *
> + * An application calls mmap(addr, 0), i.e. without MAP_FIXED, where @addr
> + * is below the border of the 47-bit address space and @addr + @len is
> + * above the border.
> + *
> + * With 4-level paging this request succeeds, but the resulting mapping
> + * address will always be within the 47-bit virtual address space, because
> + * the hint address does not result in a valid mapping and is
> + * ignored. Hence applications which are not prepared to handle virtual
> + * addresses above 47-bit work correctly.
> + *
> + * With 5-level paging this request would be granted and result in a
> + * mapping which crosses the border of the 47-bit virtual address
> + * space. If the application cannot handle addresses above 47-bit this
> + * will lead to misbehaviour and hard to diagnose failures.
> + *
> + * Therefore ignore address hints which would result in a mapping crossing
> + * the 47-bit virtual address boundary.
> + *
> + * Note, that in the same scenario with MAP_FIXED the behaviour is
> + * different. The request with @addr < 47-bit and @addr + @len > 47-bit
> + * fails on a 4-level paging machine but succeeds on a 5-level paging
> + * machine. It is reasonable to expect that an application does not rely on
> + * the failure of such a fixed mapping request, so the restriction is not
> + * applied.
> + */
> +bool mmap_address_hint_valid(unsigned long addr, unsigned long len)
> +{
> + if (TASK_SIZE - len < addr)
> + return false;
> +
> + return (addr > DEFAULT_MAP_WINDOW) == (addr + len > DEFAULT_MAP_WINDOW);
> +}
> --
> 2.15.0
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"d...@kvack.org";> em...@kvack.org </a>
--
Michal Hocko
SUSE Labs
