linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Andy Lutomirski" <luto@kernel.org>
To: "Rick P Edgecombe" <rick.p.edgecombe@intel.com>,
	"Dave Hansen" <dave.hansen@intel.com>
Cc: "Balbir Singh" <bsingharora@gmail.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Eugene Syromiatnikov" <esyr@redhat.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Yu-cheng Yu" <yu-cheng.yu@intel.com>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"Florian Weimer" <fweimer@redhat.com>,
	"Nadav Amit" <nadav.amit@gmail.com>,
	"Jann Horn" <jannh@google.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"kcc@google.com" <kcc@google.com>,
	"Borislav Petkov" <bp@alien8.de>,
	"Oleg Nesterov" <oleg@redhat.com>,
	"H.J. Lu" <hjl.tools@gmail.com>, "Pavel Machek" <pavel@ucw.cz>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"Weijiang Yang" <weijiang.yang@intel.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Moreira, Joao" <joao.moreira@intel.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Mike Kravetz" <mike.kravetz@oracle.com>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	"tarasmadan@google.com" <tarasmadan@google.com>,
	"Dave Martin" <Dave.Martin@arm.com>,
	"vedvyas.shanbhogue@intel.com" <vedvyas.shanbhogue@intel.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Linux API" <linux-api@vger.kernel.org>,
	"Cyrill Gorcunov" <gorcunov@gmail.com>
Subject: Re: [NEEDS-REVIEW] Re: [PATCH v11 25/25] x86/cet/shstk: Add arch_prctl functions for shadow stack
Date: Mon, 20 Sep 2021 09:48:10 -0700	[thread overview]
Message-ID: <b5b5787b-17ce-4e66-8bc6-ab42ae3e398d@www.fastmail.com> (raw)
In-Reply-To: <45c62101c065ed7e728fadac7207866bf8c36ec4.camel@intel.com>



On Mon, Sep 13, 2021, at 6:33 PM, Edgecombe, Rick P wrote:
> On Mon, 2020-09-14 at 11:31 -0700, Andy Lutomirski wrote:
> > > On Sep 14, 2020, at 7:50 AM, Dave Hansen <dave.hansen@intel.com>
> > > wrote:
> > > 
> > > On 9/11/20 3:59 PM, Yu-cheng Yu wrote:
> > > ...
> > > > Here are the changes if we take the mprotect(PROT_SHSTK)
> > > > approach.
> > > > Any comments/suggestions?
> > > 
> > > I still don't like it. :)
> > > 
> > > I'll also be much happier when there's a proper changelog to
> > > accompany
> > > this which also spells out the alternatives any why they suck so
> > > much.
> > > 
> > 
> > Let’s take a step back here. Ignoring the precise API, what exactly
> > is
> > a shadow stack from the perspective of a Linux user program?
> > 
> > The simplest answer is that it’s just memory that happens to have
> > certain protections.  This enables all kinds of shenanigans.  A
> > program could map a memfd twice, once as shadow stack and once as
> > non-shadow-stack, and change its control flow.  Similarly, a program
> > could mprotect its shadow stack, modify it, and mprotect it back.  In
> > some threat models, though could be seen as a WRSS bypass.  (Although
> > if an attacker can coerce a process to call mprotect(), the game is
> > likely mostly over anyway.)
> > 
> > But we could be more restrictive, or perhaps we could allow user code
> > to opt into more restrictions.  For example, we could have shadow
> > stacks be special memory that cannot be written from usermode by any
> > means other than ptrace() and friends, WRSS, and actual shadow stack
> > usage.
> > 
> > What is the goal?
> > 
> > No matter what we do, the effects of calling vfork() are going to be
> > a
> > bit odd with SHSTK enabled.  I suppose we could disallow this, but
> > that seems likely to cause its own issues.
> 
> Hi,
> 
> Resurrecting this old thread to highlight a consequence of the design
> change that came out of it. I am going to be taking over this series
> from Yu-cheng, and wanted to check if people would be interested in re-
> visiting this interface.
> 
> The consequence I wanted to highlight, is that making userspace be
> responsible for mapping memory as shadow stack, also requires moving
> the writing of the restore token to userspace for glibc ucontext
> operations. Since these operations involve creating/pivoting to new
> stacks in userspace, ucontext cet support involves also creating a new
> shadow stack. For normal thread stacks, the kernel has always done the
> shadow stack allocation and so it is never writable (in the normal
> sense) from userspace. But after this change makecontext() now first
> has to mmap() writable memory, then write the restore token, then
> mprotect() it as shadow stack. See the glibc changes to support
> PROT_SHADOW_STACK here[0].
> 
> The writable window leaves an opening for an attacker to create an
> arbitrary shadow stack that could be pivoted to later by tweaking the
> ucontext_t structure. To try to see how much this matters, we have done
> a small test that uses this window to ROP from writes in another
> thread during the makecontext()/setcontext() window. (offensive work
> credit to Joao on CC). This would require a real app to already to be
> using ucontext in the course of normal runtime.

