linux-kernel - Re: [RFC][PATCH 7/8] x86/mm/vsyscall: consider vsyscall page part of user address space

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: <CAG48ez2e8yJJjHeKRTbu46=5mbssZo84iMinBx6B5BgziDJiVQ@mail.gmail.com>
Date:   Sat, 8 Sep 2018 03:16:09 +0200
From:   Jann Horn <jannh@...gle.com>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     kernel list <linux-kernel@...r.kernel.org>,
        sean.j.christopherson@...el.com,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [RFC][PATCH 7/8] x86/mm/vsyscall: consider vsyscall page part of
 user address space

On Sat, Sep 8, 2018 at 2:28 AM Dave Hansen <dave.hansen@...ux.intel.com> wrote:
> The vsyscall page is weird.  It is in what is traditionally part of the
> kernel address space.  But, it has user permissions and we handle faults
> on it like we would on a user page: interrupts on.
>
> Right now, we handle vsyscall emulation in the "bad_area" code, which
> is used for both user-address-space and kernel-address-space faults.  Move
> the handling to the user-address-space code *only* and ensure we get there
> by "excluding" the vsyscall page from the kernel address space via a check
> in fault_in_kernel_space().
[...]
>  static int fault_in_kernel_space(unsigned long address)
>  {
> +       /*
> +        * The vsyscall page is at an address above TASK_SIZE_MAX,
> +        * but is not considered part of the kernel address space.
> +        */
> +       if (is_vsyscall_vaddr(address))
> +               return false;

I think something should check for "#ifdef CONFIG_X86_64"? 32-bit
doesn't have a vsyscall page, right? And this code probably shouldn't
veer off into the userspace-area fault handling code for addresses in
the range 0xff600000-0xff600fff... what is in that region on 32-bit?
Modules or something like that?
Maybe change is_vsyscall_vaddr() so that it always returns false on
32-bit, or put both the definition of is_vsyscall_vaddr() and this
code behind #ifdef guards.
And, in a separate patch, maybe also #ifdef-guard the definition of
VSYSCALL_ADDR in vsyscall.h? Nothing good is going to result from
making a garbage VSYSCALL_ADDR available to 32-bit code.

> +#ifdef CONFIG_X86_64
> +       /*
> +        * Instruction fetch faults in the vsyscall page might need
> +        * emulation.  The vsyscall page is at a high address
> +        * (>PAGE_OFFSET), but is considered to be part of the user
> +        * address space.
> +        *
> +        * The vsyscall page does not have a "real" VMA, so do this
> +        * emulation before we go searching for VMAse

"VMAse"? Is that a typo?