Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753046AbbFVQUV (ORCPT ); Mon, 22 Jun 2015 12:20:21 -0400 Received: from mail-lb0-f171.google.com ([209.85.217.171]:34638 "EHLO mail-lb0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751896AbbFVQUO (ORCPT ); Mon, 22 Jun 2015 12:20:14 -0400 MIME-Version: 1.0 In-Reply-To: <1434974121-32575-8-git-send-email-brgerst@gmail.com> 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 09:19:52 -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: 2368 Lines: 62 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/