linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yu-cheng Yu <yu-cheng.yu@intel.com>
To: Dave Hansen <dave.hansen@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@kernel.org>,
	Balbir Singh <bsingharora@gmail.com>,
	Borislav Petkov <bp@alien8.de>,
	Cyrill Gorcunov <gorcunov@gmail.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Eugene Syromiatnikov <esyr@redhat.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@chromium.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>,
	Randy Dunlap <rdunlap@infradead.org>,
	"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
	Vedvyas Shanbhogue <vedvyas.shanbhogue@intel.com>,
	Dave Martin <Dave.Martin@arm.com>,
	Weijiang Yang <weijiang.yang@intel.com>
Subject: Re: [PATCH v10 01/26] Documentation/x86: Add CET description
Date: Fri, 15 May 2020 16:29:25 -0700	[thread overview]
Message-ID: <b09658f92eb66c1d1be509813939b9ed827f9cf0.camel@intel.com> (raw)
In-Reply-To: <32235ffc-6e6c-fb3d-80c4-a0478e2d0e0f@intel.com>

On Fri, 2020-05-15 at 15:43 -0700, Dave Hansen wrote:
> On 5/15/20 2:33 PM, Yu-cheng Yu wrote:
> > On Fri, 2020-05-15 at 11:39 -0700, Dave Hansen wrote:
> > > On 5/12/20 4:20 PM, Yu-cheng Yu wrote:
> > > Can a binary compiled with CET run without CET?
> > 
> > Yes, but a few details:
> > 
> > - The shadow stack is transparent to the application.  A CET application does
> > not have anything different from a non-CET application.  However, if a CET
> > application uses any CET instructions (e.g. INCSSP), it must first check if CET
> > is turned on.
> > - If an application is compiled for IBT, the compiler inserts ENDBRs at branch
> > targets.  These are nops if IBT is not on.
> 
> I appreciate the detailed response, but it wasn't quite what I was
> asking.  Let's ignore IBT for now and just talk about shadow stacks.
> 
> An app compiled with the new ELF flags and running on a CET-enabled
> kernel and CPU will start off with shadow stacks allocated and enabled,
> right?  It can turn its shadow stack off per-thread with the new prctl.
>  But, otherwise, it's stuck, the only way to turn shadow stacks off at
> startup would be editing the binary.
> 
> Basically, if there ends up being a bug in an app that violates the
> shadow stack rules, the app is broken, period.  The only recourse is to
> have the kernel disable CET and reboot.
> 
> Is that right?

You must be talking about init or any of the system daemons, right?
Assuming we let the app itself start CET with an arch_prctl(), why would that be
different from the current approach?

> > > Can a binary compiled without CET run CET-enabled code?
> > 
> > Partially yes, but in reality somewhat difficult.
> ...
> > - If a not-CET application does fork(), and the child wants to turn on CET, it
> > would be difficult to manage the stack frames, unless the child knows what is is
> > doing.  
> 
> It might be hard to do, but it is possible with the patches you posted?

It is possible to add an arch_prctl() to turn on CET.  That is simple from the
kernel's perspective, but difficult for the application.  Once the app enables
shadow stack, it has to take care not to return beyond the function call layers
before that point.  It can no longer do longjmp or ucontext swaps to anything
before that point.  It will also be complicated if the app enables shadow stack
in a signal handler.

>  I think you're saying that the CET-enabled binary would do
> arch_setup_elf_property() when it was first exec()'d.  Later, it could
> use the new prctl(ARCH_X86_CET_DISABLE) to disable its shadow stack,
> then fork() and the child would not be using CET.  Right?
> 
> What is ARCH_X86_CET_DISABLE used for, anyway?

Both the parent and the child can do ARCH_X86_CET_DISABLE, if CET is not locked.

> > The JIT examples I mentioned previously run with CET enabled from the
> > beginning.  Do you have a reason to do this?  In other words, if the JIT code
> > needs CET, the app could have started with CET in the first place.
> 
> Let's say I have a JIT'd sandbox.  I want the sandbox to be
> CET-protected, but the JIT engine itself not to be.

