linux-kernel - Re: [PATCHv3 33/33] mm, x86: introduce PR_SET_MAX_VADDR and PR_GET_MAX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Feb 2017 08:50:31 -0800
From:   Andy Lutomirski <luto@...capital.net>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Dmitry Safonov <dsafonov@...tuozzo.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        X86 ML <x86@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Arnd Bergmann <arnd@...db.de>,
        "H. Peter Anvin" <hpa@...or.com>, Andi Kleen <ak@...ux.intel.com>,
        Dave Hansen <dave.hansen@...el.com>,
        linux-arch <linux-arch@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCHv3 33/33] mm, x86: introduce PR_SET_MAX_VADDR and PR_GET_MAX_VADDR

On Fri, Feb 17, 2017 at 6:13 AM, Kirill A. Shutemov
<kirill.shutemov@...ux.intel.com> wrote:
> This patch introduces two new prctl(2) handles to manage maximum virtual
> address available to userspace to map.
>
> On x86, 5-level paging enables 56-bit userspace virtual address space.
> Not all user space is ready to handle wide addresses. It's known that
> at least some JIT compilers use higher bits in pointers to encode their
> information. It collides with valid pointers with 5-level paging and
> leads to crashes.
>
> The patch aims to address this compatibility issue.
>
> MM would use the address as upper limit of virtual address available to
> map by userspace, instead of TASK_SIZE.
>
> The limit will be equal to TASK_SIZE everywhere, but the machine
> with 5-level paging enabled. In this case, the default limit would be
> (1UL << 47) - PAGE_SIZE. It’s current x86-64 TASK_SIZE_MAX with 4-level
> paging which known to be safe.


I think this patch need to be split up.  In particular, the addition
and use of mmap_max_addr() should be its own patch that doesn't change
any semantics.

> diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
> index 306c7e12af55..50bdfd6ab866 100644
> --- a/arch/x86/include/asm/mmu_context.h
> +++ b/arch/x86/include/asm/mmu_context.h
> @@ -117,6 +117,7 @@ static inline int init_new_context(struct task_struct *tsk,
>         }
>         #endif
>         init_new_context_ldt(tsk, mm);
> +       mm->context.max_vaddr = MAX_VADDR_DEFAULT;

Is this actually correct for 32-bit binaries?  Although, given the
stuff Dmitry is working on, it might pay to separately track the
32-bit and 64-bit limits per mm.  If you haven't been following it,
Dmitry is trying to fix a bug in which an explicit 32-bit syscall
(int80 or similar) in an otherwise 64-bit process can allocate a VMA
above 4GB that gets truncated.

Also, why the macro?  Why not just put the number in here?

> -#define TASK_SIZE_MAX  ((1UL << 47) - PAGE_SIZE)
> +#define TASK_SIZE_MAX  ((1UL << __VIRTUAL_MASK_SHIFT) - PAGE_SIZE)

This should be in the

> -#define STACK_TOP              TASK_SIZE
> +#define STACK_TOP              mmap_max_addr()

Off the top of my head, this looks wrong.  The 32-bit check got lost, I think.

> +unsigned long set_max_vaddr(unsigned long addr)
> +{

Perhaps this function could set a different field depending on
is_compat_syscall().


Anyway, can you and Dmitry try to reconcile your patches?