[PATCH 0/2] arm64: pointer auth cleanup

[PATCH 0/2] arm64: pointer auth cleanup

[Mark Rutland]

With the recent dynamic SCS patches conditionally disabling pointer
authentication for leaf functions, I thought I'd try to clean things up
and consistently do so.

Patch 1 is (hopefully not controversial) Makefile cleanup.

Patch 2 disables pointer authentication for leaf functions, for the
reasons laid out in the commit message (which laregly boils down to
"it's not necessary" and "our prior rationale for doing so doesn't hold
water").

Thanks,
Mark.

Mark Rutland (2):
  arm64: unify asm-arch manipulation
  arm64: pauth: don't sign leaf functions

 arch/arm64/Kconfig  |  4 +--
 arch/arm64/Makefile | 65 ++++++++++++++++++---------------------------
 2 files changed, 28 insertions(+), 41 deletions(-)

-- 
2.30.2


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

[PATCH 1/2] arm64: unify asm-arch manipulation

[Mark Rutland]

Assemblers will reject instructions not supported by a target
architecture version, and so we must explicitly tell the assembler the
latest architecture version for which we want to assemble instructions
from.

We've added a few AS_HAS_ARMV8_<N> definitions for this, in addition to
an inconsistently named AS_HAS_PAC definition, from which arm64's
top-level Makefile determines the architecture version that we intend to
target, and generates the `asm-arch` variable.

To make this a bit clearer and easier to maintain, this patch reworks
the Makefile to determine asm-arch in a single if-else-endif chain.
AS_HAS_PAC, which is defined when the assembler supports
`-march=armv8.3-a`, is renamed to AS_HAS_ARMV8_3.

As the logic for armv8.3-a is lifted out of the block handling pointer
authentication, `asm-arch` may now be set to armv8.3-a regardless of
whether support for pointer authentication is selected. This means that
it will be possible to assemble armv8.3-a instructions even if we didn't
intend to, but this is consistent with our handling of other
architecture versions, and the compiler won't generate armv8.3-a
instructions regardless.

For the moment there's no need for an CONFIG_AS_HAS_ARMV8_1, as the code
for LSE atomics and LDAPR use individual `.arch_extension` entries and
do not require the baseline asm arch to be bumped to armv8.1-a. The
other armv8.1-a features (e.g. PAN) do not require assembler support.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/Kconfig  |  4 ++--
 arch/arm64/Makefile | 37 ++++++++++++++++++-------------------
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 03934808b2ed0..be760ad7715df 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1818,7 +1818,7 @@ config ARM64_PTR_AUTH_KERNEL
 	bool "Use pointer authentication for kernel"
 	default y
 	depends on ARM64_PTR_AUTH
-	depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
+	depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_ARMV8_3
 	# Modern compilers insert a .note.gnu.property section note for PAC
 	# which is only understood by binutils starting with version 2.33.1.
 	depends on LD_IS_LLD || LD_VERSION >= 23301 || (CC_IS_GCC && GCC_VERSION < 90100)
@@ -1843,7 +1843,7 @@ config CC_HAS_SIGN_RETURN_ADDRESS
 	# GCC 7, 8
 	def_bool $(cc-option,-msign-return-address=all)
 
-config AS_HAS_PAC
+config AS_HAS_ARMV8_3
 	def_bool $(cc-option,-Wa$(comma)-march=armv8.3-a)
 
 config AS_HAS_CFI_NEGATE_RA_STATE
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index d62bd221828f7..e176eb76345b5 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -63,11 +63,6 @@ stack_protector_prepare: prepare0
 					include/generated/asm-offsets.h))
 endif
 
-ifeq ($(CONFIG_AS_HAS_ARMV8_2), y)
-# make sure to pass the newest target architecture to -march.
-asm-arch := armv8.2-a
-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-y += $(call cc-option,-mbranch-protection=none)
@@ -88,25 +83,29 @@ branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protectio
 else
 branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=$(PACRET-y)
 endif
