[v7,33/41] x86/shstk: Introduce map_shadow_stack syscall

  When operating with shadow stacks enabled, the kernel will automatically
allocate shadow stacks for new threads, however in some cases userspace
will need additional shadow stacks. The main example of this is the
ucontext family of functions, which require userspace allocating and
pivoting to userspace managed stacks.

Unlike most other user memory permissions, shadow stacks need to be
provisioned with special data in order to be useful. They need to be setup
with a restore token so that userspace can pivot to them via the RSTORSSP
instruction. But, the security design of shadow stack's is that they
should not be written to except in limited circumstances. This presents a
problem for userspace, as to how userspace can provision this special
data, without allowing for the shadow stack to be generally writable.

Previously, a new PROT_SHADOW_STACK was attempted, which could be
mprotect()ed from RW permissions after the data was provisioned. This was
found to not be secure enough, as other thread's could write to the
shadow stack during the writable window.

The kernel can use a special instruction, WRUSS, to write directly to
userspace shadow stacks. So the solution can be that memory can be mapped
as shadow stack permissions from the beginning (never generally writable
in userspace), and the kernel itself can write the restore token.

First, a new madvise() flag was explored, which could operate on the
PROT_SHADOW_STACK memory. This had a couple downsides:
1. Extra checks were needed in mprotect() to prevent writable memory from
   ever becoming PROT_SHADOW_STACK.
2. Extra checks/vma state were needed in the new madvise() to prevent
   restore tokens being written into the middle of pre-used shadow stacks.
   It is ideal to prevent restore tokens being added at arbitrary
   locations, so the check was to make sure the shadow stack had never been
   written to.
3. It stood out from the rest of the madvise flags, as more of direct
   action than a hint at future desired behavior.

So rather than repurpose two existing syscalls (mmap, madvise) that don't
quite fit, just implement a new map_shadow_stack syscall to allow
userspace to map and setup new shadow stacks in one step. While ucontext
is the primary motivator, userspace may have other unforeseen reasons to
setup it's own shadow stacks using the WRSS instruction. Towards this
provide a flag so that stacks can be optionally setup securely for the
common case of ucontext without enabling WRSS. Or potentially have the
kernel set up the shadow stack in some new way.

The following example demonstrates how to create a new shadow stack with
map_shadow_stack:
void *shstk = map_shadow_stack(addr, stack_size, SHADOW_STACK_SET_TOKEN);

Tested-by: Pengfei Xu <pengfei.xu@intel.com>
Tested-by: John Allen <john.allen@amd.com>
Tested-by: Kees Cook <keescook@chromium.org>
Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

---
v5:
 - Fix addr/mapped_addr (Kees)
 - Switch to EOPNOTSUPP (Kees suggested ENOTSUPP, but checkpatch
   suggests this)
 - Return error for addresses below 4G

v3:
 - Change syscall common -> 64 (Kees)
 - Use bit shift notation instead of 0x1 for uapi header (Kees)
 - Call do_mmap() with MAP_FIXED_NOREPLACE (Kees)
 - Block unsupported flags (Kees)
 - Require size >= 8 to set token (Kees)

v2:
 - Change syscall to take address like mmap() for CRIU's usage

v1:
 - New patch (replaces PROT_SHADOW_STACK).
---
 arch/x86/entry/syscalls/syscall_64.tbl |  1 +
 arch/x86/include/uapi/asm/mman.h       |  3 ++
 arch/x86/kernel/shstk.c                | 59 ++++++++++++++++++++++----
 include/linux/syscalls.h               |  1 +
 include/uapi/asm-generic/unistd.h      |  2 +-
 kernel/sys_ni.c                        |  1 +
 6 files changed, 58 insertions(+), 9 deletions(-)
  

The 02/27/2023 14:29, Rick Edgecombe wrote:
> Previously, a new PROT_SHADOW_STACK was attempted,
...
> So rather than repurpose two existing syscalls (mmap, madvise) that don't
> quite fit, just implement a new map_shadow_stack syscall to allow
> userspace to map and setup new shadow stacks in one step. While ucontext
> is the primary motivator, userspace may have other unforeseen reasons to
> setup it's own shadow stacks using the WRSS instruction. Towards this
> provide a flag so that stacks can be optionally setup securely for the
> common case of ucontext without enabling WRSS. Or potentially have the
> kernel set up the shadow stack in some new way.
...
> The following example demonstrates how to create a new shadow stack with
> map_shadow_stack:
> void *shstk = map_shadow_stack(addr, stack_size, SHADOW_STACK_SET_TOKEN);

i think

mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);

could do the same with less disruption to users (new syscalls
are harder to deal with than new flags). it would do the
guard page and initial token setup too (there is no flag for
it but could be squeezed in).

most of the mmap features need not be available (EINVAL) when
MAP_SHADOW_STACK is specified.

the main drawback is running out of mmap flags so extension
is limited. (but the new syscall has limitations too).
  

On Thu, 2023-03-02 at 17:22 +0000, Szabolcs Nagy wrote:
> The 02/27/2023 14:29, Rick Edgecombe wrote:
> > Previously, a new PROT_SHADOW_STACK was attempted,
> 
> ...
> > So rather than repurpose two existing syscalls (mmap, madvise) that
> > don't
> > quite fit, just implement a new map_shadow_stack syscall to allow
> > userspace to map and setup new shadow stacks in one step. While
> > ucontext
> > is the primary motivator, userspace may have other unforeseen
> > reasons to
> > setup it's own shadow stacks using the WRSS instruction. Towards
> > this
> > provide a flag so that stacks can be optionally setup securely for
> > the
> > common case of ucontext without enabling WRSS. Or potentially have
> > the
> > kernel set up the shadow stack in some new way.
> 
> ...
> > The following example demonstrates how to create a new shadow stack
> > with
> > map_shadow_stack:
> > void *shstk = map_shadow_stack(addr, stack_size,
> > SHADOW_STACK_SET_TOKEN);
> 
> i think
> 
> mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
> 
> could do the same with less disruption to users (new syscalls
> are harder to deal with than new flags). it would do the
> guard page and initial token setup too (there is no flag for
> it but could be squeezed in).
> 
> most of the mmap features need not be available (EINVAL) when
> MAP_SHADOW_STACK is specified.
> 
> the main drawback is running out of mmap flags so extension
> is limited. (but the new syscall has limitations too).

Deepak Gupta (working on riscv shadow stack) asked something similar.
Can you see if this thread answers your questions?

https://lore.kernel.org/lkml/20230223000340.GB945966@debug.ba.rivosinc.com/
  

On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy wrote:
>The 02/27/2023 14:29, Rick Edgecombe wrote:
>> Previously, a new PROT_SHADOW_STACK was attempted,
>...
>> So rather than repurpose two existing syscalls (mmap, madvise) that don't
>> quite fit, just implement a new map_shadow_stack syscall to allow
>> userspace to map and setup new shadow stacks in one step. While ucontext
>> is the primary motivator, userspace may have other unforeseen reasons to
>> setup it's own shadow stacks using the WRSS instruction. Towards this
>> provide a flag so that stacks can be optionally setup securely for the
>> common case of ucontext without enabling WRSS. Or potentially have the
>> kernel set up the shadow stack in some new way.
>...
>> The following example demonstrates how to create a new shadow stack with
>> map_shadow_stack:
>> void *shstk = map_shadow_stack(addr, stack_size, SHADOW_STACK_SET_TOKEN);
>
>i think
>
>mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
>
>could do the same with less disruption to users (new syscalls
>are harder to deal with than new flags). it would do the
>guard page and initial token setup too (there is no flag for
>it but could be squeezed in).

Discussion on this topic in v6
https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/

Again I know earlier CET patches had protection flag and somehow due to pushback
on mailing list, it was adopted to go for special syscall because no one else
had shadow stack.

Seeing a response from Szabolcs, I am assuming arm4 would also want to follow
using mmap to manufacture shadow stack. For reference RFC patches for risc-v shadow stack,
use a new protection flag = PROT_SHADOWSTACK.
https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/

I know earlier discussion had been that we let this go and do a re-factor later as other
arch support trickle in. But as I thought more on this and I think it may just be
messy from user mode point of view as well to have cognition of two different ways of
creating shadow stack. One would be special syscall (in current libc) and another `mmap`
(whenever future re-factor happens)

If it's not too late, it would be more wise to take `mmap`
approach rather than special `syscall` approach.


>
>most of the mmap features need not be available (EINVAL) when
>MAP_SHADOW_STACK is specified.
>
>the main drawback is running out of mmap flags so extension
>is limited. (but the new syscall has limitations too).
  

On Thu, 2023-03-09 at 10:55 -0800, Deepak Gupta wrote:
> On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy wrote:
> > The 02/27/2023 14:29, Rick Edgecombe wrote:
> > > Previously, a new PROT_SHADOW_STACK was attempted,
> > 
> > ...
> > > So rather than repurpose two existing syscalls (mmap, madvise)
> > > that don't
> > > quite fit, just implement a new map_shadow_stack syscall to allow
> > > userspace to map and setup new shadow stacks in one step. While
> > > ucontext
> > > is the primary motivator, userspace may have other unforeseen
> > > reasons to
> > > setup it's own shadow stacks using the WRSS instruction. Towards
> > > this
> > > provide a flag so that stacks can be optionally setup securely
> > > for the
> > > common case of ucontext without enabling WRSS. Or potentially
> > > have the
> > > kernel set up the shadow stack in some new way.
> > 
> > ...
> > > The following example demonstrates how to create a new shadow
> > > stack with
> > > map_shadow_stack:
> > > void *shstk = map_shadow_stack(addr, stack_size,
> > > SHADOW_STACK_SET_TOKEN);
> > 
> > i think
> > 
> > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
> > 
> > could do the same with less disruption to users (new syscalls
> > are harder to deal with than new flags). it would do the
> > guard page and initial token setup too (there is no flag for
> > it but could be squeezed in).
> 
> Discussion on this topic in v6
> 
https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
> 
> Again I know earlier CET patches had protection flag and somehow due
> to pushback
> on mailing list,
>  it was adopted to go for special syscall because no one else
> had shadow stack.
> 
> Seeing a response from Szabolcs, I am assuming arm4 would also want
> to follow
> using mmap to manufacture shadow stack. For reference RFC patches for
> risc-v shadow stack,
> use a new protection flag = PROT_SHADOWSTACK.
> 
https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
> 
> I know earlier discussion had been that we let this go and do a re-
> factor later as other
> arch support trickle in. But as I thought more on this and I think it
> may just be
> messy from user mode point of view as well to have cognition of two
> different ways of
> creating shadow stack. One would be special syscall (in current libc)
> and another `mmap`
> (whenever future re-factor happens)
> 
> If it's not too late, it would be more wise to take `mmap`
> approach rather than special `syscall` approach.

There is sort of two things intermixed here when we talk about a
PROT_SHADOW_STACK.

One is: what is the interface for specifying how the shadow stack
should be provisioned with data? Right now there are two ways
supported, all zero or with an X86 shadow stack restore token at the
end. Then there was already some conversation about a third type. In
which case the question would be is using mmap MAP_ flags the right
place for this? How many types of initialization will be needed in the
end and what is the overlap between the architectures?

The other thing is: should shadow stack memory creation be tightly
controlled? For example in x86 we limit this to anonymous memory, etc.
Some reasons for this are x86 specific, but some are not. So if we
disallow most of the options why allow the interface to take them? And
then you are in the position of carefully maintaining a list of not-
allowed options instead letting a list of allowed options sit there.

The only benefit I've heard is that it saves creating a new syscall,
but it also saves several MAP_ flags. That, and that the RFC for riscv
did a PROT_SHADOW_STACK to start. So, yes, two people asked the same
question, but I'm still not seeing any benefits. Can you give the pros
and cons please?