My general opinion here (take this with a grain of salt -- I haven't paged back in every single detail) is that the kernel should make it straightforward for a libc to do the right thing without nasty races, cross-thread coordination, or unnecessary permission to write to the stack.  I *also* think that it should be possible for userspace to manage its own shadow stack allocation if it wants to, since I'm sure there will be JIT or green thread or other use cases that want to do crazy things that we fail to anticipate with in-kernel magic.

So perhaps we should keep the explicit allocation and free operations, have a way to opt-in to WRSS being flipped on, but also do our best to have API that handle the known cases well.

Does that make sense?  Can we have both approaches work in the same kernel?

  parent reply	other threads:[~2021-09-20 16:49 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-25  0:25 [PATCH v11 00/25] Control-flow Enforcement: Shadow Stack Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 01/25] Documentation/x86: Add CET description Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 02/25] x86/cpufeatures: Add CET CPU feature flags for Control-flow Enforcement Technology (CET) Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 03/25] x86/fpu/xstate: Introduce CET MSR XSAVES supervisor states Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 04/25] x86/cet: Add control-protection fault handler Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 05/25] x86/cet/shstk: Add Kconfig option for user-mode Shadow Stack Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 06/25] x86/mm: Change _PAGE_DIRTY to _PAGE_DIRTY_HW Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 07/25] x86/mm: Remove _PAGE_DIRTY_HW from kernel RO pages Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 08/25] x86/mm: Introduce _PAGE_COW Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 09/25] drm/i915/gvt: Change _PAGE_DIRTY to _PAGE_DIRTY_BITS Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 10/25] x86/mm: Update pte_modify for _PAGE_COW Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 11/25] x86/mm: Update ptep_set_wrprotect() and pmdp_set_wrprotect() for transition from _PAGE_DIRTY_HW to _PAGE_COW Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 12/25] mm: Introduce VM_SHSTK for shadow stack memory Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 13/25] x86/mm: Shadow Stack page fault error checking Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 14/25] x86/mm: Update maybe_mkwrite() for shadow stack Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 15/25] mm: Fixup places that call pte_mkwrite() directly Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 16/25] mm: Add guard pages around a shadow stack Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 17/25] mm/mmap: Add shadow stack pages to memory accounting Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 18/25] mm: Update can_follow_write_pte() for shadow stack Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 19/25] mm: Re-introduce do_mmap_pgoff() Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 20/25] x86/cet/shstk: User-mode shadow stack support Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 21/25] x86/cet/shstk: Handle signals for shadow stack Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 22/25] binfmt_elf: Define GNU_PROPERTY_X86_FEATURE_1_AND properties Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 23/25] ELF: Introduce arch_setup_elf_property() Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 24/25] x86/cet/shstk: Handle thread shadow stack Yu-cheng Yu