-# -march=armv8.3-a enables the non-nops instructions for PAC, to avoid the
-# compiler to generate them and consequently to break the single image contract
-# we pass it only to the assembler. This option is utilized only in case of non
-# integrated assemblers.
-ifeq ($(CONFIG_AS_HAS_PAC), y)
-asm-arch := armv8.3-a
-endif
 endif
 
 KBUILD_CFLAGS += $(branch-prot-flags-y)
 
-ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
-# make sure to pass the newest target architecture to -march.
-asm-arch := armv8.4-a
-endif
-
+# Tell the assembler to support instructions from the latest target
+# architecture.
+#
+# For non-integrated assemblers we'll pass this on the command line, and for
+# integrated assemblers we'll define ARM64_ASM_ARCH and ARM64_ASM_PREAMBLE for
+# inline usage.
+#
+# We cannot pass the same arch flag to the compiler as this would allow it to
+# freely generate instructions which are not supported by earlier architecture
+# versions, which would prevent a single kernel image from working on earlier
+# hardware.
 ifeq ($(CONFIG_AS_HAS_ARMV8_5), y)
-# make sure to pass the newest target architecture to -march.
-asm-arch := armv8.5-a
+  asm-arch := armv8.5-a
+else ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
+  asm-arch := armv8.4-a
+else ifeq ($(CONFIG_AS_HAS_ARMV8_3), y)
+  asm-arch := armv8.3-a
+else ifeq ($(CONFIG_AS_HAS_ARMV8_2), y)
+  asm-arch := armv8.2-a
 endif
 
 ifdef asm-arch
-- 
2.30.2


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

Re: [PATCH 1/2] arm64: unify asm-arch manipulation

[Ard Biesheuvel]

