Re: [PATCH v11 25/25] x86/cet/shstk: Add arch_prctl functions for shadow stack

Yu, Yu-cheng
2020-09-09T22:09:00Z

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