linux-kernel - Re: [patch V163 39/51] x86/pti: Put the LDT in its own PGD if PTI is on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171218134648.73xsa6kpos6t6jnt@hirez.programming.kicks-ass.net>
Date:   Mon, 18 Dec 2017 14:46:48 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Lutomirsky <luto@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Borislav Petkov <bpetkov@...e.de>,
        Greg KH <gregkh@...uxfoundation.org>, keescook@...gle.com,
        hughd@...gle.com, Brian Gerst <brgerst@...il.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        Rik van Riel <riel@...hat.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        David Laight <David.Laight@...lab.com>,
        Eduardo Valentin <eduval@...zon.com>, aliguori@...zon.com,
        Will Deacon <will.deacon@....com>, daniel.gruss@...k.tugraz.at,
        Kees Cook <keescook@...omium.org>,
        Borislav Petkov <bp@...en8.de>,
        "Kirill A. Shutemov" <kirill@...temov.name>
Subject: Re: [patch V163 39/51] x86/pti: Put the LDT in its own PGD if PTI is
 on

On Mon, Dec 18, 2017 at 12:42:54PM +0100, Thomas Gleixner wrote:

> This will significantly slow down LDT users, but that shouldn't matter for
> important workloads -- the LDT is only used by DOSEMU(2), Wine, and very
> old libc implementations.


> +/*
> + * If PTI is enabled, this maps the LDT into the kernelmode and
> + * usermode tables for the given mm.
> + *
> + * There is no corresponding unmap function.  Even if the LDT is freed, we
> + * leave the PTEs around until the slot is reused or the mm is destroyed.
> + * This is harmless: the LDT is always in ordinary memory, and no one will
> + * access the freed slot.
> + *
> + * If we wanted to unmap freed LDTs, we'd also need to do a flush to make
> + * it useful, and the flush would slow down modify_ldt().
> + */
> +static int
> +map_ldt_struct(struct mm_struct *mm, struct ldt_struct *ldt, int slot)
> +{
> +#ifdef CONFIG_PAGE_TABLE_ISOLATION
> +	bool is_vmalloc, had_top_level_entry;
> +	unsigned long va;
> +	spinlock_t *ptl;
> +	pgd_t *pgd;
> +	int i;
> +
> +	if (!static_cpu_has(X86_FEATURE_PTI))
> +		return 0;
> +
> +	/*
> +	 * Any given ldt_struct should have map_ldt_struct() called at most
> +	 * once.
> +	 */
> +	WARN_ON(ldt->slot != -1);
> +
> +	/*
> +	 * Did we already have the top level entry allocated?  We can't
> +	 * use pgd_none() for this because it doens't do anything on
> +	 * 4-level page table kernels.
> +	 */
> +	pgd = pgd_offset(mm, LDT_BASE_ADDR);
> +	had_top_level_entry = (pgd->pgd != 0);
> +
> +	is_vmalloc = is_vmalloc_addr(ldt->entries);
> +
> +	for (i = 0; i * PAGE_SIZE < ldt->nr_entries * LDT_ENTRY_SIZE; i++) {
> +		unsigned long offset = i << PAGE_SHIFT;
> +		const void *src = (char *)ldt->entries + offset;
> +		unsigned long pfn;
> +		pte_t pte, *ptep;
> +
> +		va = (unsigned long)ldt_slot_va(slot) + offset;
> +		pfn = is_vmalloc ? vmalloc_to_pfn(src) :
> +			page_to_pfn(virt_to_page(src));
> +		/*
> +		 * Treat the PTI LDT range as a *userspace* range.
> +		 * get_locked_pte() will allocate all needed pagetables
> +		 * and account for them in this mm.
> +		 */
> +		ptep = get_locked_pte(mm, va, &ptl);
> +		if (!ptep)
> +			return -ENOMEM;
> +		pte = pfn_pte(pfn, __pgprot(__PAGE_KERNEL & ~_PAGE_GLOBAL));
> +		set_pte_at(mm, va, ptep, pte);
> +		pte_unmap_unlock(ptep, ptl);
> +	}
> +
> +	if (mm->context.ldt) {
> +		/*
> +		 * We already had an LDT.  The top-level entry should already
> +		 * have been allocated and synchronized with the usermode
> +		 * tables.
> +		 */
> +		WARN_ON(!had_top_level_entry);
> +		if (static_cpu_has(X86_FEATURE_PTI))
> +			WARN_ON(!kernel_to_user_pgdp(pgd)->pgd);
> +	} else {
> +		/*
> +		 * This is the first time we're mapping an LDT for this process.
> +		 * Sync the pgd to the usermode tables.
> +		 */
> +		WARN_ON(had_top_level_entry);
> +		if (static_cpu_has(X86_FEATURE_PTI)) {
> +			WARN_ON(kernel_to_user_pgdp(pgd)->pgd);
> +			set_pgd(kernel_to_user_pgdp(pgd), *pgd);
> +		}
> +	}
> +
> +	va = (unsigned long)ldt_slot_va(slot);
> +	flush_tlb_mm_range(mm, va, va + LDT_SLOT_STRIDE, 0);
> +
> +	ldt->slot = slot;
> +#endif
> +	return 0;
> +}

So if I understand this right, we need to remap+tlb-flush every time we
flip LDTs because this still uses the old vmalloc-a-new-ldt thing?

Should we base this on top of the ldt_mapping stuff tglx did? Such that
we keep the 2 arrays of pages immutable and don't need no tlb flushes?