BTW, in glibc map_shadow_stack is called from arch code. So I think
userspace wise, for this to affect other architectures there would need
to be some code that could do things generically, with somehow the
shadow stack pivot abstracted but the shadow stack allocation not.
  

On Thu, Mar 09, 2023 at 07:39:41PM +0000, Edgecombe, Rick P wrote:
>On Thu, 2023-03-09 at 10:55 -0800, Deepak Gupta wrote:
>> On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy wrote:
>> > The 02/27/2023 14:29, Rick Edgecombe wrote:
>> > > Previously, a new PROT_SHADOW_STACK was attempted,
>> >
>> > ...
>> > > So rather than repurpose two existing syscalls (mmap, madvise)
>> > > that don't
>> > > quite fit, just implement a new map_shadow_stack syscall to allow
>> > > userspace to map and setup new shadow stacks in one step. While
>> > > ucontext
>> > > is the primary motivator, userspace may have other unforeseen
>> > > reasons to
>> > > setup it's own shadow stacks using the WRSS instruction. Towards
>> > > this
>> > > provide a flag so that stacks can be optionally setup securely
>> > > for the
>> > > common case of ucontext without enabling WRSS. Or potentially
>> > > have the
>> > > kernel set up the shadow stack in some new way.
>> >
>> > ...
>> > > The following example demonstrates how to create a new shadow
>> > > stack with
>> > > map_shadow_stack:
>> > > void *shstk = map_shadow_stack(addr, stack_size,
>> > > SHADOW_STACK_SET_TOKEN);
>> >
>> > i think
>> >
>> > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
>> >
>> > could do the same with less disruption to users (new syscalls
>> > are harder to deal with than new flags). it would do the
>> > guard page and initial token setup too (there is no flag for
>> > it but could be squeezed in).
>>
>> Discussion on this topic in v6
>>
>https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
>>
>> Again I know earlier CET patches had protection flag and somehow due
>> to pushback
>> on mailing list,
>>  it was adopted to go for special syscall because no one else
>> had shadow stack.
>>
>> Seeing a response from Szabolcs, I am assuming arm4 would also want
>> to follow
>> using mmap to manufacture shadow stack. For reference RFC patches for
>> risc-v shadow stack,
>> use a new protection flag = PROT_SHADOWSTACK.
>>
>https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
>>
>> I know earlier discussion had been that we let this go and do a re-
>> factor later as other
>> arch support trickle in. But as I thought more on this and I think it
>> may just be
>> messy from user mode point of view as well to have cognition of two
>> different ways of
>> creating shadow stack. One would be special syscall (in current libc)
>> and another `mmap`
>> (whenever future re-factor happens)
>>
>> If it's not too late, it would be more wise to take `mmap`
>> approach rather than special `syscall` approach.
>
>There is sort of two things intermixed here when we talk about a
>PROT_SHADOW_STACK.
>
>One is: what is the interface for specifying how the shadow stack
>should be provisioned with data? Right now there are two ways
>supported, all zero or with an X86 shadow stack restore token at the
>end. Then there was already some conversation about a third type. In
>which case the question would be is using mmap MAP_ flags the right
>place for this? How many types of initialization will be needed in the
>end and what is the overlap between the architectures?

First of all, arches can choose to have token at the bottom or not.

Token serve following purposes
  - It allows one to put desired value in shadow stack pointer in safe/secure manner.
    Note: x86 doesn't provide any opcode encoding to value in SSP register. So having
    a token is kind of a necessity because x86 doesn't easily allow writing shadow stack.

  - A token at the bottom acts marker / barrier and can be useful in debugging

  - If (and a big *if*) we ever reach a point in future where return address is only pushed
    on shadow stack (x86 should have motivation to do this because less uops on call/ret),
    a token at the bottom (bottom means lower address) is ensuring sure shot way of getting
    a fault when exhausted.

Current RISCV zisslpcfi proposal doesn't define CPU based tokens because it's RISC.
It allows mechanisms using which software can define formatting of token for itself.
Not sure of what ARM is doing.

Now coming to the point of all zero v/s shadow stack token.
Why not always have token at the bottom?

In case of x86, Why need for two ways and why not always have a token at the bottom.
The way x86 is going, user mode is responsible for establishing shadow stack and thus
whenever shadow stack is created then if x86 kernel implementation always place a token
at the base/bottom.

Now user mode can do following:--
  - If it has access to WRSS, it can sure go ahead and create a token of its choosing and
    overwrite kernel created token. and then do RSTORSSP on it's own created token.

  - If it doesn't have access to WRSS (and dont need to create its own token), it can do
    RSTORSSP on this. As soon as it does, no other thread in process can restore to it.
    On `fork`, you get the same un-restorable token.

So why not always have a token at the bottom.
This is my plan for riscv implementation as well (to have a token at the bottom)

>
>The other thing is: should shadow stack memory creation be tightly
>controlled? For example in x86 we limit this to anonymous memory, etc.
>Some reasons for this are x86 specific, but some are not. So if we
>disallow most of the options why allow the interface to take them? And
>then you are in the position of carefully maintaining a list of not-
>allowed options instead letting a list of allowed options sit there.

I am new to linux kernel and thus may be not able to follow the argument of
limiting to anonymous memory.

Why is limiting it to anonymous memory a problem. IIRC, ARM's PROT_MTE is applicable
only to anonymous memory. I can probably find few more examples. 

Eventually syscall will also go ahead and use memory management code to
perform mapping. So I didn't understand the reasoning here. The way syscall
can limit it to anonymous memory, why mmap can't do the same if it sees
PROT_SHADOWSTACK.

>
>The only benefit I've heard is that it saves creating a new syscall,
>but it also saves several MAP_ flags. That, and that the RFC for riscv
>did a PROT_SHADOW_STACK to start. So, yes, two people asked the same
>question, but I'm still not seeing any benefits. Can you give the pros
>and cons please?

Again the way syscall will limit it to anonymous memory, Why mmap can't do same?
There is precedence for it (like PROT_MTE is applicable only to anonymous memory)

So if it can be done, then why introduce a new syscall?

>
>BTW, in glibc map_shadow_stack is called from arch code. So I think
>userspace wise, for this to affect other architectures there would need
>to be some code that could do things generically, with somehow the
>shadow stack pivot abstracted but the shadow stack allocation not.

Agreed, yes it can be done in a way where it won't put tax on other architectures.

But what about fragmentation within x86. Will x86 always choose to use system call
method map shadow stack. If future re-factor results in x86 also use `mmap` method.
Isn't it a mess for x86 glibc to figure out what to do; whether to use system call
or `mmap`?
  

On Thu, 2023-03-09 at 13:08 -0800, Deepak Gupta wrote:
> On Thu, Mar 09, 2023 at 07:39:41PM +0000, Edgecombe, Rick P wrote:
> > On Thu, 2023-03-09 at 10:55 -0800, Deepak Gupta wrote:
> > > On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy wrote:
> > > > The 02/27/2023 14:29, Rick Edgecombe wrote:
> > > > > Previously, a new PROT_SHADOW_STACK was attempted,
> > > > 
> > > > ...
> > > > > So rather than repurpose two existing syscalls (mmap,
> > > > > madvise)
> > > > > that don't
> > > > > quite fit, just implement a new map_shadow_stack syscall to
> > > > > allow
> > > > > userspace to map and setup new shadow stacks in one step.
> > > > > While
> > > > > ucontext
> > > > > is the primary motivator, userspace may have other unforeseen
> > > > > reasons to
> > > > > setup it's own shadow stacks using the WRSS instruction.
> > > > > Towards
> > > > > this
> > > > > provide a flag so that stacks can be optionally setup
> > > > > securely
> > > > > for the
> > > > > common case of ucontext without enabling WRSS. Or potentially
> > > > > have the
> > > > > kernel set up the shadow stack in some new way.
> > > > 
> > > > ...
> > > > > The following example demonstrates how to create a new shadow
> > > > > stack with
> > > > > map_shadow_stack:
> > > > > void *shstk = map_shadow_stack(addr, stack_size,
> > > > > SHADOW_STACK_SET_TOKEN);
> > > > 
> > > > i think
> > > > 
> > > > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
> > > > 
> > > > could do the same with less disruption to users (new syscalls
> > > > are harder to deal with than new flags). it would do the
> > > > guard page and initial token setup too (there is no flag for
> > > > it but could be squeezed in).
> > > 
> > > Discussion on this topic in v6
> > > 
> > 
> > 
https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
> > > 
> > > Again I know earlier CET patches had protection flag and somehow
> > > due
> > > to pushback
> > > on mailing list,
> > >  it was adopted to go for special syscall because no one else
> > > had shadow stack.
> > > 
> > > Seeing a response from Szabolcs, I am assuming arm4 would also
> > > want
> > > to follow
> > > using mmap to manufacture shadow stack. For reference RFC patches
> > > for
> > > risc-v shadow stack,
> > > use a new protection flag = PROT_SHADOWSTACK.
> > > 
> > 
> > 
https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
> > > 
> > > I know earlier discussion had been that we let this go and do a
> > > re-
> > > factor later as other
> > > arch support trickle in. But as I thought more on this and I
> > > think it
> > > may just be
> > > messy from user mode point of view as well to have cognition of
> > > two
> > > different ways of
> > > creating shadow stack. One would be special syscall (in current
> > > libc)
> > > and another `mmap`
> > > (whenever future re-factor happens)
> > > 
> > > If it's not too late, it would be more wise to take `mmap`
> > > approach rather than special `syscall` approach.
> > 
> > There is sort of two things intermixed here when we talk about a
> > PROT_SHADOW_STACK.
> > 
> > One is: what is the interface for specifying how the shadow stack
> > should be provisioned with data? Right now there are two ways
> > supported, all zero or with an X86 shadow stack restore token at
> > the
> > end. Then there was already some conversation about a third type.
> > In
> > which case the question would be is using mmap MAP_ flags the right
> > place for this? How many types of initialization will be needed in
> > the
> > end and what is the overlap between the architectures?
> 
> First of all, arches can choose to have token at the bottom or not.
> 
> Token serve following purposes
>   - It allows one to put desired value in shadow stack pointer in
> safe/secure manner.
>     Note: x86 doesn't provide any opcode encoding to value in SSP
> register. So having
>     a token is kind of a necessity because x86 doesn't easily allow
> writing shadow stack.
> 
>   - A token at the bottom acts marker / barrier and can be useful in
> debugging
> 
>   - If (and a big *if*) we ever reach a point in future where return
> address is only pushed
>     on shadow stack (x86 should have motivation to do this because
> less uops on call/ret),
>     a token at the bottom (bottom means lower address) is ensuring
> sure shot way of getting
>     a fault when exhausted.
> 
> Current RISCV zisslpcfi proposal doesn't define CPU based tokens
> because it's RISC.
> It allows mechanisms using which software can define formatting of
> token for itself.
> Not sure of what ARM is doing.

Ok, so riscv doesn't need to have the kernel write the token, but x86
does.

> 
> Now coming to the point of all zero v/s shadow stack token.
> Why not always have token at the bottom?

With WRSS you can setup the shadow stack however you want. So the user
would then have to take care to erase the token if they didn't want it.
Not the end of the world, but kind of clunky if there is no reason for
it.

> 
> In case of x86, Why need for two ways and why not always have a token
> at the bottom.
> The way x86 is going, user mode is responsible for establishing
> shadow stack and thus
> whenever shadow stack is created then if x86 kernel implementation
> always place a token
> at the base/bottom.

There was also some discussion recently of adding a token AND an end of
stack marker, as a potential solution for backtracing in ucontext
stacks. In this case it could cause an ABI break to just start adding
the end of stack marker where the token was, and so would require a new
map_shadow_stack flag.

