On Mon, Jun 22, 2015 at 12:19 PM, Andy Lutomirski <l...@amacapital.net> wrote: > On Mon, Jun 22, 2015 at 4:55 AM, Brian Gerst <brge...@gmail.com> wrote: >> Change this to CONFIG_COMPAT so both 32-bit compat and x32 will do the >> check. >> >> Signed-off-by: Brian Gerst <brge...@gmail.com> >> --- >> 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. -- Brian Gerst -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/