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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mptfsp3lvm5.fsf@arm.com>
Date:   Mon, 31 Jan 2022 16:26:26 +0000
From:   Richard Sandiford <richard.sandiford@....com>
To:     Dan Li <ashimida@...ux.alibaba.com>
Cc:     Ard Biesheuvel <ardb@...nel.org>, gcc-patches@....gnu.org,
        Richard Earnshaw <richard.earnshaw@....com>,
        marcus.shawcroft@....com, Kyrylo Tkachov <kyrylo.tkachov@....com>,
        hp@....gnu.org, Nick Desaulniers <ndesaulniers@...gle.com>,
        nsz@....gnu.org, pageexec@...il.com, qinzhao@....gnu.org,
        linux-hardening@...r.kernel.org,
        Peter Collingbourne <pcc@...gle.com>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PING^3][PATCH,v2,1/1,AARCH64][PR102768] aarch64: Add compiler support for Shadow Call Stack

Thanks for the discussion and sorry for the slow reply, was out most of
last week.

Dan Li <ashimida@...ux.alibaba.com> writes:
> Thanks, Ard,
>
> On 1/26/22 00:10, Ard Biesheuvel wrote:
>> On Wed, 26 Jan 2022 at 08:53, Dan Li <ashimida@...ux.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.

OK, thanks.  Let's go with that for now then.  There would still be
time to change our minds before GCC 12 is released, if anyone feels
that patching SCS code would be useful.

Reading it back, I think my previous message came across as sounding
like a complaint against binary patching, which wasn't the case at all.
I think it would be fine to support patching, even if it was just for a
single vendor rather than expected to be a common case.  It's just that,
if we did want to support it, we'd need to document it as a requirement
(at least within GCC) and change the implementation accordingly.

Thanks,
Richard