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

Borislav Petkov
2023-03-10T16:16:48Z

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);
> +}

--
Regards/Gruss,
Boris.