> 
> Now user mode can do following:--
>   - If it has access to WRSS, it can sure go ahead and create a token
> of its choosing and
>     overwrite kernel created token. and then do RSTORSSP on it's own
> created token.
> 
>   - If it doesn't have access to WRSS (and dont need to create its
> own token), it can do
>     RSTORSSP on this. As soon as it does, no other thread in process
> can restore to it.
>     On `fork`, you get the same un-restorable token.
> 
> So why not always have a token at the bottom.
> This is my plan for riscv implementation as well (to have a token at
> the bottom)
> 
> > 
> > The other thing is: should shadow stack memory creation be tightly
> > controlled? For example in x86 we limit this to anonymous memory,
> > etc.
> > Some reasons for this are x86 specific, but some are not. So if we
> > disallow most of the options why allow the interface to take them?
> > And
> > then you are in the position of carefully maintaining a list of
> > not-
> > allowed options instead letting a list of allowed options sit
> > there.
> 
> I am new to linux kernel and thus may be not able to follow the
> argument of
> limiting to anonymous memory.
> 
> Why is limiting it to anonymous memory a problem. IIRC, ARM's
> PROT_MTE is applicable
> only to anonymous memory. I can probably find few more examples. 

Oh I see, they have a special arch VMA flag VM_MTE_ALLOWED that only
gets set if all the rules are followed. Then PROT_MTE can only be set
on that to set VM_MTE. That is kind of nice because certain other
special situations can choose to support it.

It does take another arch vma flag though. For x86 I guess I would need
to figure out how to squeeze VM_SHADOW_STACK into other flags to have a
free flag to use the same method. It also only supports mprotect() and
shadow stack would only want to support mmap(). And you still have the
initialization stuff to plumb through. Yea, I think the PROT_MTE is a
good thing to consider, but it's not super obvious to me how similar
the logic would be for shadow stack.

The question I'm asking though is, not "can mmap code and rules be
changed to enforce the required limitations?". I think it is yes. But
the question is "why is that plumbing better than a new syscall?". I
guess to get a better idea, the mmap solution would need to get POCed.
I had half done this at one point, but abandoned the approach.

For your question about why limit it, the special x86 case is the
Dirty=1,Write=0 PTE bit combination for shadow stacks. So for shadow
stack you could have some confusion about whether a PTE is actually
dirty for writeback, etc. I wouldn't say it's known to be impossible to
do MAP_SHARED, but it has not been fully analyzed enough to know what
the changes would be. There were some solvable concrete issues that
tipped the scale as well. It was also not expected to be a common
usage, if at all.

The non-x86, general reasons for it, are for a smaller benefit. It
blocks a lot of ways shadow stack memory could be written to. Like say
you have a memory mapped writable file, and you also map it shadow
stack. So it has better security properties depending on what your
threat model is.

> 
> Eventually syscall will also go ahead and use memory management code
> to
> perform mapping. So I didn't understand the reasoning here. The way
> syscall
> can limit it to anonymous memory, why mmap can't do the same if it
> sees
> PROT_SHADOWSTACK.
> 
> > 
> > The only benefit I've heard is that it saves creating a new
> > syscall,
> > but it also saves several MAP_ flags. That, and that the RFC for
> > riscv
> > did a PROT_SHADOW_STACK to start. So, yes, two people asked the
> > same
> > question, but I'm still not seeing any benefits. Can you give the
> > pros
> > and cons please?
> 
> Again the way syscall will limit it to anonymous memory, Why mmap
> can't do same?
> There is precedence for it (like PROT_MTE is applicable only to
> anonymous memory)
> 
> So if it can be done, then why introduce a new syscall?
> 
> > 
> > BTW, in glibc map_shadow_stack is called from arch code. So I think
> > userspace wise, for this to affect other architectures there would
> > need
> > to be some code that could do things generically, with somehow the
> > shadow stack pivot abstracted but the shadow stack allocation not.
> 
> Agreed, yes it can be done in a way where it won't put tax on other
> architectures.
> 
> But what about fragmentation within x86. Will x86 always choose to
> use system call
> method map shadow stack. If future re-factor results in x86 also use
> `mmap` method.
> Isn't it a mess for x86 glibc to figure out what to do; whether to
> use system call
> or `mmap`?
> 

Ok, so this is the downside I guess. What happens if we want to support
the other types of memory in the future and end up using mmap for this?
Then we have 15-20 lines of extra syscall wrapping code to maintain to
support legacy.

For the mmap solution, we have the downside of using extra MAP_ flags,
and *some* amount of currently unknown vm_flag and address range logic,
plus mmap arch breakouts to add to core MM. Like I said earlier, you
would need to POC it out to see how bad that looks and get some core MM
feedback on the new type of MAP flag usage. But, syscalls being pretty
straightforward, it would probably be *some* amount of added complexity
_now_ to support something that might happen in the future. I'm not
seeing either one as a landslide win.

It's kind of an eternal software design philosophical question, isn't
it? How much work should you do to prepare for things that might be
needed in the future? From what I've seen the balance in the kernel
seems to be to try not to paint yourself in to an ABI corner, but
otherwise let the kernel evolve naturally in response to real usages.
If anyone wants to correct this, please do. But otherwise I think the
new syscall is aligned with that.

TBH, you are making me wonder if I'm missing something. It seems you
strongly don't prefer this approach, but I'm not hearing any huge
potential negative impacts. And you also say it won't tax the riscv
implementation. Is this just something just smells bad here? Or it
would shrink the riscv series?
  

On Mon, Feb 27, 2023 at 02:29:49PM -0800, Rick Edgecombe wrote:
> When operating with shadow stacks enabled, the kernel will automatically
> allocate shadow stacks for new threads, however in some cases userspace
> will need additional shadow stacks. The main example of this is the
> ucontext family of functions, which require userspace allocating and
> pivoting to userspace managed stacks.
> 
> Unlike most other user memory permissions, shadow stacks need to be
> provisioned with special data in order to be useful. They need to be setup
> with a restore token so that userspace can pivot to them via the RSTORSSP
> instruction. But, the security design of shadow stack's is that they

"stacks"

> should not be written to except in limited circumstances. This presents a
> problem for userspace, as to how userspace can provision this special
> data, without allowing for the shadow stack to be generally writable.
> 
> Previously, a new PROT_SHADOW_STACK was attempted, which could be
> mprotect()ed from RW permissions after the data was provisioned. This was
> found to not be secure enough, as other thread's could write to the

"threads"

> shadow stack during the writable window.
> 
> The kernel can use a special instruction, WRUSS, to write directly to
> userspace shadow stacks. So the solution can be that memory can be mapped
> as shadow stack permissions from the beginning (never generally writable
> in userspace), and the kernel itself can write the restore token.
> 
> First, a new madvise() flag was explored, which could operate on the
> PROT_SHADOW_STACK memory. This had a couple downsides:
					     ^
					     of


> 1. Extra checks were needed in mprotect() to prevent writable memory from
>    ever becoming PROT_SHADOW_STACK.
> 2. Extra checks/vma state were needed in the new madvise() to prevent
>    restore tokens being written into the middle of pre-used shadow stacks.
>    It is ideal to prevent restore tokens being added at arbitrary
>    locations, so the check was to make sure the shadow stack had never been
>    written to.
> 3. It stood out from the rest of the madvise flags, as more of direct
>    action than a hint at future desired behavior.
> 
> So rather than repurpose two existing syscalls (mmap, madvise) that don't
> quite fit, just implement a new map_shadow_stack syscall to allow
> userspace to map and setup new shadow stacks in one step. While ucontext
> is the primary motivator, userspace may have other unforeseen reasons to
> setup it's own shadow stacks using the WRSS instruction. Towards this

"its"

> provide a flag so that stacks can be optionally setup securely for the
> common case of ucontext without enabling WRSS. Or potentially have the
> kernel set up the shadow stack in some new way.
> 
> The following example demonstrates how to create a new shadow stack with
> map_shadow_stack:
> void *shstk = map_shadow_stack(addr, stack_size, SHADOW_STACK_SET_TOKEN);

...

> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index c84d12608cd2..f65c671ce3b1 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -372,6 +372,7 @@
>  448	common	process_mrelease	sys_process_mrelease
>  449	common	futex_waitv		sys_futex_waitv
>  450	common	set_mempolicy_home_node	sys_set_mempolicy_home_node
> +451	64	map_shadow_stack	sys_map_shadow_stack

Yeah, this'll need a manpage too, I presume. But later.

> +SYSCALL_DEFINE3(map_shadow_stack, unsigned long, addr, unsigned long, size, unsigned int, flags)
> +{
> +	bool set_tok = flags & SHADOW_STACK_SET_TOKEN;
> +	unsigned long aligned_size;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK))
> +		return -EOPNOTSUPP;
> +
> +	if (flags & ~SHADOW_STACK_SET_TOKEN)
> +		return -EINVAL;
> +
> +	/* If there isn't space for a token */
> +	if (set_tok && size < 8)
> +		return -EINVAL;
> +
> +	if (addr && addr <= 0xFFFFFFFF)

			< SZ_4G

> +		return -EINVAL;

Can we use distinct negative retvals in each case so that it is clear to
userspace where it fails, *if* it fails?

> +	/*
> +	 * An overflow would result in attempting to write the restore token
> +	 * to the wrong location. Not catastrophic, but just return the right
> +	 * error code and block it.
> +	 */
> +	aligned_size = PAGE_ALIGN(size);
> +	if (aligned_size < size)
> +		return -EOVERFLOW;
> +
> +	return alloc_shstk(addr, aligned_size, size, set_tok);
> +}
  

On Fri, 2023-03-10 at 17:11 +0100, Borislav Petkov wrote:

[...]

Thanks on all the text edits.

> On Mon, Feb 27, 2023 at 02:29:49PM -0800, Rick Edgecombe wrote:
> > diff --git a/arch/x86/entry/syscalls/syscall_64.tbl
> > b/arch/x86/entry/syscalls/syscall_64.tbl
> > index c84d12608cd2..f65c671ce3b1 100644
> > --- a/arch/x86/entry/syscalls/syscall_64.tbl
> > +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> > @@ -372,6 +372,7 @@
> >  448	common	process_mrelease	sys_process_mreleas
> > e
> >  449	common	futex_waitv		sys_futex_waitv
> >  450	common	set_mempolicy_home_node	sys_set_mempolicy_h
> > ome_node
> > +451	64	map_shadow_stack	sys_map_shadow_stack
> 
> Yeah, this'll need a manpage too, I presume. But later.

I have one to submit.

[...]

> > +
> > +	if (addr && addr <= 0xFFFFFFFF)
> 
> 			< SZ_4G
> 
> > +		return -EINVAL;
> 
> Can we use distinct negative retvals in each case so that it is clear
> to
> userspace where it fails, *if* it fails?

Good idea, I think maybe ERANGE.
  

On Fri, Mar 10, 2023 at 05:12:40PM +0000, Edgecombe, Rick P wrote:
> > Can we use distinct negative retvals in each case so that it is clear
> > to
> > userspace where it fails, *if* it fails?
> 
> Good idea, I think maybe ERANGE.

For those two, right?

        /* If there isn't space for a token */
        if (set_tok && size < 8)
                return -EINVAL;

        if (addr && addr <= 0xFFFFFFFF)
                return -EINVAL;

They are kinda range-checking of sorts. A wider range but still
similar...
  

