Re: [PATCH] [PATCH V4]ARM64: SCS: Add gcc plugin to support Shadow Call Stack

From: Dan Li <ashimida@linux.alibaba.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: masahiroy@kernel.org, michal.lkml@markovi.net,
	catalin.marinas@arm.com, will@kernel.org, keescook@chromium.org,
	nathan@kernel.org, tglx@linutronix.de, akpm@linux-foundation.org,
	samitolvanen@google.com, frederic@kernel.org, rppt@kernel.org,
	mark.rutland@arm.com, yifeifz2@illinois.edu, rostedt@goodmis.org,
	viresh.kumar@linaro.org, andreyknvl@gmail.com,
	colin.king@canonical.com, ojeda@kernel.org,
	luc.vanoostenryck@gmail.com, elver@google.com,
	nivedita@alum.mit.edu, ardb@kernel.org,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-hardening@vger.kernel.org,
	clang-built-linux@googlegroups.com
Subject: Re: [PATCH] [PATCH V4]ARM64: SCS: Add gcc plugin to support Shadow Call Stack
Date: Sat, 16 Oct 2021 05:52:03 +0800	[thread overview]
Message-ID: <d1873b11-2aa2-d08c-921c-0a28c4edd33f@linux.alibaba.com> (raw)
In-Reply-To: <CAKwvOdnMvBP-1=YbXTpYOgWqCBy44tUvWdtMXp8p485bYnPYNQ@mail.gmail.com>

On 10/16/21 3:13 AM, Nick Desaulniers wrote:
> On Fri, Oct 15, 2021 at 11:29 AM Dan Li <ashimida@linux.alibaba.com> wrote:
>>
>>
>>
>> On 10/15/21 2:44 AM, Nick Desaulniers wrote:
>>>    On Wed, Oct 13, 2021 at 4:28 PM Dan Li <ashimida@linux.alibaba.com> wrote:
>>>> --- a/include/linux/compiler-gcc.h
>>>> +++ b/include/linux/compiler-gcc.h
>>>> @@ -50,6 +50,10 @@
>>>>    #define __latent_entropy __attribute__((latent_entropy))
>>>>    #endif
>>>>
>>>> +#if defined(SHADOW_CALL_STACK_PLUGIN) && !defined(__CHECKER__)
>>>> +#define __noscs __attribute__((no_shadow_call_stack))
>>>> +#endif
>>>
>>> Cool this is a nice addition, and something I don't think that clang
>>> has.  For any new feature, having a function attribute to disable it
>>> at the function granularity is nice, and plays better with LTO than -f
>>> group flags.  Though that begs the question: what happens if a __noscs
>>> callee is inlined into a non-__noscs caller, or vice versa?
>> Thanks Nick,
>>
>> According to my understanding, all inline optimizations in gcc should
>> happen before inserting scs insns (scs and paciasp/autiasp use the
>> same insertion point). Therefore, the check for the __noscs attribute
>> will also occur after all inlining is completed.
>>
>> As in the following example:
>> - Since __noscs attribute is specified, scs_test1 does not insert scs insns
>> - Since normal functions scs_test2/3 uses x30, it needs to insert scs insns
>> - Since __noscs attribute is specified, scs_test4 after inlining does not
>> need to insert scs insns
>>
>> __always_inline __noscs void scs_test1(void)
>> {
>>       asm volatile("mov x1, x1\n\t":::"x30");
>> }
>>
>> //scs insns inserted after function inline
>> void scs_test2(void)
>> {
>>       scs_test1();
>> }
> 
> That may be surprising to developers.  Perhaps __always_inline on
> scs_test1 is distracting this test case, but I suspect it may not make
> a difference.  This particular issue comes up time and again with
> stack protectors; ie. the callee is marked no stack protector, then
> gets inlined into a caller and suddenly gets a stack protector.
>
Yes.

I haven’t noticed this issue. I just took a quick look at the stack
canary code, and found that the instructions are generated in the
RTL stage like scs/pac (AST => GIMPLE => RTL => asm output), and
the inlining should have been completed before this.

Generally, instructions that are sensitive to assembly order can
only be inserted during RTL, otherwise their order is difficult to
guarantee. I think this may be the reason why the similar problem
always appears.