On Wed, 25 Jan 2023 at 19:22, Mark Rutland <mark.rutland@arm.com> wrote:
>
> Assemblers will reject instructions not supported by a target
> architecture version, and so we must explicitly tell the assembler the
> latest architecture version for which we want to assemble instructions
> from.
>
> We've added a few AS_HAS_ARMV8_<N> definitions for this, in addition to
> an inconsistently named AS_HAS_PAC definition, from which arm64's
> top-level Makefile determines the architecture version that we intend to
> target, and generates the `asm-arch` variable.
>
> To make this a bit clearer and easier to maintain, this patch reworks
> the Makefile to determine asm-arch in a single if-else-endif chain.
> AS_HAS_PAC, which is defined when the assembler supports
> `-march=armv8.3-a`, is renamed to AS_HAS_ARMV8_3.
>
> As the logic for armv8.3-a is lifted out of the block handling pointer
> authentication, `asm-arch` may now be set to armv8.3-a regardless of
> whether support for pointer authentication is selected. This means that
> it will be possible to assemble armv8.3-a instructions even if we didn't
> intend to, but this is consistent with our handling of other
> architecture versions, and the compiler won't generate armv8.3-a
> instructions regardless.
>
> For the moment there's no need for an CONFIG_AS_HAS_ARMV8_1, as the code
> for LSE atomics and LDAPR use individual `.arch_extension` entries and
> do not require the baseline asm arch to be bumped to armv8.1-a. The
> other armv8.1-a features (e.g. PAN) do not require assembler support.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  arch/arm64/Kconfig  |  4 ++--
>  arch/arm64/Makefile | 37 ++++++++++++++++++-------------------
>  2 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 03934808b2ed0..be760ad7715df 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1818,7 +1818,7 @@ config ARM64_PTR_AUTH_KERNEL
>         bool "Use pointer authentication for kernel"
>         default y
>         depends on ARM64_PTR_AUTH
> -       depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
> +       depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_ARMV8_3
>         # Modern compilers insert a .note.gnu.property section note for PAC
>         # which is only understood by binutils starting with version 2.33.1.
>         depends on LD_IS_LLD || LD_VERSION >= 23301 || (CC_IS_GCC && GCC_VERSION < 90100)
> @@ -1843,7 +1843,7 @@ config CC_HAS_SIGN_RETURN_ADDRESS
>         # GCC 7, 8
>         def_bool $(cc-option,-msign-return-address=all)
>
> -config AS_HAS_PAC
> +config AS_HAS_ARMV8_3
>         def_bool $(cc-option,-Wa$(comma)-march=armv8.3-a)
>
>  config AS_HAS_CFI_NEGATE_RA_STATE
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index d62bd221828f7..e176eb76345b5 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -63,11 +63,6 @@ stack_protector_prepare: prepare0
>                                         include/generated/asm-offsets.h))
>  endif
>
> -ifeq ($(CONFIG_AS_HAS_ARMV8_2), y)
> -# make sure to pass the newest target architecture to -march.
> -asm-arch := armv8.2-a
> -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-y += $(call cc-option,-mbranch-protection=none)
> @@ -88,25 +83,29 @@ branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protectio
>  else
>  branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=$(PACRET-y)
>  endif
> -# -march=armv8.3-a enables the non-nops instructions for PAC, to avoid the
> -# compiler to generate them and consequently to break the single image contract
> -# we pass it only to the assembler. This option is utilized only in case of non
> -# integrated assemblers.
> -ifeq ($(CONFIG_AS_HAS_PAC), y)
> -asm-arch := armv8.3-a
> -endif
>  endif
>
>  KBUILD_CFLAGS += $(branch-prot-flags-y)
>
> -ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
> -# make sure to pass the newest target architecture to -march.
> -asm-arch := armv8.4-a
> -endif
> -
> +# Tell the assembler to support instructions from the latest target
> +# architecture.
> +#
> +# For non-integrated assemblers we'll pass this on the command line, and for
> +# integrated assemblers we'll define ARM64_ASM_ARCH and ARM64_ASM_PREAMBLE for
> +# inline usage.
> +#
> +# We cannot pass the same arch flag to the compiler as this would allow it to
> +# freely generate instructions which are not supported by earlier architecture
> +# versions, which would prevent a single kernel image from working on earlier
> +# hardware.
>  ifeq ($(CONFIG_AS_HAS_ARMV8_5), y)
> -# make sure to pass the newest target architecture to -march.
> -asm-arch := armv8.5-a
> +  asm-arch := armv8.5-a
> +else ifeq ($(CONFIG_AS_HAS_ARMV8_4), y)
> +  asm-arch := armv8.4-a
> +else ifeq ($(CONFIG_AS_HAS_ARMV8_3), y)
> +  asm-arch := armv8.3-a
> +else ifeq ($(CONFIG_AS_HAS_ARMV8_2), y)
> +  asm-arch := armv8.2-a
>  endif
>
>  ifdef asm-arch
> --
> 2.30.2
>

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

Re: [PATCH 1/2] arm64: unify asm-arch manipulation

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 326 bytes --]

On Wed, Jan 25, 2023 at 06:22:00PM +0000, Mark Rutland wrote:
> Assemblers will reject instructions not supported by a target
> architecture version, and so we must explicitly tell the assembler the
> latest architecture version for which we want to assemble instructions
> from.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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

[PATCH 2/2] arm64: pauth: don't sign leaf functions

[Mark Rutland]

Currently, when CONFIG_ARM64_PTR_AUTH_KERNEL=y (and
CONFIG_UNWIND_PATCH_PAC_INTO_SCS=n), we enable pointer authentication
for all functions, including leaf functions. This isn't necessary, and
is unfortunate for a few reasons:

* Any PACIASP instruction is implicitly a `BTI C` landing pad, and
  forcing the addition of a PACIASP in every function introduces a
  larger set of BTI gadgets than is necessary.

* The PACIASP and AUTIASP instructions make leaf functions larger than
  necessary, bloating the kernel Image. For a defconfig v6.2-rc3 kernel,
  this appears to add ~64KiB relative to not signing leaf functions,
  which is unfortunate but not entirely onerous.

* The PACIASP and AUTIASP instructions potentially make leaf functions
  more expensive in terms of performance and/or power. For many trivial
  leaf functions, this is clearly unnecessary, e.g.

  | <arch_local_save_flags>:
  |        d503233f        paciasp
  |        d53b4220        mrs     x0, daif
  |        d50323bf        autiasp
  |        d65f03c0        ret

  | <calibration_delay_done>:
  |        d503233f        paciasp
  |        d50323bf        autiasp
  |        d65f03c0        ret
  |        d503201f        nop

