Re: [PATCH] [RFC/RFT] AARCH64: Add gcc Shadow Call Stack support

From: Dan Li <ashimida@linux.alibaba.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Kees Cook <keescook@chromium.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Mike Rapoport <rppt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	frederic@kernel.org, yifeifz2@illinois.edu,
	Steven Rostedt <rostedt@goodmis.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	andreyknvl@gmail.com, Colin King <colin.king@canonical.com>,
	Miguel Ojeda <ojeda@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	Marco Elver <elver@google.com>,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH] [RFC/RFT] AARCH64: Add gcc Shadow Call Stack support
Date: Wed, 3 Nov 2021 00:03:24 +0800	[thread overview]
Message-ID: <b08a94af-f7a4-c228-0ec8-24709385f1d5@linux.alibaba.com> (raw)
In-Reply-To: <CANiq72meBX1g6z2-vfNpMw2C8S-BavuA4CTyPLpD4zUcX193Sw@mail.gmail.com>


On 11/2/21 5:51 PM, Miguel Ojeda wrote:
> On Tue, Nov 2, 2021 at 8:58 AM Dan Li <ashimida@linux.alibaba.com> wrote:
>>
>> I tried to submit a patch[1] to add compiler's SCS support on gcc-11.1.0.
> 
> This would go into GCC 12, right?
>
Oh, yes, gcc-11.1.0 is the version I used to test this patch.

>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583062.html
> 
> Nit: you can use the `Link: ` tag for this.
>
Thanks, Miguel :)

>>   # Supported by clang >= 7.0
> 
> We should add a comment here saying the minimum version too, e.g. GCC
>> = 12 (assuming it will be merged)
> 
Ok, I will add a comment in the next version, if this patch will be merged.

>>   config CC_HAVE_SHADOW_CALL_STACK
>> -       def_bool $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
>> +       def_bool CC_IS_CLANG || $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
> 
> This is changing the default for Clang (which looks fine due to our
> minimum Clang version), but if the test for GCC works the same way,
> why not just keep the line as it was since it covers both?
>
That sounds reasonable, keep this line unchanged is fine.

>> +#if __has_attribute(__no_sanitize_shadow_call_stack__)
>> +#define __noscs __attribute__((no_sanitize_shadow_call_stack))
>> +#else
>> +#define __noscs
>> +#endif
> 
> No need for the `else` branch here, it is done in `compiler_types.h`
> (to be consistent with Clang).
> 
Oh, I see, thanks.

> Also, I hope one day GCC and Clang doing the same for these
> sanitize-related bits...
> 
> Cheers,
> Miguel
>