On Fri, 2023-03-10 at 21:05 +0100, Borislav Petkov wrote:
> On Fri, Mar 10, 2023 at 05:12:40PM +0000, Edgecombe, Rick P wrote:
> > > Can we use distinct negative retvals in each case so that it is
> > > clear
> > > to
> > > userspace where it fails, *if* it fails?
> > 
> > Good idea, I think maybe ERANGE.
> 
> For those two, right?
> 
>         /* If there isn't space for a token */
>         if (set_tok && size < 8)
>                 return -EINVAL;
> 
>         if (addr && addr <= 0xFFFFFFFF)
>                 return -EINVAL;
> 
> They are kinda range-checking of sorts. A wider range but still
> similar... 

I was thinking ERANGE would be for the 4GB limit. This is the weird 32
bit limiting thing. So if someone hit it they could look up in the docs
what is going on. The size-big-enough-for-a-token check could be
ENOSPC? Then each reason could have a different error code.
  

On Fri, Mar 10, 2023 at 08:19:46PM +0000, Edgecombe, Rick P wrote:
> I was thinking ERANGE would be for the 4GB limit. This is the weird 32
> bit limiting thing. So if someone hit it they could look up in the docs
> what is going on. The size-big-enough-for-a-token check could be
> ENOSPC? Then each reason could have a different error code.

Yah, makes perfect sense to me.

Thx.
  

On Fri, Mar 10, 2023 at 12:14:01AM +0000, Edgecombe, Rick P wrote:
>On Thu, 2023-03-09 at 13:08 -0800, Deepak Gupta wrote:
>> On Thu, Mar 09, 2023 at 07:39:41PM +0000, Edgecombe, Rick P wrote:
>> > On Thu, 2023-03-09 at 10:55 -0800, Deepak Gupta wrote:
>> > > On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy wrote:
>> > > > The 02/27/2023 14:29, Rick Edgecombe wrote:
>> > > > > Previously, a new PROT_SHADOW_STACK was attempted,
>> > > >
>> > > > ...
>> > > > > So rather than repurpose two existing syscalls (mmap,
>> > > > > madvise)
>> > > > > that don't
>> > > > > quite fit, just implement a new map_shadow_stack syscall to
>> > > > > allow
>> > > > > userspace to map and setup new shadow stacks in one step.
>> > > > > While
>> > > > > ucontext
>> > > > > is the primary motivator, userspace may have other unforeseen
>> > > > > reasons to
>> > > > > setup it's own shadow stacks using the WRSS instruction.
>> > > > > Towards
>> > > > > this
>> > > > > provide a flag so that stacks can be optionally setup
>> > > > > securely
>> > > > > for the
>> > > > > common case of ucontext without enabling WRSS. Or potentially
>> > > > > have the
>> > > > > kernel set up the shadow stack in some new way.
>> > > >
>> > > > ...
>> > > > > The following example demonstrates how to create a new shadow
>> > > > > stack with
>> > > > > map_shadow_stack:
>> > > > > void *shstk = map_shadow_stack(addr, stack_size,
>> > > > > SHADOW_STACK_SET_TOKEN);
>> > > >
>> > > > i think
>> > > >
>> > > > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
>> > > >
>> > > > could do the same with less disruption to users (new syscalls
>> > > > are harder to deal with than new flags). it would do the
>> > > > guard page and initial token setup too (there is no flag for
>> > > > it but could be squeezed in).
>> > >
>> > > Discussion on this topic in v6
>> > >
>> >
>> >
>https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
>> > >
>> > > Again I know earlier CET patches had protection flag and somehow
>> > > due
>> > > to pushback
>> > > on mailing list,
>> > >  it was adopted to go for special syscall because no one else
>> > > had shadow stack.
>> > >
>> > > Seeing a response from Szabolcs, I am assuming arm4 would also
>> > > want
>> > > to follow
>> > > using mmap to manufacture shadow stack. For reference RFC patches
>> > > for
>> > > risc-v shadow stack,
>> > > use a new protection flag = PROT_SHADOWSTACK.
>> > >
>> >
>> >
>https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
>> > >
>> > > I know earlier discussion had been that we let this go and do a
>> > > re-
>> > > factor later as other
>> > > arch support trickle in. But as I thought more on this and I
>> > > think it
>> > > may just be
>> > > messy from user mode point of view as well to have cognition of
>> > > two
>> > > different ways of
>> > > creating shadow stack. One would be special syscall (in current
>> > > libc)
>> > > and another `mmap`
>> > > (whenever future re-factor happens)
>> > >
>> > > If it's not too late, it would be more wise to take `mmap`
>> > > approach rather than special `syscall` approach.
>> >
>> > There is sort of two things intermixed here when we talk about a
>> > PROT_SHADOW_STACK.
>> >
>> > One is: what is the interface for specifying how the shadow stack
>> > should be provisioned with data? Right now there are two ways
>> > supported, all zero or with an X86 shadow stack restore token at
>> > the
>> > end. Then there was already some conversation about a third type.
>> > In
>> > which case the question would be is using mmap MAP_ flags the right
>> > place for this? How many types of initialization will be needed in
>> > the
>> > end and what is the overlap between the architectures?
>>
>> First of all, arches can choose to have token at the bottom or not.
>>
>> Token serve following purposes
>>   - It allows one to put desired value in shadow stack pointer in
>> safe/secure manner.
>>     Note: x86 doesn't provide any opcode encoding to value in SSP
>> register. So having
>>     a token is kind of a necessity because x86 doesn't easily allow
>> writing shadow stack.
>>
>>   - A token at the bottom acts marker / barrier and can be useful in
>> debugging
>>
>>   - If (and a big *if*) we ever reach a point in future where return
>> address is only pushed
>>     on shadow stack (x86 should have motivation to do this because
>> less uops on call/ret),
>>     a token at the bottom (bottom means lower address) is ensuring
>> sure shot way of getting
>>     a fault when exhausted.
>>
>> Current RISCV zisslpcfi proposal doesn't define CPU based tokens
>> because it's RISC.
>> It allows mechanisms using which software can define formatting of
>> token for itself.
>> Not sure of what ARM is doing.
>
>Ok, so riscv doesn't need to have the kernel write the token, but x86
>does.
>
>>
>> Now coming to the point of all zero v/s shadow stack token.
>> Why not always have token at the bottom?
>
>With WRSS you can setup the shadow stack however you want. So the user
>would then have to take care to erase the token if they didn't want it.
>Not the end of the world, but kind of clunky if there is no reason for
>it.

Yes but kernel always assumes the user is going to use the token. It' upto the user
to decide whether they want to use the restore token or not. If they've WRSS capability
security posture is anyways diluted. An attacker who would be clever enough to
re-use `RSTORSSP` present in address space to restore using kernel prepared token, should
anyways can be clever enough to use WRSS as well.

It kind of makes shadow stack creation simpler for kernel to always place the token.
This point is irrespective of whether to use system call or mmap.

>
>>
>> In case of x86, Why need for two ways and why not always have a token
>> at the bottom.
>> The way x86 is going, user mode is responsible for establishing
>> shadow stack and thus
>> whenever shadow stack is created then if x86 kernel implementation
>> always place a token
>> at the base/bottom.
>
>There was also some discussion recently of adding a token AND an end of
>stack marker, as a potential solution for backtracing in ucontext
>stacks. In this case it could cause an ABI break to just start adding
>the end of stack marker where the token was, and so would require a new
>map_shadow_stack flag.

Was this discussed why restore token itself can't be used as marker for
end of stack (if we assume there is always going to be one at the bottom).
It's a unique value. An address pointing to itself.

>
>>
>> Now user mode can do following:--
>>   - If it has access to WRSS, it can sure go ahead and create a token
>> of its choosing and
>>     overwrite kernel created token. and then do RSTORSSP on it's own
>> created token.
>>
>>   - If it doesn't have access to WRSS (and dont need to create its
>> own token), it can do
>>     RSTORSSP on this. As soon as it does, no other thread in process
>> can restore to it.
>>     On `fork`, you get the same un-restorable token.
>>
>> So why not always have a token at the bottom.
>> This is my plan for riscv implementation as well (to have a token at
>> the bottom)
>>
>> >
>> > The other thing is: should shadow stack memory creation be tightly
>> > controlled? For example in x86 we limit this to anonymous memory,
>> > etc.
>> > Some reasons for this are x86 specific, but some are not. So if we
>> > disallow most of the options why allow the interface to take them?
>> > And
>> > then you are in the position of carefully maintaining a list of
>> > not-
>> > allowed options instead letting a list of allowed options sit
>> > there.
>>
>> I am new to linux kernel and thus may be not able to follow the
>> argument of
>> limiting to anonymous memory.
>>
>> Why is limiting it to anonymous memory a problem. IIRC, ARM's
>> PROT_MTE is applicable
>> only to anonymous memory. I can probably find few more examples.
>
>Oh I see, they have a special arch VMA flag VM_MTE_ALLOWED that only
>gets set if all the rules are followed. Then PROT_MTE can only be set
>on that to set VM_MTE. That is kind of nice because certain other
>special situations can choose to support it.

That's because MTE is different. It allows to assign tags to existing
virtual memory. So one need to know whether a memory can have tags assigned.

>
>It does take another arch vma flag though. For x86 I guess I would need
>to figure out how to squeeze VM_SHADOW_STACK into other flags to have a
>free flag to use the same method. It also only supports mprotect() and
>shadow stack would only want to support mmap(). And you still have the
>initialization stuff to plumb through. Yea, I think the PROT_MTE is a
>good thing to consider, but it's not super obvious to me how similar
>the logic would be for shadow stack.

I dont think you need another VMA flag. Memory tagging allows adding tags
to existing virtual memory. That's why having `mprotect` makes sense for MTE.
In shadow stack case, there is no requirement of changing a shadow stack
to regular memory or vice-versa. 

All that's needed to change is `mmap`. `mprotect` should fail. Syscall
approach gives that benefit by default because there is no protection flag
for shadow stack.