* When CONFIG_UNWIND_PATCH_PAC_INTO_SCS=y we disable pointer
  authentication for leaf functions, so clearly this is not functionally
  necessary, indicates we have an inconsistent threat model, and
  convolutes the Makefile logic.

We've used pointer authentication in leave functions since the
introduction of in-kernel pointer authentication in commit:

  74afda4016a7437e ("arm64: compile the kernel with ptrauth return address signing")

... but at the time we had no rationale for signing leaf functions.

Subsequently, we considered avoiding signing leaf functions:

  https://lore.kernel.org/linux-arm-kernel/1586856741-26839-1-git-send-email-amit.kachhap@arm.com/
  https://lore.kernel.org/linux-arm-kernel/1588149371-20310-1-git-send-email-amit.kachhap@arm.com/

... however at the time we didn't have an abundance of reasons to avoid
signing leaf functions as above (e.g. the BTI case), we had no hardware
to make performance measurements, and it was reasoned that this gave
some level of protection against a limited set of code-reuse gadgets
which would fall through to a RET. We documented this in commit:

  717b938e22f8dbf0 ("arm64: Document why we enable PAC support for leaf functions")

Notably, this was before we supported any forward-edge CFI scheme (e.g.
Arm BTI, or Clang CFI/kCFI), which would prevent jumping into the middle
of a function.

In addition, even with signing forced for leaf functions, AUTIASP may be
placed before a number of instructions which might constitute such a
gadget, e.g.

