| Message ID | 20240110132619.258809-2-ardb+git@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: scs: Work around full LTO issue with dynamic SCS | expand |
On Wed, Jan 10, 2024 at 02:26:20PM +0100, Ard Biesheuvel wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > Full LTO takes the '-mbranch-protection=none' passed to the compiler > when generating the dynamic shadow call stack patching code as a hint to > stop emitting PAC instructions altogether. (Thin LTO appears unaffected > by this) > > Work around this by stripping unwind tables from the object in question, > which should be sufficient to prevent the patching code from attempting > to patch itself. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Thanks for finding a work-around for this! Do you want to include the Reported-by: or Cc: stable@... tags for this? Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > arch/arm64/kernel/Makefile | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index d95b3d6b471a..e5d03a7039b4 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -73,7 +73,13 @@ obj-$(CONFIG_ARM64_MTE) += mte.o > obj-y += vdso-wrap.o > obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o > obj-$(CONFIG_UNWIND_PATCH_PAC_INTO_SCS) += patch-scs.o > -CFLAGS_patch-scs.o += -mbranch-protection=none > + > +# We need to prevent the SCS patching code from patching itself. Using > +# -mbranch-protection=none here to avoid the patchable PAC opcodes from being > +# generated triggers an issue with full LTO on Clang, which stops emitting PAC > +# instructions altogether. So instead, omit the unwind tables used by the > +# patching code, so it will not be able to locate its own PAC instructions. > +CFLAGS_patch-scs.o += -fno-asynchronous-unwind-tables -fno-unwind-tables > > # Force dependency (vdso*-wrap.S includes vdso.so through incbin) > $(obj)/vdso-wrap.o: $(obj)/vdso/vdso.so > -- > 2.43.0.472.g3155946c3a-goog >
Hi Ard, On Wed, Jan 10, 2024 at 5:26 AM Ard Biesheuvel <ardb+git@google.com> wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > Full LTO takes the '-mbranch-protection=none' passed to the compiler > when generating the dynamic shadow call stack patching code as a hint to > stop emitting PAC instructions altogether. (Thin LTO appears unaffected > by this) Does this affect all Clang versions? Is there a compiler bug filed for this issue? > Work around this by stripping unwind tables from the object in question, > which should be sufficient to prevent the patching code from attempting > to patch itself. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Please add a Fixes tag to ensure this gets backported. Otherwise, looks like a reasonable workaround to me. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Sami
(cc Nathan) On Wed, 10 Jan 2024 at 20:58, Sami Tolvanen <samitolvanen@google.com> wrote: > > Hi Ard, > > On Wed, Jan 10, 2024 at 5:26 AM Ard Biesheuvel <ardb+git@google.com> wrote: > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > Full LTO takes the '-mbranch-protection=none' passed to the compiler > > when generating the dynamic shadow call stack patching code as a hint to > > stop emitting PAC instructions altogether. (Thin LTO appears unaffected > > by this) > > Does this affect all Clang versions? Is there a compiler bug filed for > this issue? > No, not yet. I suppose reporting this as-is with LLVM is not going to be practical, but I'm not sure how to isolate a reproducer. Note that there are other compilation units (under arch/arm64/kernel/pi) that are also built with -mbranch-protection=none, but those don't appear to trigger this issue in the same way. > > Work around this by stripping unwind tables from the object in question, > > which should be sufficient to prevent the patching code from attempting > > to patch itself. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > Please add a Fixes tag to ensure this gets backported. Otherwise, > looks like a reasonable workaround to me. > Ok > Reviewed-by: Sami Tolvanen <samitolvanen@google.com> > Thanks > Sami
On Wed, Jan 10, 2024 at 12:22 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > (cc Nathan) > > On Wed, 10 Jan 2024 at 20:58, Sami Tolvanen <samitolvanen@google.com> wrote: > > > > Hi Ard, > > > > On Wed, Jan 10, 2024 at 5:26 AM Ard Biesheuvel <ardb+git@google.com> wrote: > > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > Full LTO takes the '-mbranch-protection=none' passed to the compiler > > > when generating the dynamic shadow call stack patching code as a hint to > > > stop emitting PAC instructions altogether. (Thin LTO appears unaffected > > > by this) > > > > Does this affect all Clang versions? Is there a compiler bug filed for > > this issue? > > > > No, not yet. > > I suppose reporting this as-is with LLVM is not going to be practical, > but I'm not sure how to isolate a reproducer. Note that there are > other compilation units (under arch/arm64/kernel/pi) that are also > built with -mbranch-protection=none, but those don't appear to trigger > this issue in the same way. It's probably because LTO is disabled for the directory, so the TUs there get compiled separately. Sami
On Wed, 10 Jan 2024 14:26:20 +0100, Ard Biesheuvel wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > Full LTO takes the '-mbranch-protection=none' passed to the compiler > when generating the dynamic shadow call stack patching code as a hint to > stop emitting PAC instructions altogether. (Thin LTO appears unaffected > by this) > > [...] Applied to arm64 (for-next/core), thanks! [1/1] arm64: scs: Work around full LTO issue with dynamic SCS https://git.kernel.org/arm64/c/8c5a19cb17a7 Cheers,
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index d95b3d6b471a..e5d03a7039b4 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -73,7 +73,13 @@ obj-$(CONFIG_ARM64_MTE) += mte.o obj-y += vdso-wrap.o obj-$(CONFIG_COMPAT_VDSO) += vdso32-wrap.o obj-$(CONFIG_UNWIND_PATCH_PAC_INTO_SCS) += patch-scs.o -CFLAGS_patch-scs.o += -mbranch-protection=none + +# We need to prevent the SCS patching code from patching itself. Using +# -mbranch-protection=none here to avoid the patchable PAC opcodes from being +# generated triggers an issue with full LTO on Clang, which stops emitting PAC +# instructions altogether. So instead, omit the unwind tables used by the +# patching code, so it will not be able to locate its own PAC instructions. +CFLAGS_patch-scs.o += -fno-asynchronous-unwind-tables -fno-unwind-tables # Force dependency (vdso*-wrap.S includes vdso.so through incbin) $(obj)/vdso-wrap.o: $(obj)/vdso/vdso.so