On Tue, May 24, 2016 at 8:55 PM, Brian Gerst <brge...@gmail.com> wrote: > On Tue, May 24, 2016 at 6:48 PM, Andy Lutomirski <l...@kernel.org> 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