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();
