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

[Andy Lutomirski]

> On Sep 7, 2018, at 12:49 PM, Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> wrote:
> 
> 
> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> 
> 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().

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

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

> 
> Signed-off-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Cc: "Peter Zijlstra (Intel)" <peterz@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: Andy Lutomirski <luto@xxxxxxxxxx>
> ---
> 
> b/arch/x86/mm/fault.c |   36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff -puN arch/x86/mm/fault.c~vsyscall-is-user-address-space arch/x86/mm/fault.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;
> 
> -#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
> 
> 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 >= TASK_SIZE_MAX;
> }
> 
> @@ -1357,6 +1352,23 @@ void do_user_addr_space_fault(struct pt_
>    if (sw_error_code & X86_PF_INSTR)
>        flags |= FAULT_FLAG_INSTRUCTION;
> 
> +#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
> _