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

From: Nick Desaulniers <ndesaulniers@google.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Dan Li <ashimida@linux.alibaba.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	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: Tue, 2 Nov 2021 11:41:58 -0700	[thread overview]
Message-ID: <CAKwvOd=0GpX59U1ndzahJ8mh26C-Gb54pjwUk4eYigb0ZoCiwg@mail.gmail.com> (raw)
In-Reply-To: <CANiq72meBX1g6z2-vfNpMw2C8S-BavuA4CTyPLpD4zUcX193Sw@mail.gmail.com>

On Tue, Nov 2, 2021 at 2:52 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> 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?
>
> > [1] https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583062.html
>
> Nit: you can use the `Link: ` tag for this.
>
> >  # 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)
>
> >  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?

Yeah, let's just have the cc-option check used for both toolchains.

> > +#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).

Do we want to move this to include/linux/compiler_attributes.h?
Respecifying these repeatedly in each include/linux/compiler-*.h feels
excessively redundant.

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

...
-- 
Thanks,
~Nick Desaulniers