Messages in this thread | | | Subject | Re: [PATCH v11 25/25] x86/cet/shstk: Add arch_prctl functions for shadow stack | From | "Yu, Yu-cheng" <> | Date | Wed, 9 Sep 2020 15:08:45 -0700 |
| |
On 9/8/2020 11:25 AM, Yu, Yu-cheng wrote: > On 9/8/2020 10:57 AM, Dave Hansen wrote: >> On 9/8/20 10:50 AM, Yu, Yu-cheng wrote: >>> What about this: >>> >>> - Do not add any new syscall or arch_prctl for creating a new shadow >>> stack. >>> >>> - Add a new arch_prctl that can turn an anonymous mapping to a shadow >>> stack mapping. >>> >>> This allows the application to do whatever is necessary. It can even >>> allow GDB or JIT code to create or fix a call stack. >> >> Fine with me. But, it's going to effectively be >> >> arch_prctl(PR_CONVERT_TO_SHS..., addr, len); >> >> when it could just as easily be: >> >> madvise(addr, len, MADV_SHSTK...); >> >> Or a new syscall. The only question in my mind is whether we want to do >> something generic that we can use for other similar things in the >> future, like: >> >> madvise2(addr, len, flags, MADV2_SHSTK...); >> >> I don't really feel strongly about it, though. Could you please share >> your logic on why you want a prctl() as opposed to a whole new syscall? >> > > A new syscall is more intrusive, I think. When creating a new shadow > stack, the kernel also installs a restore token on the top of the new > shadow stack, and it is somewhat x86-specific. So far no other arch's > need this. > > Yes, madvise is better if the kernel only needs to change the mapping. > The application itself can create the restore token before calling > madvise().
After looking at this more, I found the changes are more similar to mprotect() than madvise(). We are going to change an anonymous mapping to a read-only mapping, and add the VM_SHSTK flag to it. Would an x86-specific mprotect(PROT_SHSTK) make more sense?
One alternative would be requiring a read-only mapping for madvise(MADV_SHSTK). But that is inconvenient for the application.
Yu-cheng
| |