[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <201906271028.E2A4538@keescook>
Date: Thu, 27 Jun 2019 10:28:26 -0700
From: Kees Cook <keescook@...omium.org>
To: Andy Lutomirski <luto@...nel.org>
Cc: x86@...nel.org, LKML <linux-kernel@...r.kernel.org>,
Florian Weimer <fweimer@...hat.com>,
Jann Horn <jannh@...gle.com>
Subject: Re: [PATCH v2 3/8] x86/vsyscall: Show something useful on a read
fault
On Wed, Jun 26, 2019 at 09:45:04PM -0700, Andy Lutomirski wrote:
> Signed-off-by: Andy Lutomirski <luto@...nel.org>
Reviewed-by: Kees Cook <keescook@...omium.org>
-Kees
> ---
> arch/x86/entry/vsyscall/vsyscall_64.c | 19 ++++++++++++++++++-
> arch/x86/include/asm/vsyscall.h | 6 ++++--
> arch/x86/mm/fault.c | 11 +++++------
> 3 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
> index fedd7628f3a6..9c58ab807aeb 100644
> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> @@ -117,7 +117,8 @@ static bool write_ok_or_segv(unsigned long ptr, size_t size)
> }
> }
>
> -bool emulate_vsyscall(struct pt_regs *regs, unsigned long address)
> +bool emulate_vsyscall(unsigned long error_code,
> + struct pt_regs *regs, unsigned long address)
> {
> struct task_struct *tsk;
> unsigned long caller;
> @@ -126,6 +127,22 @@ bool emulate_vsyscall(struct pt_regs *regs, unsigned long address)
> long ret;
> unsigned long orig_dx;
>
> + /* Write faults or kernel-privilege faults never get fixed up. */
> + if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
> + return false;
> +
> + if (!(error_code & X86_PF_INSTR)) {
> + /* Failed vsyscall read */
> + if (vsyscall_mode == EMULATE)
> + return false;
> +
> + /*
> + * User code tried and failed to read the vsyscall page.
> + */
> + warn_bad_vsyscall(KERN_INFO, regs, "vsyscall read attempt denied -- look up the vsyscall kernel parameter if you need a workaround");
> + return false;
> + }
> +
> /*
> * No point in checking CS -- the only way to get here is a user mode
> * trap to a high address, which means that we're in 64-bit user code.
> diff --git a/arch/x86/include/asm/vsyscall.h b/arch/x86/include/asm/vsyscall.h
> index b986b2ca688a..ab60a71a8dcb 100644
> --- a/arch/x86/include/asm/vsyscall.h
> +++ b/arch/x86/include/asm/vsyscall.h
> @@ -13,10 +13,12 @@ extern void set_vsyscall_pgtable_user_bits(pgd_t *root);
> * Called on instruction fetch fault in vsyscall page.
> * Returns true if handled.
> */
> -extern bool emulate_vsyscall(struct pt_regs *regs, unsigned long address);
> +extern bool emulate_vsyscall(unsigned long error_code,
> + struct pt_regs *regs, unsigned long address);
> #else
> static inline void map_vsyscall(void) {}
> -static inline bool emulate_vsyscall(struct pt_regs *regs, unsigned long address)
> +static inline bool emulate_vsyscall(unsigned long error_code,
> + struct pt_regs *regs, unsigned long address)
> {
> return false;
> }
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 46df4c6aae46..288a5462076f 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -1369,16 +1369,15 @@ void do_user_addr_fault(struct pt_regs *regs,
>
> #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.
> + * 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 VMAs.
> */
> - if ((hw_error_code & X86_PF_INSTR) && is_vsyscall_vaddr(address)) {
> - if (emulate_vsyscall(regs, address))
> + if (is_vsyscall_vaddr(address)) {
> + if (emulate_vsyscall(hw_error_code, regs, address))
> return;
> }
> #endif
> --
> 2.21.0
>
--
Kees Cook
Powered by blists - more mailing lists