x86/ldt: Make the LDT mapping RO

Now that the LDT mapping is in a known area when PAGE_TABLE_ISOLATION is
enabled its a primary target for attacks, if a user space interface fails
to validate a write address correctly. That can never happen, right?

The SDM states:

    If the segment descriptors in the GDT or an LDT are placed in ROM, the
    processor can enter an indefinite loop if software or the processor
    attempts to update (write to) the ROM-based segment descriptors. To
    prevent this problem, set the accessed bits for all segment descriptors
    placed in a ROM. Also, remove operating-system or executive code that
    attempts to modify segment descriptors located in ROM.

So its a valid approach to set the ACCESS bit when setting up the LDT entry
and to map the table RO. Fixup the selftest so it can handle that new mode.

Remove the manual ACCESS bit setter in set_tls_desc() as this is now
pointless. Folded the patch from Peter Ziljstra.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index bc359dd2f7f646a379840a758c20032a86273c4a..85e23bb7b34e31538034ddf9a9e8df773fe7b9b0 100644 (file)
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -21,6 +21,8 @@ static inline void fill_ldt(struct desc_struct *desc, const struct user_desc *in
 
        desc->type              = (info->read_exec_only ^ 1) << 1;
        desc->type             |= info->contents << 2;
+       /* Set the ACCESS bit so it can be mapped RO */
+       desc->type             |= 1;
 
        desc->s                 = 1;
        desc->dpl               = 0x3;

diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c
index 9629c5d8267a8e2e91d0df877aace77eb961a949..579cc4a66fdf663c7e1c727d6272a3a83fbf0648 100644 (file)
--- a/arch/x86/kernel/ldt.c
+++ b/arch/x86/kernel/ldt.c
@@ -158,7 +158,12 @@ map_ldt_struct(struct mm_struct *mm, struct ldt_struct *ldt, int slot)
                ptep = get_locked_pte(mm, va, &ptl);
                if (!ptep)
                        return -ENOMEM;
-               pte = pfn_pte(pfn, __pgprot(__PAGE_KERNEL & ~_PAGE_GLOBAL));
+               /*
+                * Map it RO so the easy to find address is not a primary
+                * target via some kernel interface which misses a
+                * permission check.
+                */
+               pte = pfn_pte(pfn, __pgprot(__PAGE_KERNEL_RO & ~_PAGE_GLOBAL));
                set_pte_at(mm, va, ptep, pte);
                pte_unmap_unlock(ptep, ptl);
        }

diff --git a/arch/x86/kernel/tls.c b/arch/x86/kernel/tls.c
index 9a9c9b076955dd493c7de80b8d814a27dd0fceb7..a5b802a1221272402b344d75ccf94bcff4b69b38 100644 (file)
--- a/arch/x86/kernel/tls.c
+++ b/arch/x86/kernel/tls.c
@@ -93,17 +93,10 @@ static void set_tls_desc(struct task_struct *p, int idx,
        cpu = get_cpu();
 
        while (n-- > 0) {
-               if (LDT_empty(info) || LDT_zero(info)) {
+               if (LDT_empty(info) || LDT_zero(info))
                        memset(desc, 0, sizeof(*desc));
-               } else {
+               else
                        fill_ldt(desc, info);
-
-                       /*
-                        * Always set the accessed bit so that the CPU
-                        * doesn't try to write to the (read-only) GDT.
-                        */
-                       desc->type |= 1;
-               }
                ++info;
                ++desc;
        }

diff --git a/tools/testing/selftests/x86/ldt_gdt.c b/tools/testing/selftests/x86/ldt_gdt.c
index 0304ffb714f272a5646ff8134abdae92f491c5ad..1aef72df20a112a30c17f2e567ca8b972a550104 100644 (file)
--- a/tools/testing/selftests/x86/ldt_gdt.c
+++ b/tools/testing/selftests/x86/ldt_gdt.c
@@ -122,8 +122,7 @@ static void check_valid_segment(uint16_t index, int ldt,
         * NB: Different Linux versions do different things with the
         * accessed bit in set_thread_area().
         */
-       if (ar != expected_ar &&
-           (ldt || ar != (expected_ar | AR_ACCESSED))) {
+       if (ar != expected_ar && ar != (expected_ar | AR_ACCESSED)) {
                printf("[FAIL]\t%s entry %hu has AR 0x%08X but expected 0x%08X\n",
                       (ldt ? "LDT" : "GDT"), index, ar, expected_ar);
                nerrs++;