Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC][PATCH 7/8] x86/mm/vsyscall: consider vsyscall page part of user address space
From:   Andy Lutomirski <luto@amacapital.net>
In-Reply-To: <20180907194902.63F36CFE@viggo.jf.intel.com>
Date:   Fri, 7 Sep 2018 15:58:19 -0700
Cc:     linux-kernel@vger.kernel.org, sean.j.christopherson@intel.com,
        peterz@infradead.org, tglx@linutronix.de, x86@kernel.org,
        luto@kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <17109C42-4679-4042-A22E-E686FCB96012@amacapital.net>
References: <20180907194852.3C351B82@viggo.jf.intel.com> <20180907194902.63F36CFE@viggo.jf.intel.com>
To:     Dave Hansen <dave.hansen@linux.intel.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Sep 7, 2018, at 12:49 PM, Dave Hansen <dave.hansen@linux.intel.com> wro=
te:
>=20
>=20
> From: Dave Hansen <dave.hansen@linux.intel.com>
>=20
> 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.
>=20
> 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().

I assume the motivation is that you want to simplify the kernel error path. I=
f so, can you mention this?

The patch itself is Reviewed-by: Andy Lutomirski <luto@kernel.org>, although=
 adding an unlikely() somewhere might be nice.=20

>=20
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: x86@kernel.org
> Cc: Andy Lutomirski <luto@kernel.org>
> ---
>=20
> b/arch/x86/mm/fault.c |   36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>=20
> diff -puN arch/x86/mm/fault.c~vsyscall-is-user-address-space arch/x86/mm/f=
ault.c
> --- a/arch/x86/mm/fault.c~vsyscall-is-user-address-space    2018-09-07 11:=
21:48.720751896 -0700
> +++ b/arch/x86/mm/fault.c    2018-09-07 11:21:48.724751896 -0700
> @@ -873,18 +873,6 @@ __bad_area_nosemaphore(struct pt_regs *r
>        if (is_errata100(regs, address))
>            return;
>=20
> -#ifdef CONFIG_X86_64
> -        /*
> -         * Instruction fetch faults in the vsyscall page might need
> -         * emulation.
> -         */
> -        if (unlikely((error_code & X86_PF_INSTR) &&
> -                 is_vsyscall_vaddr(address))) {
> -            if (emulate_vsyscall(regs, address))
> -                return;
> -        }
> -#endif
> -
>        /*
>         * To avoid leaking information about the kernel page table
>         * layout, pretend that user-mode accesses to kernel addresses
> @@ -1192,6 +1180,13 @@ access_error(unsigned long error_code, s
>=20
> 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;
> +
>    return address >=3D TASK_SIZE_MAX;
> }
>=20
> @@ -1357,6 +1352,23 @@ void do_user_addr_space_fault(struct pt_
>    if (sw_error_code & X86_PF_INSTR)
>        flags |=3D FAULT_FLAG_INSTRUCTION;
>=20
> +#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
> +     */
> +    if (unlikely((sw_error_code & X86_PF_INSTR) &&
> +             is_vsyscall_vaddr(address))) {
> +        if (emulate_vsyscall(regs, address))
> +            return;
> +    }
> +#endif
> +
>    /*
>     * Kernel-mode access to the user address space should only occur
>     * inside well-defined areas of code listed in the exception
> _