linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
To: "bsingharora@gmail.com" <bsingharora@gmail.com>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"Syromiatnikov, Eugene" <esyr@redhat.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"Eranian, Stephane" <eranian@google.com>,
	"kirill.shutemov@linux.intel.com"
	<kirill.shutemov@linux.intel.com>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"fweimer@redhat.com" <fweimer@redhat.com>,
	"nadav.amit@gmail.com" <nadav.amit@gmail.com>,
	"jannh@google.com" <jannh@google.com>,
	"kcc@google.com" <kcc@google.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"bp@alien8.de" <bp@alien8.de>,
	"oleg@redhat.com" <oleg@redhat.com>,
	"hjl.tools@gmail.com" <hjl.tools@gmail.com>,
	"Yang, Weijiang" <weijiang.yang@intel.com>,
	"Lutomirski, Andy" <luto@kernel.org>,
	"pavel@ucw.cz" <pavel@ucw.cz>, "arnd@arndb.de" <arnd@arndb.de>,
	"Moreira, Joao" <joao.moreira@intel.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"mike.kravetz@oracle.com" <mike.kravetz@oracle.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"Dave.Martin@arm.com" <Dave.Martin@arm.com>,
	"john.allen@amd.com" <john.allen@amd.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"gorcunov@gmail.com" <gorcunov@gmail.com>
Subject: Re: [PATCH 00/35] Shadow stacks for userspace
Date: Fri, 4 Feb 2022 20:23:38 +0000	[thread overview]
Message-ID: <e83cf0ff1834bcfc7436d9370ae2807e5a026f32.camel@intel.com> (raw)
In-Reply-To: <6b04d8cb-844e-42a5-9ea2-db0e8eafaa19@www.fastmail.com>

On Thu, 2022-02-03 at 21:20 -0800, Andy Lutomirski wrote:
> On Thu, Feb 3, 2022, at 5:08 PM, Edgecombe, Rick P wrote:
> > Hi Thomas,
> > > >         Signals
> > > >         -------
> > > >         Originally signals placed the location of the shadow
> > > > stack
> > > > restore 
> > > >         token inside the saved state on the stack. This was
> > > > problematic from a 
> > > >         past ABI promises perspective.
> 
> What was the actual problem?

The meat of the discussion that I saw was this thread:

https://lore.kernel.org/lkml/CALCETrVTeYfzO-XWh+VwTuKCyPyp-oOMGH=QR_msG9tPQ4xPmA@mail.gmail.com/

The problem was that the saved shadow stack pointer was placed in a
location unexpected by userspace (at the end of the floating point
state), which apparently can be relocated by userspace that would not
be aware of the shadow stack state at the end. I think an earlier
version was not extendable either.

It does not seem to be a fully dead end, but I have to admit I didn’t
fully internalize the limits imposed by the userspace expectations of
the sigframe because the current solution seemed good. I’ll have to dig
into it a little more because alt stack, CRIU and these expectations
all seem intertwined.

Here is a question for you guys though – is a general ucontext
extension solution a nice-to-have on its own?

I was thinking that it would be better to keep CET stuff out of the
sigframe related structures if it can be avoided. One reason is that it
keeps security related register values out of writable locations. I’m
not thinking of any specific problem, but just a general idea of not
opening that stuff up if it’s not needed. An example is in the IBT
series, the wait-for-endbranch state was saved in a ucontext flag. Some
usages may want to keep it from being cleared in a signal and the
endbranch check skipped. So for shadow stack, just the general notion
that this is not ideal state to open up.

On where to keep the wait-for-endbranch state, PeterZ had suggested the
possibility of having a per-mm hashmap of (userspace stack addresses)-> 
(kernel side saved state), limited to some sensible limit of items.
This could be extendable to other state besides CET stuff. I was
thinking to look in that direction if it’s needed for the alt shadow
stack.

But, is it swimming against the place where saved state is "supposed"
to be? There is some optimum of compatibility (more apps able to opt-
in) and security. I guess it's probably not good to have the kernel
bend over backwards trying to get both.

> > > > So the restore location was
> > > > instead just 
> > > >         assumed from the shadow stack pointer. This works
> > > > because in
> > > > normal 
> > > >         allowed cases of calling sigreturn, the shadow stack
> > > > pointer
> > > > should be 
> > > >         right at the restore token at that time. There is no
> > > > alternate shadow 
> > > >         stack support. If an alt shadow stack is added later we
> > > > would
> > > >         need to
> > > 
> > > So how is that going to work? altstack is not an esoteric corner
> > > case.
> > 
> > My understanding is that the main usages for the signal stack were
> > handling stack overflows and corruption. Since the shadow stack
> > only
> > contains return addresses rather than large stack allocations, and
> > is
> > not generally writable or pivotable, I thought there was a good
> > possibility an alt shadow stack would not end up being especially
> > useful. Does it seem like reasonable guesswork?
> 
> It's also used for things like DOSEMU that execute in a weird context
> and then trap back out to the outer program using a signal handler
> and an altstack.  Also, imagine someone writing a SIGSEGV handler
> specifically intended to handle shadow stack overflow.

Interesting, thanks. I had been thinking that an alt shadow stack would
require a new interface that would mostly just sit dormant taking up
space. But probably an (obvious) better way would be to just have the
signalstack() syscall automatically create a new shadow stack, like the
rest of the series does automatically for new threads. I’ll think I’ll
see how that would look.

> 
> The shadow stack can be pivoted using RSTORSSP.

