* [PATCH] arm64: Always force a branch protection mode when the compiler has one
@ 2020-03-31 19:44 Mark Brown
2020-03-31 19:46 ` Kees Cook
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Mark Brown @ 2020-03-31 19:44 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Mark Rutland, Kees Cook, Szabolcs Nagy, stable, Mark Brown,
Amit Daniel Kachhap, linux-arm-kernel
Compilers with branch protection support can be configured to enable it by
default, it is likely that distributions will do this as part of deploying
branch protection system wide. As well as the slight overhead from having
some extra NOPs for unused branch protection features this can cause more
serious problems when the kernel is providing pointer authentication to
userspace but not built for pointer authentication itself. In that case our
switching of keys for userspace can affect the kernel unexpectedly, causing
pointer authentication instructions in the kernel to corrupt addresses.
To ensure that we get consistent and reliable behaviour always explicitly
initialise the branch protection mode, ensuring that the kernel is built
the same way regardless of the compiler defaults.
Fixes: 7503197562567 (arm64: add basic pointer authentication support)
Reported-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
arch/arm64/Kconfig | 4 ++++
arch/arm64/Makefile | 7 ++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index d3efdc095a17..1e46746e8392 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1537,6 +1537,10 @@ config ARM64_PTR_AUTH
This feature works with FUNCTION_GRAPH_TRACER option only if
DYNAMIC_FTRACE_WITH_REGS is enabled.
+config CC_HAS_BRANCH_PROT_NONE
+ # GCC 9 or later, clang 8 or later
+ def_bool $(cc-option,-mbranch-protection=none)
+
config CC_HAS_BRANCH_PROT_PAC_RET
# GCC 9 or later, clang 8 or later
def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index f15f92ba53e6..370fca6663c8 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -65,6 +65,10 @@ stack_protector_prepare: prepare0
include/generated/asm-offsets.h))
endif
+# Ensure that if the compiler supports branch protection we default it
+# off, this will be overridden if we are using branch protection.
+branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_NONE) := -mbranch-protection=none
+
ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
@@ -73,9 +77,10 @@ branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pa
# we pass it only to the assembler. This option is utilized only in case of non
# integrated assemblers.
branch-prot-flags-$(CONFIG_AS_HAS_PAC) += -Wa,-march=armv8.3-a
-KBUILD_CFLAGS += $(branch-prot-flags-y)
endif
+KBUILD_CFLAGS += $(branch-prot-flags-y)
+
ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
KBUILD_CPPFLAGS += -mbig-endian
CHECKFLAGS += -D__AARCH64EB__
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: Always force a branch protection mode when the compiler has one
2020-03-31 19:44 [PATCH] arm64: Always force a branch protection mode when the compiler has one Mark Brown
@ 2020-03-31 19:46 ` Kees Cook
2020-04-01 9:55 ` Mark Rutland
2020-04-01 17:54 ` Catalin Marinas
2 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2020-03-31 19:46 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, Szabolcs Nagy, Catalin Marinas, stable,
Amit Daniel Kachhap, Will Deacon, linux-arm-kernel
On Tue, Mar 31, 2020 at 08:44:59PM +0100, Mark Brown wrote:
> Compilers with branch protection support can be configured to enable it by
> default, it is likely that distributions will do this as part of deploying
> branch protection system wide. As well as the slight overhead from having
> some extra NOPs for unused branch protection features this can cause more
> serious problems when the kernel is providing pointer authentication to
> userspace but not built for pointer authentication itself. In that case our
> switching of keys for userspace can affect the kernel unexpectedly, causing
> pointer authentication instructions in the kernel to corrupt addresses.
>
> To ensure that we get consistent and reliable behaviour always explicitly
> initialise the branch protection mode, ensuring that the kernel is built
> the same way regardless of the compiler defaults.
>
> Fixes: 7503197562567 (arm64: add basic pointer authentication support)
> Reported-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Cc: stable@vger.kernel.org
> ---
> arch/arm64/Kconfig | 4 ++++
> arch/arm64/Makefile | 7 ++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index d3efdc095a17..1e46746e8392 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1537,6 +1537,10 @@ config ARM64_PTR_AUTH
> This feature works with FUNCTION_GRAPH_TRACER option only if
> DYNAMIC_FTRACE_WITH_REGS is enabled.
>
> +config CC_HAS_BRANCH_PROT_NONE
> + # GCC 9 or later, clang 8 or later
> + def_bool $(cc-option,-mbranch-protection=none)
> +
> config CC_HAS_BRANCH_PROT_PAC_RET
> # GCC 9 or later, clang 8 or later
> def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index f15f92ba53e6..370fca6663c8 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -65,6 +65,10 @@ stack_protector_prepare: prepare0
> include/generated/asm-offsets.h))
> endif
>
> +# Ensure that if the compiler supports branch protection we default it
> +# off, this will be overridden if we are using branch protection.
> +branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_NONE) := -mbranch-protection=none
> +
> ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
> branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
> branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> @@ -73,9 +77,10 @@ branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pa
> # we pass it only to the assembler. This option is utilized only in case of non
> # integrated assemblers.
> branch-prot-flags-$(CONFIG_AS_HAS_PAC) += -Wa,-march=armv8.3-a
> -KBUILD_CFLAGS += $(branch-prot-flags-y)
> endif
>
> +KBUILD_CFLAGS += $(branch-prot-flags-y)
> +
> ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
> KBUILD_CPPFLAGS += -mbig-endian
> CHECKFLAGS += -D__AARCH64EB__
> --
> 2.20.1
>
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: Always force a branch protection mode when the compiler has one
2020-03-31 19:44 [PATCH] arm64: Always force a branch protection mode when the compiler has one Mark Brown
2020-03-31 19:46 ` Kees Cook
@ 2020-04-01 9:55 ` Mark Rutland
2020-04-01 17:54 ` Catalin Marinas
2 siblings, 0 replies; 5+ messages in thread
From: Mark Rutland @ 2020-04-01 9:55 UTC (permalink / raw)
To: Mark Brown
Cc: Kees Cook, Szabolcs Nagy, Catalin Marinas, stable,
Amit Daniel Kachhap, Will Deacon, linux-arm-kernel
On Tue, Mar 31, 2020 at 08:44:59PM +0100, Mark Brown wrote:
> Compilers with branch protection support can be configured to enable it by
> default, it is likely that distributions will do this as part of deploying
> branch protection system wide. As well as the slight overhead from having
> some extra NOPs for unused branch protection features this can cause more
> serious problems when the kernel is providing pointer authentication to
> userspace but not built for pointer authentication itself. In that case our
> switching of keys for userspace can affect the kernel unexpectedly, causing
> pointer authentication instructions in the kernel to corrupt addresses.
>
> To ensure that we get consistent and reliable behaviour always explicitly
> initialise the branch protection mode, ensuring that the kernel is built
> the same way regardless of the compiler defaults.
>
> Fixes: 7503197562567 (arm64: add basic pointer authentication support)
> Reported-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/Kconfig | 4 ++++
> arch/arm64/Makefile | 7 ++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index d3efdc095a17..1e46746e8392 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1537,6 +1537,10 @@ config ARM64_PTR_AUTH
> This feature works with FUNCTION_GRAPH_TRACER option only if
> DYNAMIC_FTRACE_WITH_REGS is enabled.
>
> +config CC_HAS_BRANCH_PROT_NONE
> + # GCC 9 or later, clang 8 or later
> + def_bool $(cc-option,-mbranch-protection=none)
> +
> config CC_HAS_BRANCH_PROT_PAC_RET
> # GCC 9 or later, clang 8 or later
> def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index f15f92ba53e6..370fca6663c8 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -65,6 +65,10 @@ stack_protector_prepare: prepare0
> include/generated/asm-offsets.h))
> endif
>
> +# Ensure that if the compiler supports branch protection we default it
> +# off, this will be overridden if we are using branch protection.
> +branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_NONE) := -mbranch-protection=none
> +
> ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
> branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
> branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> @@ -73,9 +77,10 @@ branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pa
> # we pass it only to the assembler. This option is utilized only in case of non
> # integrated assemblers.
> branch-prot-flags-$(CONFIG_AS_HAS_PAC) += -Wa,-march=armv8.3-a
> -KBUILD_CFLAGS += $(branch-prot-flags-y)
> endif
>
> +KBUILD_CFLAGS += $(branch-prot-flags-y)
> +
> ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
> KBUILD_CPPFLAGS += -mbig-endian
> CHECKFLAGS += -D__AARCH64EB__
> --
> 2.20.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: Always force a branch protection mode when the compiler has one
2020-03-31 19:44 [PATCH] arm64: Always force a branch protection mode when the compiler has one Mark Brown
2020-03-31 19:46 ` Kees Cook
2020-04-01 9:55 ` Mark Rutland
@ 2020-04-01 17:54 ` Catalin Marinas
2020-04-01 18:07 ` Mark Brown
2 siblings, 1 reply; 5+ messages in thread
From: Catalin Marinas @ 2020-04-01 17:54 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, Kees Cook, Szabolcs Nagy, stable, Amit Kachhap,
Will Deacon, linux-arm-kernel
On Tue, Mar 31, 2020 at 08:44:59PM +0100, Mark Brown wrote:
> Compilers with branch protection support can be configured to enable it by
> default, it is likely that distributions will do this as part of deploying
> branch protection system wide. As well as the slight overhead from having
> some extra NOPs for unused branch protection features this can cause more
> serious problems when the kernel is providing pointer authentication to
> userspace but not built for pointer authentication itself.
With 5.7 you won't be able to configure user and kernel PAC support
independently. So, I guess that's something only for prior kernel
versions.
> In that case our
> switching of keys for userspace can affect the kernel unexpectedly, causing
> pointer authentication instructions in the kernel to corrupt addresses.
>
> To ensure that we get consistent and reliable behaviour always explicitly
> initialise the branch protection mode, ensuring that the kernel is built
> the same way regardless of the compiler defaults.
>
> Fixes: 7503197562567 (arm64: add basic pointer authentication support)
> Reported-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
> arch/arm64/Kconfig | 4 ++++
> arch/arm64/Makefile | 7 ++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index d3efdc095a17..1e46746e8392 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1537,6 +1537,10 @@ config ARM64_PTR_AUTH
> This feature works with FUNCTION_GRAPH_TRACER option only if
> DYNAMIC_FTRACE_WITH_REGS is enabled.
>
> +config CC_HAS_BRANCH_PROT_NONE
> + # GCC 9 or later, clang 8 or later
> + def_bool $(cc-option,-mbranch-protection=none)
I don't think we need to bother with a Kconfig entry here. We did it for
the other options since CONFIG_ARM64_PTR_AUTH has a dependency on them.
> +
> config CC_HAS_BRANCH_PROT_PAC_RET
> # GCC 9 or later, clang 8 or later
> def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index f15f92ba53e6..370fca6663c8 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -65,6 +65,10 @@ stack_protector_prepare: prepare0
> include/generated/asm-offsets.h))
> endif
>
> +# Ensure that if the compiler supports branch protection we default it
> +# off, this will be overridden if we are using branch protection.
> +branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_NONE) := -mbranch-protection=none
And a $(call cc-option,-mbranch-protection=none) here.
branch-prot-flags-y is only introduced in 5.7, so backporting may look
slightly weirder.
> +
> ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
> branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
> branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> @@ -73,9 +77,10 @@ branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pa
> # we pass it only to the assembler. This option is utilized only in case of non
> # integrated assemblers.
> branch-prot-flags-$(CONFIG_AS_HAS_PAC) += -Wa,-march=armv8.3-a
> -KBUILD_CFLAGS += $(branch-prot-flags-y)
> endif
>
> +KBUILD_CFLAGS += $(branch-prot-flags-y)
Or just use an else clause here with:
KBUILD_CFLAGS += ($call cc-option,-mbranch-protection=none).
On backports, we just drop else/endif since they don't exist.
Not a strong preference really, just looking to have backports resemble
upstream better. I can fix it up locally, whichever variant we go for
(or even this one).
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: Always force a branch protection mode when the compiler has one
2020-04-01 17:54 ` Catalin Marinas
@ 2020-04-01 18:07 ` Mark Brown
0 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2020-04-01 18:07 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, Kees Cook, Szabolcs Nagy, stable, Amit Kachhap,
Will Deacon, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 963 bytes --]
On Wed, Apr 01, 2020 at 06:54:44PM +0100, Catalin Marinas wrote:
> On Tue, Mar 31, 2020 at 08:44:59PM +0100, Mark Brown wrote:
> > Compilers with branch protection support can be configured to enable it by
> > default, it is likely that distributions will do this as part of deploying
> > branch protection system wide. As well as the slight overhead from having
> > some extra NOPs for unused branch protection features this can cause more
> > serious problems when the kernel is providing pointer authentication to
> > userspace but not built for pointer authentication itself.
> With 5.7 you won't be able to configure user and kernel PAC support
> independently. So, I guess that's something only for prior kernel
> versions.
Yes, it's really for the benefit of stable at this point - hence the Cc.
Going forward it's hopefully more for defensiveness than anything else,
it's possible something similar might come up with some future stuff but
ideally not.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-01 18:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31 19:44 [PATCH] arm64: Always force a branch protection mode when the compiler has one Mark Brown
2020-03-31 19:46 ` Kees Cook
2020-04-01 9:55 ` Mark Rutland
2020-04-01 17:54 ` Catalin Marinas
2020-04-01 18:07 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).