I was giving example that any feature which gives new meaning to virtual memory
has been able to work with existing memory mapping APIs without the need of new
system call (including whether you're dealing with anonymous memory).

>
>The question I'm asking though is, not "can mmap code and rules be
>changed to enforce the required limitations?". I think it is yes. But
>the question is "why is that plumbing better than a new syscall?". I
>guess to get a better idea, the mmap solution would need to get POCed.
>I had half done this at one point, but abandoned the approach.
>
>For your question about why limit it, the special x86 case is the
>Dirty=1,Write=0 PTE bit combination for shadow stacks. So for shadow
>stack you could have some confusion about whether a PTE is actually
>dirty for writeback, etc. I wouldn't say it's known to be impossible to
>do MAP_SHARED, but it has not been fully analyzed enough to know what
>the changes would be. There were some solvable concrete issues that
>tipped the scale as well. It was also not expected to be a common
>usage, if at all.

I am not sure how confusion of D=1,W=0 is not completely taken away by
syscall approach. It'll always be there. One can only do things to minimize
the chances.

In case of syscall approach, syscall makes sure that 

`flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_ABOVE4G`

This can be easily checked in arch specific landing function for mmap.


Additionally, If you always have the token at base, you don't need that ABI
between user and kernel.


>
>The non-x86, general reasons for it, are for a smaller benefit. It
>blocks a lot of ways shadow stack memory could be written to. Like say
>you have a memory mapped writable file, and you also map it shadow
>stack. So it has better security properties depending on what your
>threat model is.

I wouldn't say any architecture should allow such primitives. It kind of defeats
the purpose for shadow stack. Yes if some sort of secure memory is needed, there may
be new ISA extensions for that.

>
>>
>> Eventually syscall will also go ahead and use memory management code
>> to
>> perform mapping. So I didn't understand the reasoning here. The way
>> syscall
>> can limit it to anonymous memory, why mmap can't do the same if it
>> sees
>> PROT_SHADOWSTACK.
>>
>> >
>> > The only benefit I've heard is that it saves creating a new
>> > syscall,
>> > but it also saves several MAP_ flags. That, and that the RFC for
>> > riscv
>> > did a PROT_SHADOW_STACK to start. So, yes, two people asked the
>> > same
>> > question, but I'm still not seeing any benefits. Can you give the
>> > pros
>> > and cons please?
>>
>> Again the way syscall will limit it to anonymous memory, Why mmap
>> can't do same?
>> There is precedence for it (like PROT_MTE is applicable only to
>> anonymous memory)
>>
>> So if it can be done, then why introduce a new syscall?
>>
>> >
>> > BTW, in glibc map_shadow_stack is called from arch code. So I think
>> > userspace wise, for this to affect other architectures there would
>> > need
>> > to be some code that could do things generically, with somehow the
>> > shadow stack pivot abstracted but the shadow stack allocation not.
>>
>> Agreed, yes it can be done in a way where it won't put tax on other
>> architectures.
>>
>> But what about fragmentation within x86. Will x86 always choose to
>> use system call
>> method map shadow stack. If future re-factor results in x86 also use
>> `mmap` method.
>> Isn't it a mess for x86 glibc to figure out what to do; whether to
>> use system call
>> or `mmap`?
>>
>
>Ok, so this is the downside I guess. What happens if we want to support
>the other types of memory in the future and end up using mmap for this?
>Then we have 15-20 lines of extra syscall wrapping code to maintain to
>support legacy.
>
>For the mmap solution, we have the downside of using extra MAP_ flags,
>and *some* amount of currently unknown vm_flag and address range logic,
>plus mmap arch breakouts to add to core MM. Like I said earlier, you
>would need to POC it out to see how bad that looks and get some core MM
>feedback on the new type of MAP flag usage. But, syscalls being pretty
>straightforward, it would probably be *some* amount of added complexity
>_now_ to support something that might happen in the future. I'm not
>seeing either one as a landslide win.
>
>It's kind of an eternal software design philosophical question, isn't
>it? How much work should you do to prepare for things that might be
>needed in the future? From what I've seen the balance in the kernel
>seems to be to try not to paint yourself in to an ABI corner, but
>otherwise let the kernel evolve naturally in response to real usages.
>If anyone wants to correct this, please do. But otherwise I think the
>new syscall is aligned with that.
>
>TBH, you are making me wonder if I'm missing something. It seems you
>strongly don't prefer this approach, but I'm not hearing any huge
>potential negative impacts. And you also say it won't tax the riscv
>implementation. Is this just something just smells bad here? Or it
>would shrink the riscv series?

No you're not missing anything. It's just wierdness of adding a system call
which enforces certain MAP_XX flags and pretty much mapping API.
And difference between architectures on how they will create shadow stack. + 
if x86 chooses to use `mmap` in future, then there is ugliness in user mode to
decide which method to choose.

And yes you got it right, to some extent there is my own selfishness playing out
as well here to reduce riscv patches.
  

On Fri, 2023-03-10 at 13:00 -0800, Deepak Gupta wrote:
> On Fri, Mar 10, 2023 at 12:14:01AM +0000, Edgecombe, Rick P wrote:
> > On Thu, 2023-03-09 at 13:08 -0800, Deepak Gupta wrote:
> > > On Thu, Mar 09, 2023 at 07:39:41PM +0000, Edgecombe, Rick P
> > > wrote:
> > > > On Thu, 2023-03-09 at 10:55 -0800, Deepak Gupta wrote:
> > > > > On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy
> > > > > wrote:
> > > > > > The 02/27/2023 14:29, Rick Edgecombe wrote:
> > > > > > > Previously, a new PROT_SHADOW_STACK was attempted,
> > > > > > 
> > > > > > ...
> > > > > > > So rather than repurpose two existing syscalls (mmap,
> > > > > > > madvise)
> > > > > > > that don't
> > > > > > > quite fit, just implement a new map_shadow_stack syscall
> > > > > > > to
> > > > > > > allow
> > > > > > > userspace to map and setup new shadow stacks in one step.
> > > > > > > While
> > > > > > > ucontext
> > > > > > > is the primary motivator, userspace may have other
> > > > > > > unforeseen
> > > > > > > reasons to
> > > > > > > setup it's own shadow stacks using the WRSS instruction.
> > > > > > > Towards
> > > > > > > this
> > > > > > > provide a flag so that stacks can be optionally setup
> > > > > > > securely
> > > > > > > for the
> > > > > > > common case of ucontext without enabling WRSS. Or
> > > > > > > potentially
> > > > > > > have the
> > > > > > > kernel set up the shadow stack in some new way.
> > > > > > 
> > > > > > ...
> > > > > > > The following example demonstrates how to create a new
> > > > > > > shadow
> > > > > > > stack with
> > > > > > > map_shadow_stack:
> > > > > > > void *shstk = map_shadow_stack(addr, stack_size,
> > > > > > > SHADOW_STACK_SET_TOKEN);
> > > > > > 
> > > > > > i think
> > > > > > 
> > > > > > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1,
> > > > > > 0);
> > > > > > 
> > > > > > could do the same with less disruption to users (new
> > > > > > syscalls
> > > > > > are harder to deal with than new flags). it would do the
> > > > > > guard page and initial token setup too (there is no flag
> > > > > > for
> > > > > > it but could be squeezed in).
> > > > > 
> > > > > Discussion on this topic in v6
> > > > > 
> > > > 
> > > > 
> > 
> > 
https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
> > > > > 
> > > > > Again I know earlier CET patches had protection flag and
> > > > > somehow
> > > > > due
> > > > > to pushback
> > > > > on mailing list,
> > > > >  it was adopted to go for special syscall because no one else
> > > > > had shadow stack.
> > > > > 
> > > > > Seeing a response from Szabolcs, I am assuming arm4 would
> > > > > also
> > > > > want
> > > > > to follow
> > > > > using mmap to manufacture shadow stack. For reference RFC
> > > > > patches
> > > > > for
> > > > > risc-v shadow stack,
> > > > > use a new protection flag = PROT_SHADOWSTACK.
> > > > > 
> > > > 
> > > > 
> > 
> > 
https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
> > > > > 
> > > > > I know earlier discussion had been that we let this go and do
> > > > > a
> > > > > re-
> > > > > factor later as other
> > > > > arch support trickle in. But as I thought more on this and I
> > > > > think it
> > > > > may just be
> > > > > messy from user mode point of view as well to have cognition
> > > > > of
> > > > > two
> > > > > different ways of
> > > > > creating shadow stack. One would be special syscall (in
> > > > > current
> > > > > libc)
> > > > > and another `mmap`
> > > > > (whenever future re-factor happens)
> > > > > 
> > > > > If it's not too late, it would be more wise to take `mmap`
> > > > > approach rather than special `syscall` approach.
> > > > 
> > > > There is sort of two things intermixed here when we talk about
> > > > a
> > > > PROT_SHADOW_STACK.
> > > > 
> > > > One is: what is the interface for specifying how the shadow
> > > > stack
> > > > should be provisioned with data? Right now there are two ways
> > > > supported, all zero or with an X86 shadow stack restore token
> > > > at
> > > > the
> > > > end. Then there was already some conversation about a third
> > > > type.
> > > > In
> > > > which case the question would be is using mmap MAP_ flags the
> > > > right
> > > > place for this? How many types of initialization will be needed
> > > > in
> > > > the
> > > > end and what is the overlap between the architectures?
> > > 
> > > First of all, arches can choose to have token at the bottom or
> > > not.
> > > 
> > > Token serve following purposes
> > >   - It allows one to put desired value in shadow stack pointer in
> > > safe/secure manner.
> > >     Note: x86 doesn't provide any opcode encoding to value in SSP
> > > register. So having
> > >     a token is kind of a necessity because x86 doesn't easily
> > > allow
> > > writing shadow stack.
> > > 
> > >   - A token at the bottom acts marker / barrier and can be useful
> > > in
> > > debugging
> > > 
> > >   - If (and a big *if*) we ever reach a point in future where
> > > return
> > > address is only pushed
> > >     on shadow stack (x86 should have motivation to do this
> > > because
> > > less uops on call/ret),
> > >     a token at the bottom (bottom means lower address) is
> > > ensuring
> > > sure shot way of getting
> > >     a fault when exhausted.
> > > 
> > > Current RISCV zisslpcfi proposal doesn't define CPU based tokens
> > > because it's RISC.
> > > It allows mechanisms using which software can define formatting
> > > of
> > > token for itself.
> > > Not sure of what ARM is doing.
> > 
> > Ok, so riscv doesn't need to have the kernel write the token, but
> > x86
> > does.
> > 
> > > 
> > > Now coming to the point of all zero v/s shadow stack token.
> > > Why not always have token at the bottom?
> > 
> > With WRSS you can setup the shadow stack however you want. So the
> > user
> > would then have to take care to erase the token if they didn't want
> > it.
> > Not the end of the world, but kind of clunky if there is no reason
> > for
> > it.
> 
> Yes but kernel always assumes the user is going to use the token. It'
> upto the user
> to decide whether they want to use the restore token or not. If
> they've WRSS capability
> security posture is anyways diluted. An attacker who would be clever
> enough to
> re-use `RSTORSSP` present in address space to restore using kernel
> prepared token, should
> anyways can be clever enough to use WRSS as well.
> 
> It kind of makes shadow stack creation simpler for kernel to always
> place the token.
> This point is irrespective of whether to use system call or mmap.

Think about like CRIU restoring the shadow stack, or other special
cases like that. Userspace can always overwrite the token, but this
involves some amount of extra work (extra writes, earlier faulting in
the page, etc). It is clunky and very negligibly worse.

> 
> > 
> > > 
> > > In case of x86, Why need for two ways and why not always have a
> > > token
> > > at the bottom.
> > > The way x86 is going, user mode is responsible for establishing
> > > shadow stack and thus
> > > whenever shadow stack is created then if x86 kernel
> > > implementation
> > > always place a token
> > > at the base/bottom.
> > 
> > There was also some discussion recently of adding a token AND an
> > end of
> > stack marker, as a potential solution for backtracing in ucontext
> > stacks. In this case it could cause an ABI break to just start
> > adding
> > the end of stack marker where the token was, and so would require a
> > new
> > map_shadow_stack flag.
> 
> Was this discussed why restore token itself can't be used as marker
> for
> end of stack (if we assume there is always going to be one at the
> bottom).
> It's a unique value. An address pointing to itself.

I thought the same thing at first, but it gets clobbered during the
pivot and push.

> 
> > 
> > > 
> > > Now user mode can do following:--
> > >   - If it has access to WRSS, it can sure go ahead and create a
> > > token
> > > of its choosing and
> > >     overwrite kernel created token. and then do RSTORSSP on it's
> > > own
> > > created token.
> > > 
> > >   - If it doesn't have access to WRSS (and dont need to create
> > > its
> > > own token), it can do
> > >     RSTORSSP on this. As soon as it does, no other thread in
> > > process
> > > can restore to it.
> > >     On `fork`, you get the same un-restorable token.
> > > 
> > > So why not always have a token at the bottom.
> > > This is my plan for riscv implementation as well (to have a token
> > > at
> > > the bottom)
> > > 
> > > > 
> > > > The other thing is: should shadow stack memory creation be
> > > > tightly
> > > > controlled? For example in x86 we limit this to anonymous
> > > > memory,
> > > > etc.
> > > > Some reasons for this are x86 specific, but some are not. So if
> > > > we
> > > > disallow most of the options why allow the interface to take
> > > > them?
> > > > And
> > > > then you are in the position of carefully maintaining a list of
> > > > not-
> > > > allowed options instead letting a list of allowed options sit
> > > > there.
> > > 
> > > I am new to linux kernel and thus may be not able to follow the
> > > argument of
> > > limiting to anonymous memory.
> > > 
> > > Why is limiting it to anonymous memory a problem. IIRC, ARM's
> > > PROT_MTE is applicable
> > > only to anonymous memory. I can probably find few more examples.
> > 
> > Oh I see, they have a special arch VMA flag VM_MTE_ALLOWED that
> > only
> > gets set if all the rules are followed. Then PROT_MTE can only be
> > set
> > on that to set VM_MTE. That is kind of nice because certain other
> > special situations can choose to support it.
> 
> That's because MTE is different. It allows to assign tags to existing
> virtual memory. So one need to know whether a memory can have tags
> assigned.
> 
> > 
> > It does take another arch vma flag though. For x86 I guess I would
> > need
> > to figure out how to squeeze VM_SHADOW_STACK into other flags to
> > have a
> > free flag to use the same method. It also only supports mprotect()
> > and
> > shadow stack would only want to support mmap(). And you still have
> > the
> > initialization stuff to plumb through. Yea, I think the PROT_MTE is
> > a
> > good thing to consider, but it's not super obvious to me how
> > similar
> > the logic would be for shadow stack.
> 
> I dont think you need another VMA flag. Memory tagging allows adding
> tags
> to existing virtual memory.

...need another VMA flag to use the existing mmap arch breakouts in the
same way as VM_MTE. Of course changing mmap makes other solutions
possible.

>  That's why having `mprotect` makes sense for MTE.
> In shadow stack case, there is no requirement of changing a shadow
> stack
> to regular memory or vice-versa. 

uffd needs mprotect internals. You might take a look at it in regards
to your VM_WRITE/mprotect blocking approach for riscv. I was imagining,
even if mmap was the syscall, mprotect() would not be blocked in the
x86 case at least. The mprotect() blocking is a separate thing than the
syscall, right?

> 
> All that's needed to change is `mmap`. `mprotect` should fail.
> Syscall
> approach gives that benefit by default because there is no protection
> flag
> for shadow stack.
> 
> I was giving example that any feature which gives new meaning to
> virtual memory
> has been able to work with existing memory mapping APIs without the
> need of new
> system call (including whether you're dealing with anonymous memory).
> 
> > 
> > The question I'm asking though is, not "can mmap code and rules be
> > changed to enforce the required limitations?". I think it is yes.
> > But
> > the question is "why is that plumbing better than a new syscall?".
> > I
> > guess to get a better idea, the mmap solution would need to get
> > POCed.
> > I had half done this at one point, but abandoned the approach.
> > 
> > For your question about why limit it, the special x86 case is the
> > Dirty=1,Write=0 PTE bit combination for shadow stacks. So for
> > shadow
> > stack you could have some confusion about whether a PTE is actually
> > dirty for writeback, etc. I wouldn't say it's known to be
> > impossible to
> > do MAP_SHARED, but it has not been fully analyzed enough to know
> > what
> > the changes would be. There were some solvable concrete issues that
> > tipped the scale as well. It was also not expected to be a common
> > usage, if at all.
> 
> I am not sure how confusion of D=1,W=0 is not completely taken away
> by
> syscall approach. It'll always be there. One can only do things to
> minimize
> the chances.
> 
> In case of syscall approach, syscall makes sure that 
> 
> `flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_ABOVE4G`
> 
> This can be easily checked in arch specific landing function for
> mmap.

Right, this is why I listed two types of things in the mix here. The
memory features supported, and what the syscall is. You asked why limit
the memory features, so that is the explanation.

> 
> 
> Additionally, If you always have the token at base, you don't need
> that ABI
> between user and kernel.
> 
> 
> > 
> > The non-x86, general reasons for it, are for a smaller benefit. It
> > blocks a lot of ways shadow stack memory could be written to. Like
> > say
> > you have a memory mapped writable file, and you also map it shadow
> > stack. So it has better security properties depending on what your
> > threat model is.
> 
> I wouldn't say any architecture should allow such primitives. It kind
> of defeats
> the purpose for shadow stack. Yes if some sort of secure memory is
> needed, there may
> be new ISA extensions for that.

Yea, seems reasonable to prevent this regardless of the extra x86
reasons, if that is what you are saying. It depends on people's threat
models (as always in security).

> 
> > 
> > > 
> > > Eventually syscall will also go ahead and use memory management
> > > code
> > > to
> > > perform mapping. So I didn't understand the reasoning here. The
> > > way
> > > syscall
> > > can limit it to anonymous memory, why mmap can't do the same if
> > > it
> > > sees
> > > PROT_SHADOWSTACK.
> > > 
> > > > 
> > > > The only benefit I've heard is that it saves creating a new
> > > > syscall,
> > > > but it also saves several MAP_ flags. That, and that the RFC
> > > > for
> > > > riscv
> > > > did a PROT_SHADOW_STACK to start. So, yes, two people asked the
> > > > same
> > > > question, but I'm still not seeing any benefits. Can you give
> > > > the
> > > > pros
> > > > and cons please?
> > > 
> > > Again the way syscall will limit it to anonymous memory, Why mmap
> > > can't do same?
> > > There is precedence for it (like PROT_MTE is applicable only to
> > > anonymous memory)
> > > 
> > > So if it can be done, then why introduce a new syscall?
> > > 
> > > > 
> > > > BTW, in glibc map_shadow_stack is called from arch code. So I
> > > > think
> > > > userspace wise, for this to affect other architectures there
> > > > would
> > > > need
> > > > to be some code that could do things generically, with somehow
> > > > the
> > > > shadow stack pivot abstracted but the shadow stack allocation
> > > > not.
> > > 
> > > Agreed, yes it can be done in a way where it won't put tax on
> > > other
> > > architectures.
> > > 
> > > But what about fragmentation within x86. Will x86 always choose
> > > to
> > > use system call
> > > method map shadow stack. If future re-factor results in x86 also
> > > use
> > > `mmap` method.
> > > Isn't it a mess for x86 glibc to figure out what to do; whether
> > > to
> > > use system call
> > > or `mmap`?
> > > 
> > 
> > Ok, so this is the downside I guess. What happens if we want to
> > support
> > the other types of memory in the future and end up using mmap for
> > this?
> > Then we have 15-20 lines of extra syscall wrapping code to maintain
> > to
> > support legacy.
> > 
> > For the mmap solution, we have the downside of using extra MAP_
> > flags,
> > and *some* amount of currently unknown vm_flag and address range
> > logic,
> > plus mmap arch breakouts to add to core MM. Like I said earlier,
> > you
> > would need to POC it out to see how bad that looks and get some
> > core MM
> > feedback on the new type of MAP flag usage. But, syscalls being
> > pretty
> > straightforward, it would probably be *some* amount of added
> > complexity
> > _now_ to support something that might happen in the future. I'm not
> > seeing either one as a landslide win.
> > 
> > It's kind of an eternal software design philosophical question,
> > isn't
> > it? How much work should you do to prepare for things that might be
> > needed in the future? From what I've seen the balance in the kernel
> > seems to be to try not to paint yourself in to an ABI corner, but
> > otherwise let the kernel evolve naturally in response to real
> > usages.
> > If anyone wants to correct this, please do. But otherwise I think
> > the
> > new syscall is aligned with that.
> > 
> > TBH, you are making me wonder if I'm missing something. It seems
> > you
> > strongly don't prefer this approach, but I'm not hearing any huge
> > potential negative impacts. And you also say it won't tax the riscv
> > implementation. Is this just something just smells bad here? Or it
> > would shrink the riscv series?
> 
> No you're not missing anything. It's just wierdness of adding a
> system call
> which enforces certain MAP_XX flags and pretty much mapping API.
> And difference between architectures on how they will create shadow
> stack. + 
> if x86 chooses to use `mmap` in future, then there is ugliness in
> user mode to
> decide which method to choose.

Ok, I think I will leave it given it's entirely in arch/x86. It just
got some special error codes in the other thread today too.

> 
> And yes you got it right, to some extent there is my own selfishness
> playing out
> as well here to reduce riscv patches.
> 

Feel free to join the map_shadow_stack party. :)
  

Hi,

On Thu, Mar 09, 2023 at 10:55:11AM -0800, Deepak Gupta wrote:
> On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy wrote:
> > The 02/27/2023 14:29, Rick Edgecombe wrote:
> > > Previously, a new PROT_SHADOW_STACK was attempted,
> > ...
> > > So rather than repurpose two existing syscalls (mmap, madvise) that don't
> > > quite fit, just implement a new map_shadow_stack syscall to allow
> > > userspace to map and setup new shadow stacks in one step. While ucontext
> > > is the primary motivator, userspace may have other unforeseen reasons to
> > > setup it's own shadow stacks using the WRSS instruction. Towards this
> > > provide a flag so that stacks can be optionally setup securely for the
> > > common case of ucontext without enabling WRSS. Or potentially have the
> > > kernel set up the shadow stack in some new way.
> > ...
> > > The following example demonstrates how to create a new shadow stack with
> > > map_shadow_stack:
> > > void *shstk = map_shadow_stack(addr, stack_size, SHADOW_STACK_SET_TOKEN);
> > 
> > i think
> > 
> > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
> > 
> > could do the same with less disruption to users (new syscalls
> > are harder to deal with than new flags). it would do the
> > guard page and initial token setup too (there is no flag for
> > it but could be squeezed in).
> 
> Discussion on this topic in v6
> https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
> 
> Again I know earlier CET patches had protection flag and somehow due to pushback
> on mailing list, it was adopted to go for special syscall because no one else
> had shadow stack.
> 
> Seeing a response from Szabolcs, I am assuming arm4 would also want to follow
> using mmap to manufacture shadow stack. For reference RFC patches for risc-v shadow stack,
> use a new protection flag = PROT_SHADOWSTACK.
> https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
> 
> I know earlier discussion had been that we let this go and do a re-factor later as other
> arch support trickle in. But as I thought more on this and I think it may just be
> messy from user mode point of view as well to have cognition of two different ways of
> creating shadow stack. One would be special syscall (in current libc) and another `mmap`
> (whenever future re-factor happens)
> 
> If it's not too late, it would be more wise to take `mmap`
> approach rather than special `syscall` approach.
 
I disagree. 

Having shadow stack flags for mmap() adds unnecessary complexity to the
core-mm, while having a dedicated syscall hides all the details in the
architecture specific code.

Another reason to use a dedicated system call allows for better
extensibility if/when we'd need to update the way shadow stack VMA is
created.

As for the userspace convenience, it is anyway required to add special
code for creating the shadow stack and it wouldn't matter if that code
would use mmap(NEW_FLAG) or map_shadow_stack().

> > most of the mmap features need not be available (EINVAL) when
> > MAP_SHADOW_STACK is specified.
> > 
> > the main drawback is running out of mmap flags so extension
> > is limited. (but the new syscall has limitations too).
  