| <user_regs_reset_single_step>:
|        f9400022        ldr     x2, [x1]
|        d503233f        paciasp
|        d50323bf        autiasp
|        f9408401        ldr     x1, [x0, #264]
|        720b005f        tst     w2, #0x200000
|        b26b0022        orr     x2, x1, #0x200000
|        926af821        and     x1, x1, #0xffffffffffdfffff
|        9a820021        csel    x1, x1, x2, eq  // eq = none
|        f9008401        str     x1, [x0, #264]
|        d65f03c0        ret

| <fpsimd_cpu_dead>:
|        2a0003e3        mov     w3, w0
|        9000ff42        adrp    x2, ffff800009ffd000 <xen_dynamic_chip+0x48>
|        9120e042        add     x2, x2, #0x838
|        52800000        mov     w0, #0x0                        // #0
|        d503233f        paciasp
|        f000d041        adrp    x1, ffff800009a20000 <this_cpu_vector>
|        d50323bf        autiasp
|        9102c021        add     x1, x1, #0xb0
|        f8635842        ldr     x2, [x2, w3, uxtw #3]
|        f821685f        str     xzr, [x2, x1]
|        d65f03c0        ret
|        d503201f        nop

So generally, trying to use AUTIASP to detect such gadgetization is not
robust, and this is dealt with far better by forward-edge CFI (which is
designed to prevent such cases). We should bite the buller and stop
pretending that AUTIASP is a mitigation for such forward-edge
gadgetisation.

For the above reasons, this patch has the kernel consistently sign
non-leaf functions and avoid signing leaf functions.

Considering a defconfig v6.2-rc3 kernel built with LLVM 15.0.6:

* The vmlinux is ~43KiB smaller:

  | [mark@lakrids:~/src/linux]% ls -al vmlinux-*
  | -rwxr-xr-x 1 mark mark 338547808 Jan 25 17:17 vmlinux-after
  | -rwxr-xr-x 1 mark mark 338591472 Jan 25 17:22 vmlinux-before

* The resulting Image is 64KiB smaller:

  | [mark@lakrids:~/src/linux]% ls -al Image-*
  | -rwxr-xr-x 1 mark mark 32702976 Jan 25 17:17 Image-after
  | -rwxr-xr-x 1 mark mark 32768512 Jan 25 17:22 Image-before

* There are ~400 fewer BTI gadgets:

  | [mark@lakrids:~/src/linux]% usekorg 12.1.0 aarch64-linux-objdump -d vmlinux-before 2> /dev/null | grep -ow 'paciasp\|bti\sc\?' | sort | uniq -c
  |    1219 bti     c
  |   61982 paciasp

  | [mark@lakrids:~/src/linux]% usekorg 12.1.0 aarch64-linux-objdump -d vmlinux-after 2> /dev/null | grep -ow 'paciasp\|bti\sc\?' | sort | uniq -c
  |   10099 bti     c
  |   52699 paciasp

  Which is +8880 BTIs, and -9283 PACIASPs, for -403 unnecessary BTI
  gadgets.  While this is relatively small relative to the total,
  distinguishing the two cases will make it easier to analyse and reduce
  this set further in future.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/Makefile | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index e176eb76345b5..ab1f12b4f339a 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -63,30 +63,18 @@ 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-y += $(call cc-option,-mbranch-protection=none)
-
-ifeq ($(CONFIG_ARM64_PTR_AUTH_KERNEL),y)
-branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
-# We enable additional protection for leaf functions as there is some
-# narrow potential for ROP protection benefits and no substantial
-# performance impact has been observed.
-PACRET-y := pac-ret+leaf
-
-# Using a shadow call stack in leaf functions is too costly, so avoid PAC there
-# as well when we may be patching PAC into SCS
-PACRET-$(CONFIG_UNWIND_PATCH_PAC_INTO_SCS) := pac-ret
-
 ifeq ($(CONFIG_ARM64_BTI_KERNEL),y)
-branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=$(PACRET-y)+bti
+  KBUILD_CFLAGS += -mbranch-protection=pac-ret+bti
+else ifeq ($(CONFIG_ARM64_PTR_AUTH_KERNEL),y)
+  ifeq ($(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET),y)
+    KBUILD_CFLAGS += -mbranch-protection=pac-ret
+  else
+    KBUILD_CFLAGS += -msign-return-address=non-leaf
+  endif
 else
-branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=$(PACRET-y)
-endif
+  KBUILD_CFLAGS += $(call cc-option,-mbranch-protection=none)
 endif
 
-KBUILD_CFLAGS += $(branch-prot-flags-y)
-
 # Tell the assembler to support instructions from the latest target
 # architecture.
 #
-- 
2.30.2


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

Re: [PATCH 2/2] arm64: pauth: don't sign leaf functions

[Ard Biesheuvel]

On Wed, 25 Jan 2023 at 19:22, Mark Rutland <mark.rutland@arm.com> wrote:
>
> Currently, when CONFIG_ARM64_PTR_AUTH_KERNEL=y (and
> CONFIG_UNWIND_PATCH_PAC_INTO_SCS=n), we enable pointer authentication
> for all functions, including leaf functions. This isn't necessary, and
> is unfortunate for a few reasons:
>
> * Any PACIASP instruction is implicitly a `BTI C` landing pad, and
>   forcing the addition of a PACIASP in every function introduces a
>   larger set of BTI gadgets than is necessary.
>
> * The PACIASP and AUTIASP instructions make leaf functions larger than
>   necessary, bloating the kernel Image. For a defconfig v6.2-rc3 kernel,
>   this appears to add ~64KiB relative to not signing leaf functions,
>   which is unfortunate but not entirely onerous.
>
> * The PACIASP and AUTIASP instructions potentially make leaf functions
>   more expensive in terms of performance and/or power. For many trivial
>   leaf functions, this is clearly unnecessary, e.g.
>
>   | <arch_local_save_flags>:
>   |        d503233f        paciasp
>   |        d53b4220        mrs     x0, daif
>   |        d50323bf        autiasp
>   |        d65f03c0        ret
>
>   | <calibration_delay_done>:
>   |        d503233f        paciasp
>   |        d50323bf        autiasp
>   |        d65f03c0        ret
>   |        d503201f        nop
>
> * When CONFIG_UNWIND_PATCH_PAC_INTO_SCS=y we disable pointer
>   authentication for leaf functions, so clearly this is not functionally
>   necessary, indicates we have an inconsistent threat model, and
>   convolutes the Makefile logic.
>
> We've used pointer authentication in leave functions since the

leaf

> introduction of in-kernel pointer authentication in commit:
>
>   74afda4016a7437e ("arm64: compile the kernel with ptrauth return address signing")
>
> ... but at the time we had no rationale for signing leaf functions.
>
> Subsequently, we considered avoiding signing leaf functions:
>
>   https://lore.kernel.org/linux-arm-kernel/1586856741-26839-1-git-send-email-amit.kachhap@arm.com/
>   https://lore.kernel.org/linux-arm-kernel/1588149371-20310-1-git-send-email-amit.kachhap@arm.com/
>
> ... however at the time we didn't have an abundance of reasons to avoid
> signing leaf functions as above (e.g. the BTI case), we had no hardware
> to make performance measurements, and it was reasoned that this gave
> some level of protection against a limited set of code-reuse gadgets
> which would fall through to a RET. We documented this in commit:
>
>   717b938e22f8dbf0 ("arm64: Document why we enable PAC support for leaf functions")
>
> Notably, this was before we supported any forward-edge CFI scheme (e.g.
> Arm BTI, or Clang CFI/kCFI), which would prevent jumping into the middle
> of a function.
>
> In addition, even with signing forced for leaf functions, AUTIASP may be
> placed before a number of instructions which might constitute such a
> gadget, e.g.
>
> | <user_regs_reset_single_step>:
> |        f9400022        ldr     x2, [x1]
> |        d503233f        paciasp
> |        d50323bf        autiasp
> |        f9408401        ldr     x1, [x0, #264]
> |        720b005f        tst     w2, #0x200000
> |        b26b0022        orr     x2, x1, #0x200000
> |        926af821        and     x1, x1, #0xffffffffffdfffff
> |        9a820021        csel    x1, x1, x2, eq  // eq = none
> |        f9008401        str     x1, [x0, #264]
> |        d65f03c0        ret
>
> | <fpsimd_cpu_dead>:
> |        2a0003e3        mov     w3, w0
> |        9000ff42        adrp    x2, ffff800009ffd000 <xen_dynamic_chip+0x48>
> |        9120e042        add     x2, x2, #0x838
> |        52800000        mov     w0, #0x0                        // #0
> |        d503233f        paciasp
> |        f000d041        adrp    x1, ffff800009a20000 <this_cpu_vector>
> |        d50323bf        autiasp
> |        9102c021        add     x1, x1, #0xb0
> |        f8635842        ldr     x2, [x2, w3, uxtw #3]
> |        f821685f        str     xzr, [x2, x1]
> |        d65f03c0        ret
> |        d503201f        nop
>
> So generally, trying to use AUTIASP to detect such gadgetization is not
> robust, and this is dealt with far better by forward-edge CFI (which is
> designed to prevent such cases). We should bite the buller and stop

bullet

> pretending that AUTIASP is a mitigation for such forward-edge
> gadgetisation.
>

Nit: this has an 's' whereas the previous occurrence had a 'z'

> For the above reasons, this patch has the kernel consistently sign
> non-leaf functions and avoid signing leaf functions.
>
> Considering a defconfig v6.2-rc3 kernel built with LLVM 15.0.6:
>
> * The vmlinux is ~43KiB smaller:
>
>   | [mark@lakrids:~/src/linux]% ls -al vmlinux-*
>   | -rwxr-xr-x 1 mark mark 338547808 Jan 25 17:17 vmlinux-after
>   | -rwxr-xr-x 1 mark mark 338591472 Jan 25 17:22 vmlinux-before
>
> * The resulting Image is 64KiB smaller:
>
>   | [mark@lakrids:~/src/linux]% ls -al Image-*
>   | -rwxr-xr-x 1 mark mark 32702976 Jan 25 17:17 Image-after
>   | -rwxr-xr-x 1 mark mark 32768512 Jan 25 17:22 Image-before
>
> * There are ~400 fewer BTI gadgets:
>
>   | [mark@lakrids:~/src/linux]% usekorg 12.1.0 aarch64-linux-objdump -d vmlinux-before 2> /dev/null | grep -ow 'paciasp\|bti\sc\?' | sort | uniq -c
>   |    1219 bti     c
>   |   61982 paciasp
>
>   | [mark@lakrids:~/src/linux]% usekorg 12.1.0 aarch64-linux-objdump -d vmlinux-after 2> /dev/null | grep -ow 'paciasp\|bti\sc\?' | sort | uniq -c
>   |   10099 bti     c
>   |   52699 paciasp
>
>   Which is +8880 BTIs, and -9283 PACIASPs, for -403 unnecessary BTI
>   gadgets.  While this is relatively small relative to the total,
>   distinguishing the two cases will make it easier to analyse and reduce
>   this set further in future.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Will Deacon <will@kernel.org>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  arch/arm64/Makefile | 28 ++++++++--------------------
>  1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index e176eb76345b5..ab1f12b4f339a 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -63,30 +63,18 @@ 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-y += $(call cc-option,-mbranch-protection=none)
> -
> -ifeq ($(CONFIG_ARM64_PTR_AUTH_KERNEL),y)
> -branch-prot-flags-$(CONFIG_CC_HAS_SIGN_RETURN_ADDRESS) := -msign-return-address=all
> -# We enable additional protection for leaf functions as there is some
> -# narrow potential for ROP protection benefits and no substantial
> -# performance impact has been observed.
> -PACRET-y := pac-ret+leaf
> -
> -# Using a shadow call stack in leaf functions is too costly, so avoid PAC there
> -# as well when we may be patching PAC into SCS
> -PACRET-$(CONFIG_UNWIND_PATCH_PAC_INTO_SCS) := pac-ret
> -
>  ifeq ($(CONFIG_ARM64_BTI_KERNEL),y)
> -branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET_BTI) := -mbranch-protection=$(PACRET-y)+bti
> +  KBUILD_CFLAGS += -mbranch-protection=pac-ret+bti
> +else ifeq ($(CONFIG_ARM64_PTR_AUTH_KERNEL),y)
> +  ifeq ($(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET),y)
> +    KBUILD_CFLAGS += -mbranch-protection=pac-ret
> +  else
> +    KBUILD_CFLAGS += -msign-return-address=non-leaf
> +  endif
>  else
> -branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=$(PACRET-y)
> -endif
> +  KBUILD_CFLAGS += $(call cc-option,-mbranch-protection=none)
>  endif
>
> -KBUILD_CFLAGS += $(branch-prot-flags-y)
> -
>  # Tell the assembler to support instructions from the latest target
>  # architecture.
>  #
> --
> 2.30.2
>

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

Re: [PATCH 2/2] arm64: pauth: don't sign leaf functions

[Mark Rutland]

On Thu, Jan 26, 2023 at 09:40:33AM +0100, Ard Biesheuvel wrote:
> On Wed, 25 Jan 2023 at 19:22, Mark Rutland <mark.rutland@arm.com> wrote:
> > We've used pointer authentication in leave functions since the
> 
> leaf

Thanks; fixed locally.

[...]

> > So generally, trying to use AUTIASP to detect such gadgetization is not
> > robust, and this is dealt with far better by forward-edge CFI (which is
> > designed to prevent such cases). We should bite the buller and stop
> 
> bullet

Thanks; fixed locally.

> > pretending that AUTIASP is a mitigation for such forward-edge
> > gadgetisation.
> >
> 
> Nit: this has an 's' whereas the previous occurrence had a 'z'

Thanks; I've made those both use 'z'.

[...]

> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Will Deacon <will@kernel.org>
> 
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Thanks!

Mark.

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

Re: [PATCH 2/2] arm64: pauth: don't sign leaf functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 344 bytes --]

On Wed, Jan 25, 2023 at 06:22:01PM +0000, Mark Rutland wrote:

> Currently, when CONFIG_ARM64_PTR_AUTH_KERNEL=y (and
> CONFIG_UNWIND_PATCH_PAC_INTO_SCS=n), we enable pointer authentication
> for all functions, including leaf functions. This isn't necessary, and
> is unfortunate for a few reasons:

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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