From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755936AbcEYRTX (ORCPT ); Wed, 25 May 2016 13:19:23 -0400 Received: from mail-wm0-f51.google.com ([74.125.82.51]:35628 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754167AbcEYRTW (ORCPT ); Wed, 25 May 2016 13:19:22 -0400 MIME-Version: 1.0 In-Reply-To: References: From: Kees Cook Date: Wed, 25 May 2016 10:19:19 -0700 X-Google-Sender-Auth: zdt8Z63JMODlndUaGiP0pg8pmNA Message-ID: Subject: Re: [PATCH 0/7] x86: uaccess hardening, easy part To: Brian Gerst Cc: Andy Lutomirski , "the arch/x86 maintainers" , Linux Kernel Mailing List , Borislav Petkov Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 24, 2016 at 8:55 PM, Brian Gerst wrote: > On Tue, May 24, 2016 at 6:48 PM, Andy Lutomirski wrote: >> This series hardens x86's uaccess code a bit. It adds warnings for >> some screwups, adds an OOPS for a major exploitable screwup, and it >> improves debuggability a bit by indicating non-default fs in oopses. >> >> It shouldn't cause any new OOPSes except in the particularly >> dangerous case where the kernel faults on a kernel address under >> USER_DS, which indicates that an access_ok is missing and is likely >> to be easily exploitable -- OOPSing will make it harder to exploit. >> >> I have some draft patches to force OOPSes on user address accesses >> under KERNEL_DS (which is a big no-no), but I'd rather make those >> warn instead of OOPSing, and I don't have a good implementation of >> that yet. Those patches aren't part of this series. >> >> Andy Lutomirski (7): >> x86/xen: Simplify set_aliased_prot >> x86/extable: Pass error_code and an extra unsigned long to exhandlers >> x86/uaccess: Give uaccess faults their own handler >> x86/dumpstack: If addr_limit is non-default, display it >> x86/uaccess: Warn on uaccess faults other than #PF >> x86/uaccess: Don't fix up USER_DS uaccess faults to kernel addresses >> x86/uaccess: OOPS or warn on a fault with KERNEL_DS and >> !pagefault_disabled() >> >> arch/x86/include/asm/uaccess.h | 19 ++++--- >> arch/x86/kernel/cpu/mcheck/mce.c | 2 +- >> arch/x86/kernel/dumpstack_32.c | 4 ++ >> arch/x86/kernel/dumpstack_64.c | 5 ++ >> arch/x86/kernel/kprobes/core.c | 6 +- >> arch/x86/kernel/traps.c | 6 +- >> arch/x86/lib/getuser.S | 12 ++-- >> arch/x86/lib/putuser.S | 10 ++-- >> arch/x86/mm/extable.c | 120 ++++++++++++++++++++++++++++++++++----- >> arch/x86/mm/fault.c | 2 +- >> arch/x86/xen/enlighten.c | 4 +- >> 11 files changed, 145 insertions(+), 45 deletions(-) > > I'd also like to see the use of set_fs() (which has been grossly > misnamed since ancient versions of Linux) significantly reduced. Many > of these uses are in compat syscalls, which do: > - read the user memory > - convert it to the native format > - call set_fs(KERNEL_DS) > - pass it to the native syscall which does another user copy. > > By separating the core syscall code from the userspace accesses so > that it only touches kernel memory, you can eliminate the set_fs() and > the extra copy from the compat case. > > I had started work on this a while back but never finished it. I'll > look at bringing it up to date. Yes, please! This kind of compat handling is very wrong. :( We've had nothing but problems from having such a large piece of code running under KERNEL_DS. -Kees -- Kees Cook Chrome OS & Brillo Security