| Message ID | 20191018161033.261971-13-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add support for Clang's Shadow Call Stack | expand |
On Fri, Oct 18, 2019 at 9:11 AM 'Sami Tolvanen' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > Only reserve x18 with CONFIG_SHADOW_CALL_STACK. Note that all external > kernel modules must also have x18 reserved if the kernel uses SCS. Ah, ok. The tradeoff for maintainers to consider, either: 1. one less GPR for ALL kernel code or 2. remember not to use x18 in inline as lest you potentially break SCS This patch is 2 (the earlier patch was 1). Maybe we don't write enough inline asm that this will be hard to remember, and we do have CI in Android to watch for this (on mainline, not sure about -next). Either way, Acked-by: Nick Desaulniers <ndesaulniers@google.com> > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > arch/arm64/Makefile | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index 1c7b276bc7c5..ef76101201b2 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -55,7 +55,7 @@ endif > > KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \ > $(compat_vdso) $(cc_has_k_constraint) > -KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -ffixed-x18 > +KBUILD_CFLAGS += -fno-asynchronous-unwind-tables > KBUILD_CFLAGS += $(call cc-disable-warning, psabi) > KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) $(compat_vdso) > > @@ -72,6 +72,10 @@ stack_protector_prepare: prepare0 > include/generated/asm-offsets.h)) > endif > > +ifeq ($(CONFIG_SHADOW_CALL_STACK), y) > +KBUILD_CFLAGS += -ffixed-x18 > +endif > + > ifeq ($(CONFIG_CPU_BIG_ENDIAN), y) > KBUILD_CPPFLAGS += -mbig-endian > CHECKFLAGS += -D__AARCH64EB__ > -- > 2.23.0.866.gb869b98d4c-goog > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20191018161033.261971-13-samitolvanen%40google.com.
On Fri, Oct 18, 2019 at 02:23:10PM -0700, Nick Desaulniers wrote: > On Fri, Oct 18, 2019 at 9:11 AM 'Sami Tolvanen' via Clang Built Linux > <clang-built-linux@googlegroups.com> wrote: > > > > Only reserve x18 with CONFIG_SHADOW_CALL_STACK. Note that all external > > kernel modules must also have x18 reserved if the kernel uses SCS. > > Ah, ok. The tradeoff for maintainers to consider, either: > 1. one less GPR for ALL kernel code or > 2. remember not to use x18 in inline as lest you potentially break SCS This option only affects compiler-generated code, so I don't think that matters. I think it's fine to say that we should always avoid the use of x18 in hand-written assembly (with manual register allocation), while also allowing the compiler to use x18 if we're not using SCS. This can be folded into the earlier patch which always reserved x18. > This patch is 2 (the earlier patch was 1). Maybe we don't write > enough inline asm that this will be hard to remember, and we do have > CI in Android to watch for this (on mainline, not sure about -next). I think that we can trust the set of people who regularly review arm64 assembly to remember this. We could also document this somewhere -- we might need to document other constraints or conventions for assembly in preparation for livepatching and so on. If we wanted to, we could periodically grep for x18 to find any illicit usage. Thanks, Mark.
On Tue, Oct 22, 2019 at 05:00:10PM +0100, Mark Rutland wrote: > If we wanted to, we could periodically grep for x18 to find any illicit > usage. Now we need objtool for arm64! :) (It seems CONFIG_HAVE_STACK_VALIDATION is rather a narrow description for what objtool does now...)
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 1c7b276bc7c5..ef76101201b2 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -55,7 +55,7 @@ endif KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \ $(compat_vdso) $(cc_has_k_constraint) -KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -ffixed-x18 +KBUILD_CFLAGS += -fno-asynchronous-unwind-tables KBUILD_CFLAGS += $(call cc-disable-warning, psabi) KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) $(compat_vdso) @@ -72,6 +72,10 @@ stack_protector_prepare: prepare0 include/generated/asm-offsets.h)) endif +ifeq ($(CONFIG_SHADOW_CALL_STACK), y) +KBUILD_CFLAGS += -ffixed-x18 +endif + ifeq ($(CONFIG_CPU_BIG_ENDIAN), y) KBUILD_CPPFLAGS += -mbig-endian CHECKFLAGS += -D__AARCH64EB__
Only reserve x18 with CONFIG_SHADOW_CALL_STACK. Note that all external kernel modules must also have x18 reserved if the kernel uses SCS. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- arch/arm64/Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)