I do not have any objections to this use case, but it needs some cautions as
stated above.  It will be much easier and cleaner if the sandbox is in a
separate exec'ed task with CET on.

> > - If you are asking about dlopen(), the library will have the same setting as
> > the main application.  Do you have any reason to have a library running with
> > CET, but the application does not have CET?
> 
> Sure, using old binaries.  That's why IBT has a legacy bitmap and things
> like MPX had ways of jumping into old non-enabled binaries.

If the app has CET, but libs do not, then bitmap can help.
If the app does not have CET, we don't run the libs with CET, right?  This is
the case right now.

> > > Can different threads in a process have different CET enabling state?
> > 
> > Yes, if the parent starts with CET, children can turn it off.
> 
> How would that work, though?  clone() by default will copy the parent
> xsave state, which means it will be CET-enabled, which means it needs a
> shadow stack.  So, if I want a CET-free child thread, I need to clone(),
> then turn CET off, then free the shadow stack?

Yes, the child itself turns off CET.

> > > Does this *code* work?  Could you please indicate which JITs have been
> > > enabled to use the code in this series?  How much of the new ABI is in use?
> > 
> > JIT does not necessarily use all of the ABI.  The JIT changes mainly fix stack
> > frames and insert ENDBRs.  I do not work on JIT.  What I found is LLVM JIT fixes
> > are tested and in the master branch.  Sljit fixes are in the release.
> 
> Huh, so who is using the new prctl() ABIs?

Any code can use the ABI, but JIT code CET-enabling part mostly do not use these
new prctl()'s, except, probably to get CET status.

> > > Where are the selftests/ for this new ABI?  Were you planning on
> > > submitting any with this series?
> > 
> > The ABI is more related to the application side, and therefore most suitable for
> > GLIBC unit tests.
> 
> I was mostly concerned with the kernel selftests.  The things in
> tools/testing/selftests/x86 in the kernel tree.

I have run them with CET enabled.  All of them pass, except for the following:
Sigreturn from 64-bit to 32-bit fails, because shadow stack is at a 64-bit
address.  This is understandable.

> > The more complicated areas such as pthreads, signals, ucontext,
> > fork() are all included there.  I have been constantly running these 
> > tests without any problems.  I can provide more details if testing is
> > the concern.
> 
> For something this complicated, with new kernel ABIs, we need an
> in-kernel sefltest.
> 
> MPX was not that much different from this feature.  It required a
> boatload of compiler and linker changes to function.  Yet, there was a
> simple in-kernel test for it that didn't require *any* of that big pile
> of toolchain bits.
> 
> Is there a reason we don't have one of those for CET?

I have a quick test that checks shadow stack and ibt in both main program and in
signals.  Currently it is public on Github.  If that is desired, I can submit it
to the mailing list.

Yu-cheng


  reply	other threads:[~2020-05-15 23:29 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29 22:07 [PATCH v10 00/26] Control-flow Enforcement: Shadow Stack Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 01/26] Documentation/x86: Add CET description Yu-cheng Yu
