On Tue, Dec 12, 2017 at 9:32 AM, Thomas Gleixner <t...@linutronix.de> wrote: > From: Peter Zijlstra <pet...@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) <pet...@infradead.org> > Signed-off-by: Thomas Gleixner <t...@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
