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
