From: Dave Hansen <dave.hansen@linux.intel.com>
To: Yu-cheng Yu <yu-cheng.yu@intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-mm@kvack.org, linux-arch@vger.kernel.org,
linux-api@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
Andy Lutomirski <luto@amacapital.net>,
Balbir Singh <bsingharora@gmail.com>,
Cyrill Gorcunov <gorcunov@gmail.com>,
Florian Weimer <fweimer@redhat.com>,
"H.J. Lu" <hjl.tools@gmail.com>, Jann Horn <jannh@google.com>,
Jonathan Corbet <corbet@lwn.net>,
Kees Cook <keescook@chromiun.org>,
Mike Kravetz <mike.kravetz@oracle.com>,
Nadav Amit <nadav.amit@gmail.com>,
Oleg Nesterov <oleg@redhat.com>, Pavel Machek <pavel@ucw.cz>,
Peter Zijlstra <peterz@infradead.org>,
"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
Vedvyas Shanbhogue <vedvyas.shanbhogue@intel.com>
Subject: Re: [RFC PATCH v2 22/27] x86/cet/ibt: User-mode indirect branch tracking support
Date: Tue, 10 Jul 2018 17:11:43 -0700 [thread overview]
Message-ID: <3a7e9ce4-03c6-cc28-017b-d00108459e94@linux.intel.com> (raw)
In-Reply-To: <20180710222639.8241-23-yu-cheng.yu@intel.com>
Is this feature *integral* to shadow stacks? Or, should it just be in a
different series?
> diff --git a/arch/x86/include/asm/cet.h b/arch/x86/include/asm/cet.h
> index d9ae3d86cdd7..71da2cccba16 100644
> --- a/arch/x86/include/asm/cet.h
> +++ b/arch/x86/include/asm/cet.h
> @@ -12,7 +12,10 @@ struct task_struct;
> struct cet_status {
> unsigned long shstk_base;
> unsigned long shstk_size;
> + unsigned long ibt_bitmap_addr;
> + unsigned long ibt_bitmap_size;
> unsigned int shstk_enabled:1;
> + unsigned int ibt_enabled:1;
> };
Is there a reason we're not using pointers here? This seems like the
kind of place that we probably want __user pointers.
> +static unsigned long ibt_mmap(unsigned long addr, unsigned long len)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned long populate;
> +
> + down_write(&mm->mmap_sem);
> + addr = do_mmap(NULL, addr, len, PROT_READ | PROT_WRITE,
> + MAP_ANONYMOUS | MAP_PRIVATE,
> + VM_DONTDUMP, 0, &populate, NULL);
> + up_write(&mm->mmap_sem);
> +
> + if (populate)
> + mm_populate(addr, populate);
> +
> + return addr;
> +}
We're going to have to start consolidating these at some point. We have
at least three of them now, maybe more.
> +int cet_setup_ibt_bitmap(void)
> +{
> + u64 r;
> + unsigned long bitmap;
> + unsigned long size;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_IBT))
> + return -EOPNOTSUPP;
> +
> + size = TASK_SIZE_MAX / PAGE_SIZE / BITS_PER_BYTE;
Just a note: this table is going to be gigantic on 5-level paging
systems, and userspace won't, by default use any of that extra address
space. I think it ends up being a 512GB allocation in a 128TB address
space.
Is that a problem?
On 5-level paging systems, maybe we should just stick it up in the high
part of the address space.
> + bitmap = ibt_mmap(0, size);
> +
> + if (bitmap >= TASK_SIZE_MAX)
> + return -ENOMEM;
> +
> + bitmap &= PAGE_MASK;
We're page-aligning the result of an mmap()? Why?
> + rdmsrl(MSR_IA32_U_CET, r);
> + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> + wrmsrl(MSR_IA32_U_CET, r);
Comments, please. What is this doing, logically? Also, why are we
OR'ing the results into this MSR? What are we trying to preserve?
> + current->thread.cet.ibt_bitmap_addr = bitmap;
> + current->thread.cet.ibt_bitmap_size = size;
> + return 0;
> +}
> +
> +void cet_disable_ibt(void)
> +{
> + u64 r;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_IBT))
> + return;
Does this need a check for being already disabled?
> + rdmsrl(MSR_IA32_U_CET, r);
> + r &= ~(MSR_IA32_CET_ENDBR_EN | MSR_IA32_CET_LEG_IW_EN |
> + MSR_IA32_CET_NO_TRACK_EN);
> + wrmsrl(MSR_IA32_U_CET, r);
> + current->thread.cet.ibt_enabled = 0;
> +}
What's the locking for current->thread.cet?
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 705467839ce8..c609c9ce5691 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -413,7 +413,8 @@ __setup("nopku", setup_disable_pku);
>
> static __always_inline void setup_cet(struct cpuinfo_x86 *c)
> {
> - if (cpu_feature_enabled(X86_FEATURE_SHSTK))
> + if (cpu_feature_enabled(X86_FEATURE_SHSTK) ||
> + cpu_feature_enabled(X86_FEATURE_IBT))
> cr4_set_bits(X86_CR4_CET);
> }
>
> @@ -434,6 +435,23 @@ static __init int setup_disable_shstk(char *s)
> __setup("no_cet_shstk", setup_disable_shstk);
> #endif
>
> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> +static __init int setup_disable_ibt(char *s)
> +{
> + /* require an exact match without trailing characters */
> + if (strlen(s))
> + return 0;
> +
> + if (!boot_cpu_has(X86_FEATURE_IBT))
> + return 1;
> +
> + setup_clear_cpu_cap(X86_FEATURE_IBT);
> + pr_info("x86: 'no_cet_ibt' specified, disabling Branch Tracking\n");
> + return 1;
> +}
> +__setup("no_cet_ibt", setup_disable_ibt);
> +#endif
> /*
> * Some CPU features depend on higher CPUID levels, which may not always
> * be available due to CPUID level capping or broken virtualization
> diff --git a/arch/x86/kernel/elf.c b/arch/x86/kernel/elf.c
> index 233f6dad9c1f..42e08d3b573e 100644
> --- a/arch/x86/kernel/elf.c
> +++ b/arch/x86/kernel/elf.c
> @@ -15,6 +15,7 @@
> #include <linux/fs.h>
> #include <linux/uaccess.h>
> #include <linux/string.h>
> +#include <linux/compat.h>
>
> /*
> * The .note.gnu.property layout:
> @@ -222,7 +223,8 @@ int arch_setup_features(void *ehdr_p, void *phdr_p,
>
> struct elf64_hdr *ehdr64 = ehdr_p;
>
> - if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
> + if (!cpu_feature_enabled(X86_FEATURE_SHSTK) &&
> + !cpu_feature_enabled(X86_FEATURE_IBT))
> return 0;
>
> if (ehdr64->e_ident[EI_CLASS] == ELFCLASS64) {
> @@ -250,6 +252,9 @@ int arch_setup_features(void *ehdr_p, void *phdr_p,
> current->thread.cet.shstk_enabled = 0;
> current->thread.cet.shstk_base = 0;
> current->thread.cet.shstk_size = 0;
> + current->thread.cet.ibt_enabled = 0;
> + current->thread.cet.ibt_bitmap_addr = 0;
> + current->thread.cet.ibt_bitmap_size = 0;
> if (cpu_feature_enabled(X86_FEATURE_SHSTK)) {
> if (shstk) {
> err = cet_setup_shstk();
> @@ -257,6 +262,15 @@ int arch_setup_features(void *ehdr_p, void *phdr_p,
> goto out;
> }
> }
> +
> + if (cpu_feature_enabled(X86_FEATURE_IBT)) {
> + if (ibt) {
> + err = cet_setup_ibt();
> + if (err < 0)
> + goto out;
> + }
> + }
You introduced 'ibt' before it was used. Please wait to introduce it
until you actually use it to make it easier to review.
Also, what's wrong with:
if (cpu_feature_enabled(X86_FEATURE_IBT) && ibt) {
...
}
?
next prev parent reply other threads:[~2018-07-11 0:11 UTC|newest]
Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-10 22:26 [RFC PATCH v2 00/27] Control Flow Enforcement (CET) Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 01/27] x86/cpufeatures: Add CPUIDs for Control-flow Enforcement Technology (CET) Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 02/27] x86/fpu/xstate: Change some names to separate XSAVES system and user states Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 03/27] x86/fpu/xstate: Enable XSAVES system states Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 04/27] x86/fpu/xstate: Add XSAVES system states for shadow stack Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 05/27] Documentation/x86: Add CET description Yu-cheng Yu
2018-07-11 8:27 ` Pavel Machek
2018-07-11 15:25 ` Yu-cheng Yu
2018-07-11 9:57 ` Florian Weimer
2018-07-11 13:47 ` H.J. Lu
2018-07-11 14:53 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 06/27] x86/cet: Control protection exception handler Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 07/27] x86/cet/shstk: Add Kconfig option for user-mode shadow stack Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 08/27] mm: Introduce VM_SHSTK for shadow stack memory Yu-cheng Yu
2018-07-11 8:34 ` Peter Zijlstra
2018-07-11 16:15 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 09/27] x86/mm: Change _PAGE_DIRTY to _PAGE_DIRTY_HW Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 10/27] x86/mm: Introduce _PAGE_DIRTY_SW Yu-cheng Yu
2018-07-11 8:45 ` Peter Zijlstra
2018-07-11 9:21 ` Peter Zijlstra
2018-07-10 22:26 ` [RFC PATCH v2 11/27] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW Yu-cheng Yu
2018-07-10 22:44 ` Dave Hansen
2018-07-10 23:23 ` Nadav Amit
2018-07-10 23:52 ` Dave Hansen
2018-07-11 8:48 ` Peter Zijlstra
2018-07-10 22:26 ` [RFC PATCH v2 12/27] x86/mm: Shadow stack page fault error checking Yu-cheng Yu
2018-07-10 22:52 ` Dave Hansen
2018-07-11 17:28 ` Yu-cheng Yu
2018-07-10 23:24 ` Dave Hansen
2018-07-10 22:26 ` [RFC PATCH v2 13/27] mm: Handle shadow stack page fault Yu-cheng Yu
2018-07-10 23:06 ` Dave Hansen
2018-07-11 9:06 ` Peter Zijlstra
2018-08-14 21:28 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 14/27] mm: Handle THP/HugeTLB " Yu-cheng Yu
2018-07-10 23:08 ` Dave Hansen
2018-07-11 9:10 ` Peter Zijlstra
2018-07-11 16:11 ` Yu-cheng Yu
2018-07-20 14:20 ` Dave Hansen
2018-07-20 14:58 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 15/27] mm/mprotect: Prevent mprotect from changing shadow stack Yu-cheng Yu
2018-07-10 23:10 ` Dave Hansen
2018-07-11 9:12 ` Peter Zijlstra
2018-07-11 16:07 ` Yu-cheng Yu
2018-07-11 16:22 ` Dave Hansen
2018-07-10 22:26 ` [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for " Yu-cheng Yu
2018-07-10 23:37 ` Dave Hansen
2018-07-11 17:05 ` Yu-cheng Yu
2018-07-13 18:26 ` Dave Hansen
2018-07-17 23:03 ` Yu-cheng Yu
2018-07-17 23:11 ` Dave Hansen
2018-07-17 23:15 ` Dave Hansen
2018-07-18 20:14 ` Yu-cheng Yu
2018-07-18 21:45 ` Dave Hansen
2018-07-18 23:10 ` Yu-cheng Yu
2018-07-19 0:06 ` Dave Hansen
2018-07-19 17:06 ` Yu-cheng Yu
2018-07-19 19:31 ` Dave Hansen
2018-07-11 9:29 ` Peter Zijlstra
2018-07-17 23:00 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 17/27] x86/cet/shstk: User-mode shadow stack support Yu-cheng Yu
2018-07-10 23:40 ` Dave Hansen
2018-07-11 9:34 ` Peter Zijlstra
2018-07-11 15:45 ` Yu-cheng Yu
2018-07-11 9:36 ` Peter Zijlstra
2018-07-11 21:10 ` Jann Horn
2018-07-11 21:34 ` Andy Lutomirski
2018-07-11 21:51 ` Jann Horn
2018-07-11 22:21 ` Andy Lutomirski
2018-07-13 18:03 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction Yu-cheng Yu
2018-07-10 23:48 ` Dave Hansen
2018-07-12 22:59 ` Yu-cheng Yu
2018-07-12 23:49 ` Dave Hansen
2018-07-13 1:50 ` Dave Hansen
2018-07-13 2:21 ` Andy Lutomirski
2018-07-13 4:16 ` Dave Hansen
2018-07-13 4:18 ` Dave Hansen
2018-07-13 17:39 ` Yu-cheng Yu
2018-07-13 5:55 ` Andy Lutomirski
2018-07-11 9:44 ` Peter Zijlstra
2018-07-11 15:06 ` Yu-cheng Yu
2018-07-11 15:30 ` Peter Zijlstra
2018-07-11 9:45 ` Peter Zijlstra
2018-07-11 14:58 ` Yu-cheng Yu
2018-07-11 15:27 ` Peter Zijlstra
2018-07-11 15:41 ` Yu-cheng Yu
2018-07-13 12:12 ` Dave Hansen
2018-07-13 17:37 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 19/27] x86/cet/shstk: Signal handling for shadow stack Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 20/27] x86/cet/shstk: ELF header parsing of CET Yu-cheng Yu
2018-07-11 11:12 ` Florian Weimer
2018-07-11 19:37 ` Jann Horn
2018-07-11 20:53 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 21/27] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 22/27] x86/cet/ibt: User-mode indirect branch tracking support Yu-cheng Yu
2018-07-11 0:11 ` Dave Hansen [this message]
2018-07-11 22:10 ` Yu-cheng Yu
2018-07-11 22:40 ` Dave Hansen
2018-07-11 23:00 ` Yu-cheng Yu
2018-07-11 23:16 ` Dave Hansen
2018-07-13 17:56 ` Yu-cheng Yu
2018-07-13 18:05 ` Dave Hansen
2018-07-11 21:07 ` Jann Horn
2018-07-10 22:26 ` [RFC PATCH v2 23/27] mm/mmap: Add IBT bitmap size to address space limit check Yu-cheng Yu
2018-07-10 23:57 ` Dave Hansen
2018-07-11 16:56 ` Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 24/27] x86: Insert endbr32/endbr64 to vDSO Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 25/27] x86/cet: Add PTRACE interface for CET Yu-cheng Yu
2018-07-11 10:20 ` Ingo Molnar
2018-07-11 15:40 ` Yu-cheng Yu
2018-07-12 14:03 ` Ingo Molnar
2018-07-12 22:37 ` Yu-cheng Yu
2018-07-12 23:08 ` Thomas Gleixner
2018-07-13 16:07 ` Yu-cheng Yu
2018-07-13 6:28 ` Pavel Machek
2018-07-13 13:33 ` Ingo Molnar
2018-07-14 6:27 ` Pavel Machek
2018-07-10 22:26 ` [RFC PATCH v2 26/27] x86/cet/shstk: Handle thread shadow stack Yu-cheng Yu
2018-07-10 22:26 ` [RFC PATCH v2 27/27] x86/cet: Add arch_prctl functions for CET Yu-cheng Yu
2018-07-11 12:19 ` Florian Weimer
2018-07-11 21:02 ` Yu-cheng Yu
2018-07-11 19:45 ` Jann Horn
2018-07-11 20:55 ` Yu-cheng Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3a7e9ce4-03c6-cc28-017b-d00108459e94@linux.intel.com \
--to=dave.hansen@linux.intel.com \
--cc=arnd@arndb.de \
--cc=bsingharora@gmail.com \
--cc=corbet@lwn.net \
--cc=fweimer@redhat.com \
--cc=gorcunov@gmail.com \
--cc=hjl.tools@gmail.com \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=keescook@chromiun.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@amacapital.net \
--cc=mike.kravetz@oracle.com \
--cc=mingo@redhat.com \
--cc=nadav.amit@gmail.com \
--cc=oleg@redhat.com \
--cc=pavel@ucw.cz \
--cc=peterz@infradead.org \
--cc=ravi.v.shankar@intel.com \
--cc=tglx@linutronix.de \
--cc=vedvyas.shanbhogue@intel.com \
--cc=x86@kernel.org \
--cc=yu-cheng.yu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).