linux-kernel - Re: [PATCH 07/12] x86/compat: get_gate_vma should check for both 32-bit compat and x32

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]

Message-ID: <CALCETrVVmJ4G0_9BkvFCo==Ub8uxoqdnYequbJYwevov65gGzA@mail.gmail.com>
Date:	Mon, 22 Jun 2015 09:19:52 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Brian Gerst <brgerst@...il.com>
Cc:	X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Denys Vlasenko <dvlasenk@...hat.com>
Subject: Re: [PATCH 07/12] x86/compat: get_gate_vma should check for both
 32-bit compat and x32

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