Re: [PING^3][PATCH,v2,1/1,AARCH64][PR102768] aarch64: Add compiler support for Shadow Call Stack

[Dan Li <ashimida@linux.alibaba.com>]

Thanks, Ard,

On 1/26/22 00:10, Ard Biesheuvel wrote:
> On Wed, 26 Jan 2022 at 08:53, Dan Li <ashimida@linux.alibaba.com> wrote:
>>
>> Hi, all,
>>
>> Sorry for bothering.
>>
>> I'm trying to commit aarch64 scs code to the gcc and there is an issue
>> that I'm not sure about, could someone give me some suggestions?
>> (To avoid noise, I did't cc PING^3 [1] to the kernel mail list :) )
>>
>> When clang enables scs, the following instructions are usually generated:
>>
>> str     x30, [x18], 8
>> ldp     x29, x30, [sp], 16
>> ......
>> ldp     x29, x30, [sp], 16              ## x30 pop
>> ldr     x30, [x18, -8]!                 ## x30 pop again
>> ret
>>
>> The x30 register is popped twice here, Richard suggested that we can
>> omit the first x30 pop here.
>>
>> AFAICT, it seems fine and also safe for SCS. But I'm not sure if I'm
>> missing something with the kernel, could someone give some suggestions?
>>
>> The previous discussion can be found here [1].
>>
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-January/589257.html
>>
> 
> As was pointed out in the discussion, binary patching is in fact a
> concern for the Linux kernel. E.g., Android uses generic binary
> builds, but we would like to be able to switch between pointer
> authentication and shadow call stack at boot time, rather than always
> support both, and take the SCS performance hit on systems that
> implement PAC as well.
> 
> However, it seems more straight-forward to patch PACIASP and AUTIASP
> instructions into SCS push/pop instructions rather than the other way
> around, as we can force the use of these exact opcodes [in the NOP
> space]), as well as rely on existing unwind annotations to locate any
> such instruction in the binary.
> 

Well, then I think I don't need to submit a kernel patch to
enable SCS for gcc :)

BTW:
Do we have a plan to submit patches of dynamic patch PAC into
the kernel recently?

> So omitting the load of X30 from the ordinary stack seems fine to me.
> 
>> On 1/25/22 22:51, Dan Li wrote:
>>>
>>>
>>> On 1/25/22 02:19, Richard Sandiford wrote:
>>>
>>> Well, probably sticking to pop x30 twice is not a good idea.
>>> AFAICT, there doesn't seem to be an explicit requirement that
>
>>>>
>>>
>>> Ok, I'll cc some kernel folks to make sure I didn't miss something.
>>>
To Richard:

Sorry for my mistake.

Due to binary compatibility issues, SCS related code may not
be directly merged into libgcc/glibc, do we still need to
add this patch into GCC? (I'd like to finish it if that
makes sense).


Thanks all for your time!
Dan

>>>> If binary patching is supposed to be possible then scs_push and
>>>> scs_pop *do* need to be separate define_insns.  But they also need
>>>> to have some magic unspec that differentiates them from normal
>>>> pushes and pops, e.g.:
>>>>
>>>>     (set ...mem...
>>>>          (unspec:DI [...reg...] UNSPEC_SCS_PUSH))
>>>>
>>>> so that there is no chance that the pattern would be treated as
>>>> a normal move and optimised accordingly.
>>>>
>>>
>>> Yeah, this template looks more appropriate if it is to be treated
>>> as a special directive.
>>>
>>> Thanks for your suggestions,
>>> Dan