On Tue, Mar 14, 2023 at 12:19 AM Mike Rapoport <rppt@kernel.org> wrote:
>
> Hi,
>
> On Thu, Mar 09, 2023 at 10:55:11AM -0800, Deepak Gupta wrote:
> > On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy wrote:
> > > The 02/27/2023 14:29, Rick Edgecombe wrote:
> > > > Previously, a new PROT_SHADOW_STACK was attempted,
> > > ...
> > > > So rather than repurpose two existing syscalls (mmap, madvise) that don't
> > > > quite fit, just implement a new map_shadow_stack syscall to allow
> > > > userspace to map and setup new shadow stacks in one step. While ucontext
> > > > is the primary motivator, userspace may have other unforeseen reasons to
> > > > setup it's own shadow stacks using the WRSS instruction. Towards this
> > > > provide a flag so that stacks can be optionally setup securely for the
> > > > common case of ucontext without enabling WRSS. Or potentially have the
> > > > kernel set up the shadow stack in some new way.
> > > ...
> > > > The following example demonstrates how to create a new shadow stack with
> > > > map_shadow_stack:
> > > > void *shstk = map_shadow_stack(addr, stack_size, SHADOW_STACK_SET_TOKEN);
> > >
> > > i think
> > >
> > > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1, 0);
> > >
> > > could do the same with less disruption to users (new syscalls
> > > are harder to deal with than new flags). it would do the
> > > guard page and initial token setup too (there is no flag for
> > > it but could be squeezed in).
> >
> > Discussion on this topic in v6
> > https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
> >
> > Again I know earlier CET patches had protection flag and somehow due to pushback
> > on mailing list, it was adopted to go for special syscall because no one else
> > had shadow stack.
> >
> > Seeing a response from Szabolcs, I am assuming arm4 would also want to follow
> > using mmap to manufacture shadow stack. For reference RFC patches for risc-v shadow stack,
> > use a new protection flag = PROT_SHADOWSTACK.
> > https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
> >
> > I know earlier discussion had been that we let this go and do a re-factor later as other
> > arch support trickle in. But as I thought more on this and I think it may just be
> > messy from user mode point of view as well to have cognition of two different ways of
> > creating shadow stack. One would be special syscall (in current libc) and another `mmap`
> > (whenever future re-factor happens)
> >
> > If it's not too late, it would be more wise to take `mmap`
> > approach rather than special `syscall` approach.
>
> I disagree.
>
> Having shadow stack flags for mmap() adds unnecessary complexity to the
> core-mm, while having a dedicated syscall hides all the details in the
> architecture specific code.

Again reiterating it would've made sense if only x86 had a shadow stack.
aarch64 announced support for guarded stack. risc-v spec is in
development to support shadow stack.

So there will be shadow stack related flow in these arches.

>
> Another reason to use a dedicated system call allows for better
> extensibility if/when we'd need to update the way shadow stack VMA is
> created.

I see two valid points here
    - Shadow stack doesn't need conversion into different memory types
(which is usually the case for address ranges created by mmap)
      So there is a static page permissions on shadow stack which is
not mutable.

    - Future feature addition (if there is one needed) at the time of
shadow stack creation
      It would avoid future tax on mmap

I'll think more about this.

>
> As for the userspace convenience, it is anyway required to add special
> code for creating the shadow stack and it wouldn't matter if that code
> would use mmap(NEW_FLAG) or map_shadow_stack().

Yes *strictly* from userspace convenience, it doesn't matter which option.

>
> > > most of the mmap features need not be available (EINVAL) when
> > > MAP_SHADOW_STACK is specified.
> > >
> > > the main drawback is running out of mmap flags so extension
> > > is limited. (but the new syscall has limitations too).
>
> --
> Sincerely yours,
> Mike.
  

On Fri, Mar 10, 2023 at 1:43 PM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Fri, 2023-03-10 at 13:00 -0800, Deepak Gupta wrote:
> > On Fri, Mar 10, 2023 at 12:14:01AM +0000, Edgecombe, Rick P wrote:
> > > On Thu, 2023-03-09 at 13:08 -0800, Deepak Gupta wrote:
> > > > On Thu, Mar 09, 2023 at 07:39:41PM +0000, Edgecombe, Rick P
> > > > wrote:
> > > > > On Thu, 2023-03-09 at 10:55 -0800, Deepak Gupta wrote:
> > > > > > On Thu, Mar 02, 2023 at 05:22:07PM +0000, Szabolcs Nagy
> > > > > > wrote:
> > > > > > > The 02/27/2023 14:29, Rick Edgecombe wrote:
> > > > > > > > Previously, a new PROT_SHADOW_STACK was attempted,
> > > > > > >
> > > > > > > ...
> > > > > > > > So rather than repurpose two existing syscalls (mmap,
> > > > > > > > madvise)
> > > > > > > > that don't
> > > > > > > > quite fit, just implement a new map_shadow_stack syscall
> > > > > > > > to
> > > > > > > > allow
> > > > > > > > userspace to map and setup new shadow stacks in one step.
> > > > > > > > While
> > > > > > > > ucontext
> > > > > > > > is the primary motivator, userspace may have other
> > > > > > > > unforeseen
> > > > > > > > reasons to
> > > > > > > > setup it's own shadow stacks using the WRSS instruction.
> > > > > > > > Towards
> > > > > > > > this
> > > > > > > > provide a flag so that stacks can be optionally setup
> > > > > > > > securely
> > > > > > > > for the
> > > > > > > > common case of ucontext without enabling WRSS. Or
> > > > > > > > potentially
> > > > > > > > have the
> > > > > > > > kernel set up the shadow stack in some new way.
> > > > > > >
> > > > > > > ...
> > > > > > > > The following example demonstrates how to create a new
> > > > > > > > shadow
> > > > > > > > stack with
> > > > > > > > map_shadow_stack:
> > > > > > > > void *shstk = map_shadow_stack(addr, stack_size,
> > > > > > > > SHADOW_STACK_SET_TOKEN);
> > > > > > >
> > > > > > > i think
> > > > > > >
> > > > > > > mmap(addr, size, PROT_READ, MAP_ANON|MAP_SHADOW_STACK, -1,
> > > > > > > 0);
> > > > > > >
> > > > > > > could do the same with less disruption to users (new
> > > > > > > syscalls
> > > > > > > are harder to deal with than new flags). it would do the
> > > > > > > guard page and initial token setup too (there is no flag
> > > > > > > for
> > > > > > > it but could be squeezed in).
> > > > > >
> > > > > > Discussion on this topic in v6
> > > > > >
> > > > >
> > > > >
> > >
> > >
> https://lore.kernel.org/all/20230223000340.GB945966@debug.ba.rivosinc.com/
> > > > > >
> > > > > > Again I know earlier CET patches had protection flag and
> > > > > > somehow
> > > > > > due
> > > > > > to pushback
> > > > > > on mailing list,
> > > > > >  it was adopted to go for special syscall because no one else
> > > > > > had shadow stack.
> > > > > >
> > > > > > Seeing a response from Szabolcs, I am assuming arm4 would
> > > > > > also
> > > > > > want
> > > > > > to follow
> > > > > > using mmap to manufacture shadow stack. For reference RFC
> > > > > > patches
> > > > > > for
> > > > > > risc-v shadow stack,
> > > > > > use a new protection flag = PROT_SHADOWSTACK.
> > > > > >
> > > > >
> > > > >
> > >
> > >
> https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
> > > > > >
> > > > > > I know earlier discussion had been that we let this go and do
> > > > > > a
> > > > > > re-
> > > > > > factor later as other
> > > > > > arch support trickle in. But as I thought more on this and I
> > > > > > think it
> > > > > > may just be
> > > > > > messy from user mode point of view as well to have cognition
> > > > > > of
> > > > > > two
> > > > > > different ways of
> > > > > > creating shadow stack. One would be special syscall (in
> > > > > > current
> > > > > > libc)
> > > > > > and another `mmap`
> > > > > > (whenever future re-factor happens)
> > > > > >
> > > > > > If it's not too late, it would be more wise to take `mmap`
> > > > > > approach rather than special `syscall` approach.
> > > > >
> > > > > There is sort of two things intermixed here when we talk about
> > > > > a
> > > > > PROT_SHADOW_STACK.
> > > > >
> > > > > One is: what is the interface for specifying how the shadow
> > > > > stack
> > > > > should be provisioned with data? Right now there are two ways
> > > > > supported, all zero or with an X86 shadow stack restore token
> > > > > at
> > > > > the
> > > > > end. Then there was already some conversation about a third
> > > > > type.
> > > > > In
> > > > > which case the question would be is using mmap MAP_ flags the
> > > > > right
> > > > > place for this? How many types of initialization will be needed
> > > > > in
> > > > > the
> > > > > end and what is the overlap between the architectures?
> > > >
> > > > First of all, arches can choose to have token at the bottom or
> > > > not.
> > > >
> > > > Token serve following purposes
> > > >   - It allows one to put desired value in shadow stack pointer in
> > > > safe/secure manner.
> > > >     Note: x86 doesn't provide any opcode encoding to value in SSP
> > > > register. So having
> > > >     a token is kind of a necessity because x86 doesn't easily
> > > > allow
> > > > writing shadow stack.
> > > >
> > > >   - A token at the bottom acts marker / barrier and can be useful
> > > > in
> > > > debugging
> > > >
> > > >   - If (and a big *if*) we ever reach a point in future where
> > > > return
> > > > address is only pushed
> > > >     on shadow stack (x86 should have motivation to do this
> > > > because
> > > > less uops on call/ret),
> > > >     a token at the bottom (bottom means lower address) is
> > > > ensuring
> > > > sure shot way of getting
> > > >     a fault when exhausted.
> > > >
> > > > Current RISCV zisslpcfi proposal doesn't define CPU based tokens
> > > > because it's RISC.
> > > > It allows mechanisms using which software can define formatting
> > > > of
> > > > token for itself.
> > > > Not sure of what ARM is doing.
> > >
> > > Ok, so riscv doesn't need to have the kernel write the token, but
> > > x86
> > > does.
> > >
> > > >
> > > > Now coming to the point of all zero v/s shadow stack token.
> > > > Why not always have token at the bottom?
> > >
> > > With WRSS you can setup the shadow stack however you want. So the
> > > user
> > > would then have to take care to erase the token if they didn't want
> > > it.
> > > Not the end of the world, but kind of clunky if there is no reason
> > > for
> > > it.
> >
> > Yes but kernel always assumes the user is going to use the token. It'
> > upto the user
> > to decide whether they want to use the restore token or not. If
> > they've WRSS capability
> > security posture is anyways diluted. An attacker who would be clever
> > enough to
> > re-use `RSTORSSP` present in address space to restore using kernel
> > prepared token, should
> > anyways can be clever enough to use WRSS as well.
> >
> > It kind of makes shadow stack creation simpler for kernel to always
> > place the token.
> > This point is irrespective of whether to use system call or mmap.
>
> Think about like CRIU restoring the shadow stack, or other special
> cases like that. Userspace can always overwrite the token, but this
> involves some amount of extra work (extra writes, earlier faulting in
> the page, etc). It is clunky and very negligibly worse.

Earlier faulting in the page because the kernel is writing the token at base?

>
> >
> > >
> > > >
> > > > In case of x86, Why need for two ways and why not always have a
> > > > token
> > > > at the bottom.
> > > > The way x86 is going, user mode is responsible for establishing
> > > > shadow stack and thus
> > > > whenever shadow stack is created then if x86 kernel
> > > > implementation
> > > > always place a token
> > > > at the base/bottom.
> > >
> > > There was also some discussion recently of adding a token AND an
> > > end of
> > > stack marker, as a potential solution for backtracing in ucontext
> > > stacks. In this case it could cause an ABI break to just start
> > > adding
> > > the end of stack marker where the token was, and so would require a
> > > new
> > > map_shadow_stack flag.
> >
> > Was this discussed why restore token itself can't be used as marker
> > for
> > end of stack (if we assume there is always going to be one at the
> > bottom).
> > It's a unique value. An address pointing to itself.
>
> I thought the same thing at first, but it gets clobbered during the
> pivot and push.

aah I remember. It was changed from savessp/rstorssp pair to
rstorssp/saveprevssp to follow the `make before break` model.

