Re: [PATCH v20 25/25] mm: Introduce PROT_SHSTK for shadow stack

[Kees Cook]

On Wed, Feb 10, 2021 at 09:57:03AM -0800, Yu-cheng Yu wrote:
> There are three possible options to create a shadow stack allocation API:
> an arch_prctl, a new syscall, or adding PROT_SHSTK to mmap()/mprotect().
> Each has its advantages and compromises.
> 
> An arch_prctl() is the least intrusive.  However, the existing x86
> arch_prctl() takes only two parameters.  Multiple parameters must be
> passed in a memory buffer.  There is a proposal to pass more parameters in
> registers [1], but no active discussion on that.
> 
> A new syscall minimizes compatibility issues and offers an extensible frame
> work to other architectures, but this will likely result in some overlap of
> mmap()/mprotect().
> 
> The introduction of PROT_SHSTK to mmap()/mprotect() takes advantage of
> existing APIs.  The x86-specific PROT_SHSTK is translated to VM_SHSTK and
> a shadow stack mapping is created without reinventing the wheel.  There are
> potential pitfalls though.  The most obvious one would be using this as a
> bypass to shadow stack protection.  However, the attacker would have to get
> to the syscall first.
> 
> Since arch_calc_vm_prot_bits() is modified, I have moved arch_vm_get_page
> _prot() and arch_calc_vm_prot_bits() to x86/include/asm/mman.h.
> This will be more consistent with other architectures.

This portion of the patch seems logically separate from the PROT_SHSTK
implementation. Can you please separate it into its own patch?

> 
> [1] https://lore.kernel.org/lkml/20200828121624.108243-1-hjl.tools@xxxxxxxxx/
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>

With that done:

Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>

-- 
Kees Cook