Yes, I just meant that the ability to pivot or modify is restricted (in
RSTORSSP's case by restore token checks) and so with less ability to
interact with it, it could be less likely for there to be corruptions.
This if of course just speculation.

  reply	other threads:[~2022-02-04 20:23 UTC|newest]

Thread overview: 154+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-30 21:18 [PATCH 00/35] Shadow stacks for userspace Rick Edgecombe
2022-01-30 21:18 ` [PATCH 01/35] Documentation/x86: Add CET description Rick Edgecombe
2022-01-30 21:18 ` [PATCH 02/35] x86/cet/shstk: Add Kconfig option for Shadow Stack Rick Edgecombe
2022-02-07 22:39   ` Dave Hansen
2022-02-08  8:41     ` Thomas Gleixner
2022-02-08 20:20       ` Edgecombe, Rick P
2022-02-08  8:39   ` Thomas Gleixner
2022-01-30 21:18 ` [PATCH 03/35] x86/cpufeatures: Add CET CPU feature flags for Control-flow Enforcement Technology (CET) Rick Edgecombe
2022-02-07 22:45   ` Dave Hansen
2022-02-08 20:23     ` Edgecombe, Rick P
2022-02-09  1:10   ` Kees Cook
2022-01-30 21:18 ` [PATCH 04/35] x86/cpufeatures: Introduce CPU setup and option parsing for CET Rick Edgecombe
2022-02-07 22:49   ` Dave Hansen
2022-02-08 20:29     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 05/35] x86/fpu/xstate: Introduce CET MSR and XSAVES supervisor states Rick Edgecombe
2022-02-07 23:28   ` Dave Hansen
2022-02-08 21:36     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 06/35] x86/cet: Add control-protection fault handler Rick Edgecombe
2022-02-07 23:56   ` Dave Hansen
2022-02-08 22:23     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 07/35] x86/mm: Remove _PAGE_DIRTY from kernel RO pages Rick Edgecombe
2022-02-08  0:13   ` Dave Hansen
2022-02-08 22:52     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 08/35] x86/mm: Move pmd_write(), pud_write() up in the file Rick Edgecombe
2022-01-30 21:18 ` [PATCH 09/35] x86/mm: Introduce _PAGE_COW Rick Edgecombe
2022-02-08  1:05   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 10/35] drm/i915/gvt: Change _PAGE_DIRTY to _PAGE_DIRTY_BITS Rick Edgecombe
2022-02-09 16:58   ` Dave Hansen
2022-02-11  1:39     ` Edgecombe, Rick P
2022-02-11  7:13       ` Wang, Zhi A
2022-02-12  1:45         ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 11/35] x86/mm: Update pte_modify for _PAGE_COW Rick Edgecombe
2022-02-09 18:00   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 12/35] x86/mm: Update ptep_set_wrprotect() and pmdp_set_wrprotect() for transition from _PAGE_DIRTY to _PAGE_COW Rick Edgecombe
2022-02-09 18:30   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 13/35] mm: Move VM_UFFD_MINOR_BIT from 37 to 38 Rick Edgecombe
2022-01-30 21:18 ` [PATCH 14/35] mm: Introduce VM_SHADOW_STACK for shadow stack memory Rick Edgecombe
2022-02-09 21:55   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 15/35] x86/mm: Check Shadow Stack page fault errors Rick Edgecombe
2022-02-09 19:06   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 16/35] x86/mm: Update maybe_mkwrite() for shadow stack Rick Edgecombe
2022-02-09 21:16   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 17/35] mm: Fixup places that call pte_mkwrite() directly Rick Edgecombe
2022-02-09 21:51   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 18/35] mm: Add guard pages around a shadow stack Rick Edgecombe
2022-02-09 22:23   ` Dave Hansen
2022-02-10 22:38     ` David Laight
2022-02-10 23:42       ` Edgecombe, Rick P
2022-02-11  9:08         ` David Laight
2022-02-10 22:43   ` Dave Hansen
2022-02-10 23:07     ` Andy Lutomirski
2022-02-10 23:40       ` Edgecombe, Rick P
2022-02-11 17:54         ` Andy Lutomirski
2022-02-12  0:10           ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 19/35] mm/mmap: Add shadow stack pages to memory accounting Rick Edgecombe
2022-02-09 22:27   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 20/35] mm: Update can_follow_write_pte() for shadow stack Rick Edgecombe
2022-02-09 22:50   ` Dave Hansen
2022-02-09 22:52   ` Dave Hansen
2022-02-10 22:45     ` David Laight
2022-01-30 21:18 ` [PATCH 21/35] mm/mprotect: Exclude shadow stack from preserve_write Rick Edgecombe
2022-02-10 19:27   ` Dave Hansen
2022-01-30 21:18 ` [PATCH 22/35] x86/mm: Prevent VM_WRITE shadow stacks Rick Edgecombe
2022-02-11 22:19   ` Dave Hansen
2022-02-12  1:44     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 23/35] x86/fpu: Add helpers for modifying supervisor xstate Rick Edgecombe
2022-02-08  8:51   ` Thomas Gleixner
2022-02-09 19:55     ` Edgecombe, Rick P
2022-02-12  0:27   ` Dave Hansen
2022-02-12  2:31     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 24/35] mm: Re-introduce vm_flags to do_mmap() Rick Edgecombe
2022-01-30 21:18 ` [PATCH 25/35] x86/cet/shstk: Add user-mode shadow stack support Rick Edgecombe
2022-02-11 23:37   ` Dave Hansen
2022-02-12  0:07     ` Andy Lutomirski
2022-02-12  0:11       ` Dave Hansen
2022-02-12  0:12     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 26/35] x86/process: Change copy_thread() argument 'arg' to 'stack_size' Rick Edgecombe
2022-02-08  8:38   ` Thomas Gleixner
2022-02-11  2:09     ` Edgecombe, Rick P
2022-02-14 12:33   ` Jann Horn
2022-02-15  1:22     ` Edgecombe, Rick P
2022-02-15  8:49       ` Christian Brauner
2022-01-30 21:18 ` [PATCH 27/35] x86/fpu: Add unsafe xsave buffer helpers Rick Edgecombe
2022-01-30 21:18 ` [PATCH 28/35] x86/cet/shstk: Handle thread shadow stack Rick Edgecombe
2022-01-30 21:18 ` [PATCH 29/35] x86/cet/shstk: Introduce shadow stack token setup/verify routines Rick Edgecombe
2022-01-30 21:18 ` [PATCH 30/35] x86/cet/shstk: Handle signals for shadow stack Rick Edgecombe
2022-01-30 21:18 ` [PATCH 31/35] x86/cet/shstk: Add arch_prctl elf feature functions Rick Edgecombe
2022-01-30 21:18 ` [PATCH 32/35] x86/cet/shstk: Introduce map_shadow_stack syscall Rick Edgecombe
2022-01-30 21:18 ` [PATCH 33/35] selftests/x86: Add map_shadow_stack syscall test Rick Edgecombe
2022-02-03 22:42   ` Dave Hansen
2022-02-04  1:22     ` Edgecombe, Rick P
2022-01-30 21:18 ` [PATCH 34/35] x86/cet/shstk: Support wrss for userspace Rick Edgecombe
2022-01-31  7:56   ` Florian Weimer
2022-01-31 18:26     ` H.J. Lu
2022-01-31 18:45       ` Florian Weimer
2022-01-30 21:18 ` [PATCH 35/35] x86/cpufeatures: Limit shadow stack to Intel CPUs Rick Edgecombe
2022-02-03 21:58   ` John Allen
2022-02-03 22:23     ` H.J. Lu
2022-02-04 22:21       ` John Allen
2022-02-03 21:07 ` [PATCH 00/35] Shadow stacks for userspace Thomas Gleixner
2022-02-04  1:08   ` Edgecombe, Rick P
2022-02-04  5:20     ` Andy Lutomirski
2022-02-04 20:23       ` Edgecombe, Rick P [this message]
2022-02-05 13:26     ` David Laight
2022-02-05 13:29       ` H.J. Lu
2022-02-05 20:15         ` Edgecombe, Rick P
2022-02-05 20:21           ` H.J. Lu
2022-02-06 13:19             ` Peter Zijlstra
2022-02-06 13:42           ` David Laight
2022-02-06 13:55             ` H.J. Lu
2022-02-07 10:22             ` Florian Weimer
2022-02-08  1:46             ` Edgecombe, Rick P
2022-02-08  1:31           ` Andy Lutomirski
2022-02-08  9:31             ` Thomas Gleixner
2022-02-08 16:15               ` Andy Lutomirski
2022-02-06 13:06     ` Peter Zijlstra
2022-02-06 18:42 ` Mike Rapoport
2022-02-07  7:20   ` Adrian Reber
2022-02-07 16:30     ` Dave Hansen
2022-02-08  9:16       ` Mike Rapoport
2022-02-08  9:29         ` Cyrill Gorcunov
2022-02-08 16:21           ` Andy Lutomirski
2022-02-08 17:02             ` Cyrill Gorcunov
2022-02-08 21:54               ` Dmitry Safonov
2022-02-09  6:37                 ` Cyrill Gorcunov
2022-02-09  2:18               ` Edgecombe, Rick P
2022-02-09  6:43                 ` Cyrill Gorcunov
2022-02-09 10:53                 ` Mike Rapoport
2022-02-10  2:37                 ` Andy Lutomirski
2022-02-10  2:53                   ` H.J. Lu
2022-02-10 13:52                     ` Willgerodt, Felix
2022-02-11  7:41                   ` avagin
2022-02-11  8:04                     ` Mike Rapoport
2022-02-28 20:27                   ` Mike Rapoport
2022-02-28 20:30                     ` Andy Lutomirski
2022-02-28 21:30                       ` Mike Rapoport
2022-02-28 22:55                         ` Andy Lutomirski
2022-03-03 19:40                           ` Mike Rapoport
2022-03-03 23:00                             ` Andy Lutomirski
2022-03-04  1:30                               ` Edgecombe, Rick P
2022-03-04 19:13                                 ` Andy Lutomirski
2022-03-07 18:56                                   ` Mike Rapoport
2022-03-07 19:07                                     ` H.J. Lu
2022-05-31 11:59                                       ` Mike Rapoport
2022-05-31 16:25                                         ` Edgecombe, Rick P
2022-05-31 16:36                                           ` Mike Rapoport
2022-05-31 17:34                                             ` Edgecombe, Rick P
2022-05-31 18:00                                               ` H.J. Lu
2022-06-01 17:27                                                 ` Edgecombe, Rick P
2022-06-01 19:27                                                   ` H.J. Lu
2022-06-01  8:06                                               ` Mike Rapoport
2022-06-01 17:24                                                 ` Edgecombe, Rick P
2022-06-09 18:04                                                   ` Mike Rapoport
2022-03-07 22:21                                     ` David Laight

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=e83cf0ff1834bcfc7436d9370ae2807e5a026f32.camel@intel.com \
    --to=rick.p.edgecombe@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@linux.intel.com \
    --cc=eranian@google.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=john.allen@amd.com \
    --cc=kcc@google.com \
    --cc=keescook@chromium.org \
    --cc=kirill.shutemov@linux.intel.com \
    --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=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).