lkml.org 
[lkml]   [2017]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [patch 11/16] x86/ldt: Force access bit for CS/SS
On Tue, Dec 12, 2017 at 9:32 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> From: Peter Zijlstra <peterz@infradead.org>
>
> When mapping the LDT RO the hardware will typically generate write faults
> on first use. These faults can be trapped and the backing pages can be
> modified by the kernel.
>
> There is one exception; IRET will immediately load CS/SS and unrecoverably
> #GP. To avoid this issue access the LDT descriptors used by CS/SS before
> the IRET to userspace.
>
> For this use LAR, which is a safe operation in that it will happily consume
> an invalid LDT descriptor without traps. It gets the CPU to load the
> descriptor and observes the (preset) ACCESS bit.
>
> So far none of the obvious candidates like dosemu/wine/etc. do care about
> the ACCESS bit at all, so it should be rather safe to enforce it.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/x86/entry/common.c | 8 ++++-
> arch/x86/include/asm/desc.h | 2 +
> arch/x86/include/asm/mmu_context.h | 53 +++++++++++++++++++++++--------------
> arch/x86/include/asm/thread_info.h | 4 ++
> arch/x86/kernel/cpu/common.c | 4 +-
> arch/x86/kernel/ldt.c | 30 ++++++++++++++++++++
> arch/x86/mm/tlb.c | 2 -
> arch/x86/power/cpu.c | 2 -
> 8 files changed, 78 insertions(+), 27 deletions(-)
>
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -30,6 +30,7 @@
> #include <asm/vdso.h>
> #include <linux/uaccess.h>
> #include <asm/cpufeature.h>
> +#include <asm/mmu_context.h>
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/syscalls.h>
> @@ -130,8 +131,8 @@ static long syscall_trace_enter(struct p
> return ret ?: regs->orig_ax;
> }
>
> -#define EXIT_TO_USERMODE_LOOP_FLAGS \
> - (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE | \
> +#define EXIT_TO_USERMODE_LOOP_FLAGS \
> + (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE | _TIF_LDT |\
> _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY | _TIF_PATCH_PENDING)
>
> static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
> @@ -171,6 +172,9 @@ static void exit_to_usermode_loop(struct
> /* Disable IRQs and retry */
> local_irq_disable();
>
> + if (cached_flags & _TIF_LDT)
> + ldt_exit_user(regs);

Nope. To the extent that this code actually does anything (which it
shouldn't since you already forced the access bit), it's racy against
flush_ldt() from another thread, and that race will be exploitable for
privilege escalation. It needs to be outside the loopy part.

--Andy

\
 
 \ /
  Last update: 2017-12-12 19:04    [W:0.245 / U:1.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site