2020-08-25  0:25 ` [PATCH v11 25/25] x86/cet/shstk: Add arch_prctl functions for " Yu-cheng Yu
2020-08-25  0:36   ` Andy Lutomirski
2020-08-25 18:43     ` Yu, Yu-cheng
2020-08-25 19:19       ` Dave Hansen
2020-08-25 21:04         ` Yu, Yu-cheng
2020-08-25 23:20           ` Dave Hansen
2020-08-25 23:34             ` Yu, Yu-cheng
2020-08-26 16:46               ` Dave Martin
2020-08-26 16:51                 ` Florian Weimer
2020-08-26 17:04                   ` Andy Lutomirski
2020-08-26 18:49                     ` Yu, Yu-cheng
2020-08-26 19:43                       ` H.J. Lu
2020-08-26 19:57                       ` Dave Hansen
2020-08-27 13:26                         ` H.J. Lu
2020-09-01 10:28                           ` Dave Martin
2020-09-01 17:23                             ` Yu, Yu-cheng
2020-09-01 17:45                               ` Andy Lutomirski
2020-09-01 18:11                                 ` Dave Hansen
2020-09-02 13:58                                   ` Dave Martin
     [not found]                                   ` <46dffdfd-92f8-0f05-6164-945f217b0958@intel.com>
2020-09-08 17:57                                     ` Dave Hansen
2020-09-08 18:25                                       ` Yu, Yu-cheng
2020-09-09 22:08                                         ` Yu, Yu-cheng
2020-09-09 22:59                                           ` Dave Hansen
2020-09-09 23:07                                             ` Yu, Yu-cheng
2020-09-09 23:11                                               ` Dave Hansen
2020-09-09 23:25                                                 ` Yu, Yu-cheng
2020-09-09 23:29                                                   ` Dave Hansen
2020-09-09 23:45                                                     ` Yu, Yu-cheng
2020-09-11 22:59                                                     ` Yu-cheng Yu
2020-09-14 14:50                                                       ` [NEEDS-REVIEW] " Dave Hansen
2020-09-14 18:31                                                         ` Andy Lutomirski
2020-09-14 20:44                                                           ` Yu, Yu-cheng
2020-09-14 21:14                                                           ` Dave Hansen
2020-09-16 13:52                                                             ` Andy Lutomirski
2020-09-16 19:25                                                               ` Yu, Yu-cheng
2021-09-14  1:33                                                           ` [NEEDS-REVIEW] " Edgecombe, Rick P
2021-09-14  9:53                                                             ` Borislav Petkov
2021-09-20 16:48                                                             ` Andy Lutomirski [this message]
2021-09-23 23:32                                                               ` Edgecombe, Rick P
     [not found]                                                         ` <bf2ab309-f8c4-83da-1c0a-5684e5bc5c82@intel.com>
2020-09-15 19:08                                                           ` Yu-cheng Yu
2020-09-15 19:24                                                             ` Dave Hansen
2020-09-15 20:16                                                               ` Yu, Yu-cheng
2020-08-26 17:08                   ` Dave Martin
2020-08-27 13:18                     ` Florian Weimer
2020-08-27 13:28                       ` H.J. Lu
2020-08-27 13:36                         ` Florian Weimer
2020-08-27 14:07                           ` H.J. Lu
2020-08-27 14:08                             ` H.J. Lu
2020-09-01 17:49                               ` Yu, Yu-cheng
2020-09-01 17:50                                 ` Florian Weimer
2020-09-01 17:58                                   ` Yu, Yu-cheng
2020-09-01 18:17                                     ` Florian Weimer
2020-09-01 18:19                                       ` H.J. Lu
2020-09-01 18:24                                       ` Yu, Yu-cheng
2020-08-27 18:13                           ` Yu, Yu-cheng
2020-08-27 18:56                             ` Andy Lutomirski
2020-08-27 19:33                               ` Yu, Yu-cheng
2020-08-27 19:37                               ` H.J. Lu
2020-08-28  1:35                                 ` Andy Lutomirski
2020-08-28  1:44                                   ` H.J. Lu
2020-08-28  6:23                                     ` Florian Weimer
2020-08-28 11:37                                       ` H.J. Lu
2020-08-28 17:39                                         ` Andy Lutomirski
2020-08-28 17:45                                           ` H.J. Lu

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=b5b5787b-17ce-4e66-8bc6-ab42ae3e398d@www.fastmail.com \
    --to=luto@kernel.org \
    --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=joao.moreira@intel.com \
    --cc=kcc@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=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=rick.p.edgecombe@intel.com \
    --cc=tarasmadan@google.com \
    --cc=tglx@linutronix.de \
    --cc=vedvyas.shanbhogue@intel.com \
    --cc=weijiang.yang@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).