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

From: Andy Lutomirski
Date: Fri Sep 07 2018 - 18:58:26 EST




> 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
> _