[PATCH] arm64: scs: Work around full LTO issue with dynamic SCS

[PATCH] arm64: scs: Work around full LTO issue with dynamic SCS

[Ard Biesheuvel]

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>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: scs: Work around full LTO issue with dynamic SCS

[Kees Cook]

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
> 

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: scs: Work around full LTO issue with dynamic SCS

[Sami Tolvanen]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: scs: Work around full LTO issue with dynamic SCS

[Ard Biesheuvel]

(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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: scs: Work around full LTO issue with dynamic SCS

[Sami Tolvanen]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: scs: Work around full LTO issue with dynamic SCS

[Will Deacon]

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,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel