Re: [PATCHv3 33/33] mm, x86: introduce PR_SET_MAX_VADDR and PR_GET_MAX_VADDR
From: Andy Lutomirski <hidden>
Date: 2017-02-17 16:50:55
Also in:
linux-arch, linux-mm, lkml
Possibly related (same subject, not in this thread)
- 2017-02-21 · Re: [PATCHv3 33/33] mm, x86: introduce PR_SET_MAX_VADDR and PR_GET_MAX_VADDR · Michael Pratt <hidden>
- 2017-02-21 · Re: [PATCHv3 33/33] mm, x86: introduce PR_SET_MAX_VADDR and PR_GET_MAX_VADDR · Michael Pratt <hidden>
On Fri, Feb 17, 2017 at 6:13 AM, Kirill A. Shutemov [off-list ref] 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.
quoted hunk ↗ jump to hunk
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?