Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751094AbbFVRMr (ORCPT ); Mon, 22 Jun 2015 13:12:47 -0400 Received: from mail-lb0-f172.google.com ([209.85.217.172]:34579 "EHLO mail-lb0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750838AbbFVRMj (ORCPT ); Mon, 22 Jun 2015 13:12:39 -0400 MIME-Version: 1.0 In-Reply-To: References: <1434974121-32575-1-git-send-email-brgerst@gmail.com> <1434974121-32575-8-git-send-email-brgerst@gmail.com> From: Andy Lutomirski Date: Mon, 22 Jun 2015 10:12:17 -0700 Message-ID: Subject: Re: [PATCH 07/12] x86/compat: get_gate_vma should check for both 32-bit compat and x32 To: Brian Gerst Cc: X86 ML , "linux-kernel@vger.kernel.org" , Ingo Molnar , "H. Peter Anvin" , Denys Vlasenko Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3000 Lines: 82 On Mon, Jun 22, 2015 at 10:05 AM, Brian Gerst wrote: > On Mon, Jun 22, 2015 at 12:19 PM, Andy Lutomirski wrote: >> On Mon, Jun 22, 2015 at 4:55 AM, Brian Gerst wrote: >>> Change this to CONFIG_COMPAT so both 32-bit compat and x32 will do the >>> check. >>> >>> Signed-off-by: Brian Gerst >>> --- >>> arch/x86/entry/vsyscall/vsyscall_64.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c >>> index 2dcc6ff..26a46f4 100644 >>> --- a/arch/x86/entry/vsyscall/vsyscall_64.c >>> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c >>> @@ -290,7 +290,7 @@ static struct vm_area_struct gate_vma = { >>> >>> struct vm_area_struct *get_gate_vma(struct mm_struct *mm) >>> { >>> -#ifdef CONFIG_IA32_EMULATION >>> +#ifdef CONFIG_COMPAT >>> if (!mm || mm->context.ia32_compat) >>> return NULL; >>> #endif >> >> >> This makes little sense to me. >> >> First, why is the !mm check guarded by any ifdef at all? If this said >> "if (mm && mm->...)", it would make no sense. >> >> Second, and more importantly, what does mm->context.ia32_compat mean >> in the new less-nonsensical regime? The flag itself is defined in a >> way that makes no sense (it's either 0, TIF_X32, or TIF_IA32 -- >> presumably it should be an enum). There aren't a whole lot of things >> that care -- it's just this check and some uprobe thing. At some >> point, mpx will start caring, too. There's also TIF_ADDR32, which is >> similarly ridiculous (why isn't it part of mm->context?, and why does >> it exist at all). >> >> I think that the questions we want to be able to answer are: >> >> 1. Is this mm intended to be addressable using 32 bits? If so, we >> should probably not show the vsyscall area in /proc/self/maps. >> >> 2. Is this mm's mpx context intended to be used by 32-bit userspace? >> (That's real 32 bit, not x32 -- x32 is certainly 64-bit as far as MPX >> is concerned.) >> >> 3. Is the current mmap call intended to return something in the low 32 >> bits? Presumably that should depend only on the mmap call's bitness >> (is_compat_task, etc, which we really need to rename to something like >> in_compat_syscall). >> >> I find myself wondering whether there is any legitimate reason that >> TASK_SIZE isn't the same thing as TASK_SIZE_MAX... >> >> Anyway, your patch is probably fine -- you're not actually making >> anything worse. >> >> --Andy > > I agree there are more cleanups needed here. But that's probably a > whole patch series in itself. This patch is intended to be no code > change when X32 doesn't depend on 32-bit compat anymore. Fair enough. Acked-by: Andy Lutomirski > > -- > Brian Gerst -- Andy Lutomirski AMA Capital Management, LLC -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/