2020-04-29 22:53   ` Dave Hansen
2020-04-29 23:02     ` Yu-cheng Yu
2020-05-12 23:20       ` Yu-cheng Yu
2020-05-15 18:39         ` Dave Hansen
2020-05-15 21:33           ` Yu-cheng Yu
2020-05-15 22:43             ` Dave Hansen
2020-05-15 23:29               ` Yu-cheng Yu [this message]
2020-05-15 23:56                 ` Dave Hansen
2020-05-16  2:51                   ` H.J. Lu
2020-05-17 23:09                     ` Dave Hansen
2020-05-16  2:53                   ` Yu-cheng Yu
2020-05-18 13:41                     ` Dave Hansen
2020-05-18 14:01                       ` H.J. Lu
2020-05-18 14:26                         ` Dave Hansen
2020-05-18 14:21                       ` Yu-cheng Yu
2020-05-18 23:47                     ` Yu-cheng Yu
2020-05-19  0:38                       ` Dave Hansen
2020-05-19  1:35                         ` Andy Lutomirski
2020-05-20  1:04                           ` Andy Lutomirski
2020-05-29  2:08                             ` Yu-cheng Yu
2020-05-16  0:13               ` Andrew Cooper
2020-05-16  2:37                 ` H.J. Lu
2020-05-16 14:09                   ` Andrew Cooper
2020-05-22 16:49                     ` Peter Zijlstra
2020-05-22 17:48                       ` Andrew Cooper
2020-04-29 22:07 ` [PATCH v10 02/26] x86/cpufeatures: Add CET CPU feature flags for Control-flow Enforcement Technology (CET) Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 03/26] x86/fpu/xstate: Introduce CET MSR XSAVES supervisor states Yu-cheng Yu
2020-07-23 16:10   ` Sean Christopherson
2020-07-23 16:21     ` Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 04/26] x86/cet: Add control-protection fault handler Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 05/26] x86/cet/shstk: Add Kconfig option for user-mode Shadow Stack Yu-cheng Yu
2020-05-07 15:55   ` Dave Hansen
2020-05-07 16:59     ` Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 06/26] x86/mm: Change _PAGE_DIRTY to _PAGE_DIRTY_HW Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 07/26] x86/mm: Remove _PAGE_DIRTY_HW from kernel RO pages Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 08/26] x86/mm: Introduce _PAGE_COW Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 09/26] drm/i915/gvt: Change _PAGE_DIRTY to _PAGE_DIRTY_BITS Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 10/26] x86/mm: Update pte_modify for _PAGE_COW Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 11/26] x86/mm: Update ptep_set_wrprotect() and pmdp_set_wrprotect() for transition from _PAGE_DIRTY_HW to _PAGE_COW Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 12/26] mm: Introduce VM_SHSTK for shadow stack memory Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 13/26] x86/mm: Shadow Stack page fault error checking Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 14/26] x86/mm: Update maybe_mkwrite() for shadow stack Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 15/26] mm: Fixup places that call pte_mkwrite() directly Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 16/26] mm: Add guard pages around a shadow stack Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 17/26] mm/mmap: Add shadow stack pages to memory accounting Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 18/26] mm: Update can_follow_write_pte() for shadow stack Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 19/26] x86/cet/shstk: User-mode shadow stack support Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 20/26] x86/cet/shstk: Handle signals for shadow stack Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 21/26] ELF: UAPI and Kconfig additions for ELF program properties Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 22/26] ELF: Add ELF program property parsing support Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 23/26] ELF: Introduce arch_setup_elf_property() Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 24/26] x86/cet/shstk: ELF header parsing for shadow stack Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 25/26] x86/cet/shstk: Handle thread " Yu-cheng Yu
2020-04-29 22:07 ` [PATCH v10 26/26] x86/cet/shstk: Add arch_prctl functions for " Yu-cheng Yu
2020-05-21 22:42   ` Kees Cook
2020-05-22 17:17     ` Yu-cheng Yu
2020-05-22 17:29       ` Eugene Syromiatnikov
2020-05-22 18:13         ` Yu-cheng Yu
2020-05-21 15:15 ` [PATCH v10 00/26] Control-flow Enforcement: Shadow Stack Josh Poimboeuf
2020-05-21 15:57   ` Yu-cheng Yu
2020-05-21 18:50     ` Josh Poimboeuf
2020-05-21 19:08       ` Yu-cheng Yu
2020-07-23 16:25 ` Sean Christopherson
2020-07-23 16:41   ` Dave Hansen
2020-07-23 16:56     ` Sean Christopherson
2020-07-23 18:41       ` Dave Hansen
2020-07-24  3:40         ` Yu-cheng Yu
2020-07-24  4:50           ` Sean Christopherson
2020-07-24  4:59         ` Sean Christopherson

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=b09658f92eb66c1d1be509813939b9ed827f9cf0.camel@intel.com \
    --to=yu-cheng.yu@intel.com \
    --cc=Dave.Martin@arm.com \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=bsingharora@gmail.com \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=esyr@redhat.com \
    --cc=fweimer@redhat.com \
    --cc=gorcunov@gmail.com \
    --cc=hjl.tools@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.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@kernel.org \
    --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=rdunlap@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vedvyas.shanbhogue@intel.com \
    --cc=weijiang.yang@intel.com \
    --cc=x86@kernel.org \
    /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).