>
> >
> > >
> > > >
> > > > Now user mode can do following:--
> > > >   - If it has access to WRSS, it can sure go ahead and create a
> > > > token
> > > > of its choosing and
> > > >     overwrite kernel created token. and then do RSTORSSP on it's
> > > > own
> > > > created token.
> > > >
> > > >   - If it doesn't have access to WRSS (and dont need to create
> > > > its
> > > > own token), it can do
> > > >     RSTORSSP on this. As soon as it does, no other thread in
> > > > process
> > > > can restore to it.
> > > >     On `fork`, you get the same un-restorable token.
> > > >
> > > > So why not always have a token at the bottom.
> > > > This is my plan for riscv implementation as well (to have a token
> > > > at
> > > > the bottom)
> > > >
> > > > >
> > > > > The other thing is: should shadow stack memory creation be
> > > > > tightly
> > > > > controlled? For example in x86 we limit this to anonymous
> > > > > memory,
> > > > > etc.
> > > > > Some reasons for this are x86 specific, but some are not. So if
> > > > > we
> > > > > disallow most of the options why allow the interface to take
> > > > > them?
> > > > > And
> > > > > then you are in the position of carefully maintaining a list of
> > > > > not-
> > > > > allowed options instead letting a list of allowed options sit
> > > > > there.
> > > >
> > > > I am new to linux kernel and thus may be not able to follow the
> > > > argument of
> > > > limiting to anonymous memory.
> > > >
> > > > Why is limiting it to anonymous memory a problem. IIRC, ARM's
> > > > PROT_MTE is applicable
> > > > only to anonymous memory. I can probably find few more examples.
> > >
> > > Oh I see, they have a special arch VMA flag VM_MTE_ALLOWED that
> > > only
> > > gets set if all the rules are followed. Then PROT_MTE can only be
> > > set
> > > on that to set VM_MTE. That is kind of nice because certain other
> > > special situations can choose to support it.
> >
> > That's because MTE is different. It allows to assign tags to existing
> > virtual memory. So one need to know whether a memory can have tags
> > assigned.
> >
> > >
> > > It does take another arch vma flag though. For x86 I guess I would
> > > need
> > > to figure out how to squeeze VM_SHADOW_STACK into other flags to
> > > have a
> > > free flag to use the same method. It also only supports mprotect()
> > > and
> > > shadow stack would only want to support mmap(). And you still have
> > > the
> > > initialization stuff to plumb through. Yea, I think the PROT_MTE is
> > > a
> > > good thing to consider, but it's not super obvious to me how
> > > similar
> > > the logic would be for shadow stack.
> >
> > I dont think you need another VMA flag. Memory tagging allows adding
> > tags
> > to existing virtual memory.
>
> ...need another VMA flag to use the existing mmap arch breakouts in the
> same way as VM_MTE. Of course changing mmap makes other solutions
> possible.
>
> >  That's why having `mprotect` makes sense for MTE.
> > In shadow stack case, there is no requirement of changing a shadow
> > stack
> > to regular memory or vice-versa.
>
> uffd needs mprotect internals. You might take a look at it in regards
> to your VM_WRITE/mprotect blocking approach for riscv. I was imagining,
> even if mmap was the syscall, mprotect() would not be blocked in the
> x86 case at least. The mprotect() blocking is a separate thing than the
> syscall, right?

Yes, mprotect blocking is a different thing.
VM_XXX flags are not exposed to mprotect (or any memory mapping API).
PROT_XXX flags are. On riscv, in my current plan if mprotect or mmap
specifies PROT_WRITE (no PROT_READ),
It'll be mapped to `VM_READ | VM_WRITE` on vma flags (this is to make
sure we don't break compat with existing user code which has
been using only PROT_WRITE)

If PROT_SHADOWSTACK (new protection flag) is specified, it'll be
mapped to `VM_WRITE` on the vma_flag.

Yes I am aware of uffd. I intend to handle it the same way I am
handling the fork of riscv shadow stack.
core-mm write protect checks if VM_WRiTE is specified it'll convert
PTE encodings to read-only.
uffd for regular memory should work as it is. In case if someone was
monitoring shadow stack memory, following could occur

1) A write happens on shadow stack memory, a store page fault would occur.
2) A read happens, this would be allowed.
3) A shadow stack load / store happens, a store access fault would occur.

Case 1 and 3 are reported to the kernel and it can make sure the uffd
monitor is reported about it.

Was there a specific concern here with respect to uffd and x86 shadow stack?

>
> >
> > All that's needed to change is `mmap`. `mprotect` should fail.
> > Syscall
> > approach gives that benefit by default because there is no protection
> > flag
> > for shadow stack.
> >
> > I was giving example that any feature which gives new meaning to
> > virtual memory
> > has been able to work with existing memory mapping APIs without the
> > need of new
> > system call (including whether you're dealing with anonymous memory).
> >
> > >
> > > The question I'm asking though is, not "can mmap code and rules be
> > > changed to enforce the required limitations?". I think it is yes.
> > > But
> > > the question is "why is that plumbing better than a new syscall?".
> > > I
> > > guess to get a better idea, the mmap solution would need to get
> > > POCed.
> > > I had half done this at one point, but abandoned the approach.
> > >
> > > For your question about why limit it, the special x86 case is the
> > > Dirty=1,Write=0 PTE bit combination for shadow stacks. So for
> > > shadow
> > > stack you could have some confusion about whether a PTE is actually
> > > dirty for writeback, etc. I wouldn't say it's known to be
> > > impossible to
> > > do MAP_SHARED, but it has not been fully analyzed enough to know
> > > what
> > > the changes would be. There were some solvable concrete issues that
> > > tipped the scale as well. It was also not expected to be a common
> > > usage, if at all.
> >
> > I am not sure how confusion of D=1,W=0 is not completely taken away
> > by
> > syscall approach. It'll always be there. One can only do things to
> > minimize
> > the chances.
> >
> > In case of syscall approach, syscall makes sure that
> >
> > `flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_ABOVE4G`
> >
> > This can be easily checked in arch specific landing function for
> > mmap.
>
> Right, this is why I listed two types of things in the mix here. The
> memory features supported, and what the syscall is. You asked why limit
> the memory features, so that is the explanation.
>
> >
> >
> > Additionally, If you always have the token at base, you don't need
> > that ABI
> > between user and kernel.
> >
> >
> > >
> > > The non-x86, general reasons for it, are for a smaller benefit. It
> > > blocks a lot of ways shadow stack memory could be written to. Like
> > > say
> > > you have a memory mapped writable file, and you also map it shadow
> > > stack. So it has better security properties depending on what your
> > > threat model is.
> >
> > I wouldn't say any architecture should allow such primitives. It kind
> > of defeats
> > the purpose for shadow stack. Yes if some sort of secure memory is
> > needed, there may
> > be new ISA extensions for that.
>
> Yea, seems reasonable to prevent this regardless of the extra x86
> reasons, if that is what you are saying. It depends on people's threat
> models (as always in security).
>
> >
> > >
> > > >
> > > > Eventually syscall will also go ahead and use memory management
> > > > code
> > > > to
> > > > perform mapping. So I didn't understand the reasoning here. The
> > > > way
> > > > syscall
> > > > can limit it to anonymous memory, why mmap can't do the same if
> > > > it
> > > > sees
> > > > PROT_SHADOWSTACK.
> > > >
> > > > >
> > > > > The only benefit I've heard is that it saves creating a new
> > > > > syscall,
> > > > > but it also saves several MAP_ flags. That, and that the RFC
> > > > > for
> > > > > riscv
> > > > > did a PROT_SHADOW_STACK to start. So, yes, two people asked the
> > > > > same
> > > > > question, but I'm still not seeing any benefits. Can you give
> > > > > the
> > > > > pros
> > > > > and cons please?
> > > >
> > > > Again the way syscall will limit it to anonymous memory, Why mmap
> > > > can't do same?
> > > > There is precedence for it (like PROT_MTE is applicable only to
> > > > anonymous memory)
> > > >
> > > > So if it can be done, then why introduce a new syscall?
> > > >
> > > > >
> > > > > BTW, in glibc map_shadow_stack is called from arch code. So I
> > > > > think
> > > > > userspace wise, for this to affect other architectures there
> > > > > would
> > > > > need
> > > > > to be some code that could do things generically, with somehow
> > > > > the
> > > > > shadow stack pivot abstracted but the shadow stack allocation
> > > > > not.
> > > >
> > > > Agreed, yes it can be done in a way where it won't put tax on
> > > > other
> > > > architectures.
> > > >
> > > > But what about fragmentation within x86. Will x86 always choose
> > > > to
> > > > use system call
> > > > method map shadow stack. If future re-factor results in x86 also
> > > > use
> > > > `mmap` method.
> > > > Isn't it a mess for x86 glibc to figure out what to do; whether
> > > > to
> > > > use system call
> > > > or `mmap`?
> > > >
> > >
> > > Ok, so this is the downside I guess. What happens if we want to
> > > support
> > > the other types of memory in the future and end up using mmap for
> > > this?
> > > Then we have 15-20 lines of extra syscall wrapping code to maintain
> > > to
> > > support legacy.
> > >
> > > For the mmap solution, we have the downside of using extra MAP_
> > > flags,
> > > and *some* amount of currently unknown vm_flag and address range
> > > logic,
> > > plus mmap arch breakouts to add to core MM. Like I said earlier,
> > > you
> > > would need to POC it out to see how bad that looks and get some
> > > core MM
> > > feedback on the new type of MAP flag usage. But, syscalls being
> > > pretty
> > > straightforward, it would probably be *some* amount of added
> > > complexity
> > > _now_ to support something that might happen in the future. I'm not
> > > seeing either one as a landslide win.
> > >
> > > It's kind of an eternal software design philosophical question,
> > > isn't
> > > it? How much work should you do to prepare for things that might be
> > > needed in the future? From what I've seen the balance in the kernel
> > > seems to be to try not to paint yourself in to an ABI corner, but
> > > otherwise let the kernel evolve naturally in response to real
> > > usages.
> > > If anyone wants to correct this, please do. But otherwise I think
> > > the
> > > new syscall is aligned with that.
> > >
> > > TBH, you are making me wonder if I'm missing something. It seems
> > > you
> > > strongly don't prefer this approach, but I'm not hearing any huge
> > > potential negative impacts. And you also say it won't tax the riscv
> > > implementation. Is this just something just smells bad here? Or it
> > > would shrink the riscv series?
> >
> > No you're not missing anything. It's just wierdness of adding a
> > system call
> > which enforces certain MAP_XX flags and pretty much mapping API.
> > And difference between architectures on how they will create shadow
> > stack. +
> > if x86 chooses to use `mmap` in future, then there is ugliness in
> > user mode to
> > decide which method to choose.
>
> Ok, I think I will leave it given it's entirely in arch/x86. It just
> got some special error codes in the other thread today too.
>
> >
> > And yes you got it right, to some extent there is my own selfishness
> > playing out
> > as well here to reduce riscv patches.
> >
>
> Feel free to join the map_shadow_stack party. :)


I am warming up to it :-)
  

The 03/16/2023 12:30, Deepak Gupta wrote:
> On Tue, Mar 14, 2023 at 12:19 AM Mike Rapoport <rppt@kernel.org> wrote:
> > As for the userspace convenience, it is anyway required to add special
> > code for creating the shadow stack and it wouldn't matter if that code
> > would use mmap(NEW_FLAG) or map_shadow_stack().
> 
> Yes *strictly* from userspace convenience, it doesn't matter which option.

everybody seems to assume that the new syscall only matters for
the code allocating the shadow stack.

there are tools like strace, seccomp,.. that need to learn
about the new syscall and anything that's built on top of them
as well as libc api interposers like address sanitizer need to
learn about the related new libc apis (if there are any.. which
will be another long debate on the userspace side, delaying the
usability of shadow stacks even more). such tools already know
about mmap and often can handle new flags without much change.

i agree that too much special logic in mmap is not ideal and
using an mmap flag limits future extensions of both mmap and
shadow map functionality. but i disagree that a new syscall is
generally easy for userspace to deal with. in this case the
cost seems acceptable to me, but it's not free at all.