Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754247AbdLHQU2 (ORCPT ); Fri, 8 Dec 2017 11:20:28 -0500 Received: from bombadil.infradead.org ([65.50.211.133]:48499 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754040AbdLHQUZ (ORCPT ); Fri, 8 Dec 2017 11:20:25 -0500 Date: Fri, 8 Dec 2017 17:20:12 +0100 From: Peter Zijlstra To: Andy Lutomirski Cc: Thomas Gleixner , Ingo Molnar , Andy Lutomirski , Borislav Petkov , X86 ML , "linux-kernel@vger.kernel.org" , Brian Gerst , David Laight , Kees Cook Subject: Re: [PATCH] LDT improvements Message-ID: <20171208162012.6bi6h5yko27vaoc5@hirez.programming.kicks-ass.net> References: <48fe5cf1382d6a95c7b1837415882edcc81a9781.1512631324.git.luto@kernel.org> <20171207124347.p7kdj7q4qqs3ivri@pd.tnic> <665F1CA8-D012-4465-B5F7-E81E088847DB@amacapital.net> <20171208073454.dicyefwncsihq7sm@gmail.com> <6363C18D-D84A-40E4-8ED4-FE996609467B@amacapital.net> <20171208140654.bths5fx2yxmndm42@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171208140654.bths5fx2yxmndm42@hirez.programming.kicks-ass.net> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2684 Lines: 82 On Fri, Dec 08, 2017 at 03:06:54PM +0100, Peter Zijlstra wrote: > On Fri, Dec 08, 2017 at 05:20:00AM -0800, Andy Lutomirski wrote: > > > > > > The error code of such an access is always 0x03. So I added a special > > > handler, which checks whether the address is in the LDT map range and > > > verifies that the access bit in the descriptor is 0. If that's the case it > > > sets it and returns. If not, the thing dies. That works. > > > > What if you are in kernel mode and try to return to a context with SS > > or CS pointing to a non-accessed segment? Or what if you try to > > schedule to a context with fs or, worse, gs pointing to such a > > segment? > > How would that be different from setting a 'crap' GS in modify_ldt() and > then returning from the syscall? That is something we should be able to > deal with already, no? > > Is this something ldt_gdt.c already tests? The current "Test GS" is in > test_gdt_invalidation() which seems to suggest not. > > Could we get a testcase for the exact situation you worry about? I'm not > sure I'd trust myself to get it right, all this LDT magic is new to me. I ended up with the below; that loads something in LDT-2, sets GS to LDT-2 and then again loads that same thing in LDT-2. AFAIU calling modify_ldt will clear that ACCESSED bit, so this would then trigger that on the return to user from modify_ldt, no? Seems to work with tglx's patches. diff --git a/tools/testing/selftests/x86/ldt_gdt.c b/tools/testing/selftests/x86/ldt_gdt.c index 66e5ce5b91f0..d46a620c3734 100644 --- a/tools/testing/selftests/x86/ldt_gdt.c +++ b/tools/testing/selftests/x86/ldt_gdt.c @@ -242,6 +242,36 @@ static void fail_install(struct user_desc *desc) } } +static void do_ldt_gs_test(void) +{ + unsigned short prev_sel, sel = (2 << 3) | (1 << 2) | 3; + + low_user_desc->entry_number = 2; + + safe_modify_ldt(1, low_user_desc, sizeof(*low_user_desc)); + + /* + * syscall (eax) 123 - modify_ldt + * (ebx) - func + * (ecx) - ptr + * (edx) - bytecount + */ + + int eax = 123; + int ebx = 1; + int ecx = (unsigned int)low_user_desc; + int edx = sizeof(struct user_desc); + + asm volatile ("movw %%gs, %[prev_sel]\n\t" + "movw %[sel], %%gs\n\t" + "int $0x80\n\t" + "mov %[prev_sel], %%gs" + : [prev_sel] "=&R" (prev_sel), [sel] "+R" (sel), "+a" (eax) + : "b" (ebx), "c" (ecx), "d" (edx) + : INT80_CLOBBERS); + +} + static void do_simple_tests(void) { struct user_desc desc = { @@ -919,6 +946,8 @@ int main(int argc, char **argv) setup_counter_page(); setup_low_user_desc(); + do_ldt_gs_test(); + do_simple_tests(); do_multicpu_tests();