[PATCH 00/13] arm64: extable: remove anonymous out-of-line fixups

[PATCH 00/13] arm64: extable: remove anonymous out-of-line fixups

[Mark Rutland]

We recently realised that out-of-line extable fixups cause a number of problems
for backtracing (mattering both for developers and for RELIABLE_STACKTRACE and
LIVEPATCH). Dmitry spotted a confusing backtrace, which we identified was due
to problems with unwinding fixups, as summarized in:

  https://lore.kernel.org/linux-arm-kernel/20210927171812.GB9201@C02TD0UTHF1T.local/

The gist is that while backtracing through a fixup, the fixup gets symmbolized
as an offset from the nearest prior symbol (which happens to be
`__entry_tramp_text_end`), and we the backtrace misses the function that was
being fixed up (because the fixup handling adjusts the PC, then the fixup does
a direct branch back to the original function). We can't reliably map from an
arbitrary PC in the fixup text back to the original function.

The way we create fixups is a bit unfortunate: most fixups are generated from
common templates, and only differ in register to be poked and the address to
branch back to, leading to redundant copies of the same logic that must pollute
Since the fixups are all written in assembly, and duplicated for each fixup
site, we can only perform very simple fixups, and can't handle any complex
triage that we might need for some exceptions (e.g. MTE faults).

This series address these concerns by getting rid of the out-of-line anonymous
fixup logic:

* For plain assembly functions, we move the fixup into the body of
  the function, after the usual return, as we already do for our cache
  routines. This simplifies the source code, and only adds a handful of
  instructions to the main body of `.text`.

  This is handled by the first three patches, which I think are trivial and
  could be queued regardless of the rest of the series.

* For inline assembly, we add specialised handlers which run in exception
  context to update registers, then adjust the PC *within* the faulting
  function. This requires some new glue to capture the handler and metadata in
  struct exception_table_entry (costing 32 bits per fixup), but for any
  non-trivial fixup (which is all of the inline asm cases), this removes at
  least two instructions of out-of-line fixup.

As the fixups are now handled from C code in exception context, we can more
easily extend these in future with more complex triage if necessary.

Overall, this doesn't have an appreciable impact on Image size (in local
testing the size of the Image was identical before/after), but does shift the
boundary between .text and .ordata, making .text smaller and .rodata bigger.
.text somewhat while growing .rodata somewhat. 

I've tested this with both GCC and clang (including with clang CFI), and
everything is working as expected.

Other than changes to backtracing, there should be no functional change as a
result of this series.

Thanks
Mark.

Mark Rutland (13):
  arm64: lib: __arch_clear_user(): fold fixups into body
  arm64: lib: __arch_copy_from_user(): fold fixups into body
  arm64: lib: __arch_copy_to_user(): fold fixups into body
  arm64: kvm: use kvm_exception_table_entry
  arm64: factor out GPR numbering helpers
  arm64: gpr-num: support W registers
  arm64: extable: consolidate definitions
  arm64: extable: make fixup_exception() return bool
  arm64: extable: use `ex` for `exception_table_entry`
  arm64: extable: add `type` and `data` fields
  arm64: extable: add a dedicated uaccess handler
  arm64: extable: add load_unaligned_zeropad() handler
  arm64: vmlinux.lds.S: remove `.fixup` section

 arch/arm64/include/asm/asm-extable.h    | 95 +++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/asm-uaccess.h    |  7 ++-
 arch/arm64/include/asm/assembler.h      | 29 +---------
 arch/arm64/include/asm/extable.h        | 23 +++++---
 arch/arm64/include/asm/futex.h          | 25 +++------
 arch/arm64/include/asm/gpr-num.h        | 26 +++++++++
 arch/arm64/include/asm/kvm_asm.h        |  7 +--
 arch/arm64/include/asm/sysreg.h         | 25 +++------
 arch/arm64/include/asm/uaccess.h        | 26 ++-------
 arch/arm64/include/asm/word-at-a-time.h | 21 ++------
 arch/arm64/kernel/armv8_deprecated.c    | 12 ++---
 arch/arm64/kernel/traps.c               |  9 +---
 arch/arm64/kernel/vmlinux.lds.S         |  1 -
 arch/arm64/kvm/hyp/include/hyp/switch.h | 10 ++--
 arch/arm64/lib/clear_user.S             |  9 ++--
 arch/arm64/lib/copy_from_user.S         |  7 +--
 arch/arm64/lib/copy_to_user.S           |  7 +--
 arch/arm64/mm/extable.c                 | 85 +++++++++++++++++++++++++----
 arch/arm64/net/bpf_jit_comp.c           |  9 ++--
 scripts/sorttable.c                     | 30 +++++++++++
 20 files changed, 306 insertions(+), 157 deletions(-)
 create mode 100644 arch/arm64/include/asm/asm-extable.h
 create mode 100644 arch/arm64/include/asm/gpr-num.h

-- 
2.11.0


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

[PATCH 01/13] arm64: lib: __arch_clear_user(): fold fixups into body

[Mark Rutland]

Like other functions, __arch_clear_user() places its exception fixups in
the `.fixup` section without any clear association with
__arch_clear_user() itself. If we backtrace the fixup code, it will be
symbolized as an offset from the nearest prior symbol, which happens to
be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_clear_user() itself will be missing from the backtrace.

This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.

To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_clear_user().

Inline assembly will be dealt with in subsequent patches.

Other than the improved backtracing, 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: James Morse <james.morse@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/lib/clear_user.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index a7efb2ad2a1c..dac13df4a1ed 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -45,13 +45,10 @@ USER(9f, sttrh	wzr, [x0])
 USER(7f, sttrb	wzr, [x2, #-1])
 5:	mov	x0, #0
 	ret
-SYM_FUNC_END(__arch_clear_user)
-EXPORT_SYMBOL(__arch_clear_user)
 
-	.section .fixup,"ax"
-	.align	2
 7:	sub	x0, x2, #5	// Adjust for faulting on the final byte...
 8:	add	x0, x0, #4	// ...or the second word of the 4-7 byte case
 9:	sub	x0, x2, x0
 	ret
-	.previous
+SYM_FUNC_END(__arch_clear_user)
+EXPORT_SYMBOL(__arch_clear_user)
-- 
2.11.0


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

[PATCH 02/13] arm64: lib: __arch_copy_from_user(): fold fixups into body

[Mark Rutland]

Like other functions, __arch_copy_from_user() places its exception
fixups in the `.fixup` section without any clear association with
__arch_copy_from_user() itself. If we backtrace the fixup code, it will
be symbolized as an offset from the nearest prior symbol, which happens
to be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_copy_from_user() itself will be missing from the backtrace.

This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.

To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_copy_from_user().

Inline assembly will be dealt with in subsequent patches.

Other than the improved backtracing, 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: James Morse <james.morse@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/lib/copy_from_user.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
index 2cf999e41d30..f50ba67a73b4 100644
--- a/arch/arm64/lib/copy_from_user.S
+++ b/arch/arm64/lib/copy_from_user.S
@@ -60,11 +60,7 @@ SYM_FUNC_START(__arch_copy_from_user)
 #include "copy_template.S"
 	mov	x0, #0				// Nothing to copy
 	ret
-SYM_FUNC_END(__arch_copy_from_user)
-EXPORT_SYMBOL(__arch_copy_from_user)
 
-	.section .fixup,"ax"
-	.align	2
 9997:	cmp	dst, dstin
 	b.ne	9998f
 	// Before being absolutely sure we couldn't copy anything, try harder
@@ -72,4 +68,5 @@ USER(9998f, ldtrb tmp1w, [srcin])
 	strb	tmp1w, [dst], #1
 9998:	sub	x0, end, dst			// bytes not copied
 	ret
-	.previous
+SYM_FUNC_END(__arch_copy_from_user)
+EXPORT_SYMBOL(__arch_copy_from_user)
-- 
2.11.0


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

[PATCH 03/13] arm64: lib: __arch_copy_to_user(): fold fixups into body

[Mark Rutland]

Like other functions, __arch_copy_to_user() places its exception fixups
in the `.fixup` section without any clear association with
__arch_copy_to_user() itself. If we backtrace the fixup code, it will be
symbolized as an offset from the nearest prior symbol, which happens to
be `__entry_tramp_text_end`. Further, since the PC adjustment for the
fixup is akin to a direct branch rather than a function call,
__arch_copy_to_user() itself will be missing from the backtrace.

This is confusing and hinders debugging. In general this pattern will
also be problematic for CONFIG_LIVEPATCH, since fixups often return to
their associated function, but this isn't accurately captured in the
stacktrace.

To solve these issues for assembly functions, we must move fixups into
the body of the functions themselves, after the usual fast-path returns.
This patch does so for __arch_copy_to_user().

Inline assembly will be dealt with in subsequent patches.

Other than the improved backtracing, 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: James Morse <james.morse@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/lib/copy_to_user.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
index 9f380eecf653..2afc27bc0681 100644
--- a/arch/arm64/lib/copy_to_user.S
+++ b/arch/arm64/lib/copy_to_user.S
@@ -59,11 +59,7 @@ SYM_FUNC_START(__arch_copy_to_user)
 #include "copy_template.S"
 	mov	x0, #0
 	ret
-SYM_FUNC_END(__arch_copy_to_user)
-EXPORT_SYMBOL(__arch_copy_to_user)
 
-	.section .fixup,"ax"
-	.align	2
 9997:	cmp	dst, dstin
 	b.ne	9998f
 	// Before being absolutely sure we couldn't copy anything, try harder
@@ -72,4 +68,5 @@ USER(9998f, sttrb tmp1w, [dst])
 	add	dst, dst, #1
 9998:	sub	x0, end, dst			// bytes not copied
 	ret
-	.previous
+SYM_FUNC_END(__arch_copy_to_user)
+EXPORT_SYMBOL(__arch_copy_to_user)
-- 
2.11.0


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

[PATCH 04/13] arm64: kvm: use kvm_exception_table_entry

[Mark Rutland]

In subsequent patches we'll alter `struct exception_table_entry`, adding
fields that are not needed for KVM exception fixups.

In preparation for this, migrate KVM to its own `struct
kvm_exception_table_entry`, which is identical to the current format of
`struct exception_table_entry`. Comments are updated accordingly.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/kvm_asm.h        |  7 ++++---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 10 +++++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index e86045ac43ba..6486b1db268e 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -263,9 +263,10 @@ extern u64 __kvm_get_mdcr_el2(void);
 
 /*
  * KVM extable for unexpected exceptions.
- * In the same format _asm_extable, but output to a different section so that
- * it can be mapped to EL2. The KVM version is not sorted. The caller must
- * ensure:
+ * Create a struct kvm_exception_table_entry output to a section that can be
+ * mapped by EL2. The table is not sorted.
+ *
+ * The caller must ensure:
  * x18 has the hypervisor value to allow any Shadow-Call-Stack instrumented
  * code to write to it, and that SPSR_EL2 and ELR_EL2 are restored by the fixup.
  */
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index a0e78a6027be..d5a47b93ef9b 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -30,8 +30,12 @@
 #include <asm/processor.h>
 #include <asm/thread_info.h>
 
-extern struct exception_table_entry __start___kvm_ex_table;
-extern struct exception_table_entry __stop___kvm_ex_table;
+struct kvm_exception_table_entry {
+	int insn, fixup;
+};
+
+extern struct kvm_exception_table_entry __start___kvm_ex_table;
+extern struct kvm_exception_table_entry __stop___kvm_ex_table;
 
 /* Check whether the FP regs were dirtied while in the host-side run loop: */
 static inline bool update_fp_enabled(struct kvm_vcpu *vcpu)
@@ -510,7 +514,7 @@ static inline void __kvm_unexpected_el2_exception(void)
 {
 	extern char __guest_exit_panic[];
 	unsigned long addr, fixup;
-	struct exception_table_entry *entry, *end;
+	struct kvm_exception_table_entry *entry, *end;
 	unsigned long elr_el2 = read_sysreg(elr_el2);
 
 	entry = &__start___kvm_ex_table;
-- 
2.11.0


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

[PATCH 05/13] arm64: factor out GPR numbering helpers

[Mark Rutland]

In <asm/sysreg.h> we have macros to convert the names of general purpose
registers (GPRs) into integer constants, which we use to manually build
the encoding for `MRS` and `MSR` instructions where we can't rely on the
assembler to do so for us.

In subsequent patches we'll need to map the same GPR names to integer
constants so that we can use this to build metadata for exception
fixups.

So that the we can use the mappings elsewhere, factor out the
definitions into a new <asm/gpr-num.h> header, renaming the definitions
to align with this "GPR num" naming for clarity.

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: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/gpr-num.h | 22 ++++++++++++++++++++++
 arch/arm64/include/asm/sysreg.h  | 25 ++++++++-----------------
 2 files changed, 30 insertions(+), 17 deletions(-)
 create mode 100644 arch/arm64/include/asm/gpr-num.h

diff --git a/arch/arm64/include/asm/gpr-num.h b/arch/arm64/include/asm/gpr-num.h
new file mode 100644
index 000000000000..f936aa34dc63
--- /dev/null
+++ b/arch/arm64/include/asm/gpr-num.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GPR_NUM_H
+#define __ASM_GPR_NUM_H
+
+#ifdef __ASSEMBLY__
+
+	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
+	.equ	.L__gpr_num_x\num, \num
+	.endr
+	.equ	.L__gpr_num_xzr, 31
+
+#else /* __ASSEMBLY__ */
+
+#define __DEFINE_ASM_GPR_NUMS					\
+"	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n" \
+"	.equ	.L__gpr_num_x\\num, \\num\n"			\
+"	.endr\n"						\
+"	.equ	.L__gpr_num_xzr, 31\n"
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_GPR_NUM_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index b268082d67ed..58f6e669dab4 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -13,6 +13,8 @@
 #include <linux/stringify.h>
 #include <linux/kasan-tags.h>
 
+#include <asm/gpr-num.h>
+
 /*
  * ARMv8 ARM reserves the following encoding for system registers:
  * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
@@ -1192,17 +1194,12 @@
 
 #ifdef __ASSEMBLY__
 
-	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
-	.equ	.L__reg_num_x\num, \num
-	.endr
-	.equ	.L__reg_num_xzr, 31
-
 	.macro	mrs_s, rt, sreg
-	 __emit_inst(0xd5200000|(\sreg)|(.L__reg_num_\rt))
+	 __emit_inst(0xd5200000|(\sreg)|(.L__gpr_num_\rt))
 	.endm
 
 	.macro	msr_s, sreg, rt
-	__emit_inst(0xd5000000|(\sreg)|(.L__reg_num_\rt))
+	__emit_inst(0xd5000000|(\sreg)|(.L__gpr_num_\rt))
 	.endm
 
 #else
@@ -1211,22 +1208,16 @@
 #include <linux/types.h>
 #include <asm/alternative.h>
 
-#define __DEFINE_MRS_MSR_S_REGNUM				\
-"	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n" \
-"	.equ	.L__reg_num_x\\num, \\num\n"			\
-"	.endr\n"						\
-"	.equ	.L__reg_num_xzr, 31\n"
-
 #define DEFINE_MRS_S						\
-	__DEFINE_MRS_MSR_S_REGNUM				\
+	__DEFINE_ASM_GPR_NUMS					\
 "	.macro	mrs_s, rt, sreg\n"				\
-	__emit_inst(0xd5200000|(\\sreg)|(.L__reg_num_\\rt))	\
+	__emit_inst(0xd5200000|(\\sreg)|(.L__gpr_num_\\rt))	\
 "	.endm\n"
 
 #define DEFINE_MSR_S						\
-	__DEFINE_MRS_MSR_S_REGNUM				\
+	__DEFINE_ASM_GPR_NUMS					\
 "	.macro	msr_s, sreg, rt\n"				\
-	__emit_inst(0xd5000000|(\\sreg)|(.L__reg_num_\\rt))	\
+	__emit_inst(0xd5000000|(\\sreg)|(.L__gpr_num_\\rt))	\
 "	.endm\n"
 
 #define UNDEFINE_MRS_S						\
-- 
2.11.0


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

[PATCH 06/13] arm64: gpr-num: support W registers

[Mark Rutland]

In subsequent patches we'll want to map W registers to their register
numbers. Update gpr-num.h so that we can do this.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/gpr-num.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/gpr-num.h b/arch/arm64/include/asm/gpr-num.h
index f936aa34dc63..05da4a7c5788 100644
--- a/arch/arm64/include/asm/gpr-num.h
+++ b/arch/arm64/include/asm/gpr-num.h
@@ -6,16 +6,20 @@
 
 	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
 	.equ	.L__gpr_num_x\num, \num
+	.equ	.L__gpr_num_w\num, \num
 	.endr
 	.equ	.L__gpr_num_xzr, 31
+	.equ	.L__gpr_num_wzr, 31
 
 #else /* __ASSEMBLY__ */
 
 #define __DEFINE_ASM_GPR_NUMS					\
 "	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n" \
 "	.equ	.L__gpr_num_x\\num, \\num\n"			\
+"	.equ	.L__gpr_num_w\\num, \\num\n"			\
 "	.endr\n"						\
-"	.equ	.L__gpr_num_xzr, 31\n"
+"	.equ	.L__gpr_num_xzr, 31\n"				\
+"	.equ	.L__gpr_num_wzr, 31\n"
 
 #endif /* __ASSEMBLY__ */
 
-- 
2.11.0


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

[PATCH 07/13] arm64: extable: consolidate definitions

[Mark Rutland]

In subsequent patches we'll alter the structure and usage of struct
exception_table_entry. For inline assembly, we create these using the
`_ASM_EXTABLE()` CPP macro defined in <asm/uaccess.h>, and for plain
assembly code we use the `_asm_extable()` GAS macro defined in
<asm/assembler.h>, which are largely identical save for different
escaping and stringification requirements.

This patch moves the common definitions to a new <asm/asm-extable.h>
header, so that it's easier to keep the two in-sync, and to remove the
implication that these are only used for uaccess helpers (as e.g.
load_unaligned_zeropad() is only used on kernel memory, and depends upon
`_ASM_EXTABLE()`.

At the same time, a few minor modifications are made for clarity and in
preparation for subsequent patches:

* The structure creation is factored out into an `__ASM_EXTABLE_RAW()`
  macro. This will make it easier to support different fixup variants in
  subsequent patches without needing to update all users of
  `_ASM_EXTABLE()`, and makes it easier to see tha the CPP and GAS
  variants of the macros are structurally identical.

  For the CPP macro, the stringification of fields is left to the
  wrapper macro, `_ASM_EXTABLE()`, as in subsequent patches it will be
  necessary to stringify fields in wrapper macros to safely concatenate
  strings which cannot be token-pasted together in CPP.

* The fields of the structure are created separately on their own lines.
  This will make it easier to add/remove/modify individual fields
  clearly.

* Additional parentheses are added around the use of macro arguments in
  field definitions to avoid any potential problems with evaluation due
  to operator precedence, and to make errors upon misuse clearer.

* USER() is moved into <asm/asm-uaccess.h>, as it is not required by all
  assembly code, and is already refered to by comments in that file.

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: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/asm-extable.h | 48 ++++++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/asm-uaccess.h |  7 +++++-
 arch/arm64/include/asm/assembler.h   | 29 ++--------------------
 arch/arm64/include/asm/uaccess.h     |  7 +-----
 arch/arm64/lib/clear_user.S          |  2 +-
 5 files changed, 58 insertions(+), 35 deletions(-)
 create mode 100644 arch/arm64/include/asm/asm-extable.h

diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
new file mode 100644
index 000000000000..986b4c0d4792
--- /dev/null
+++ b/arch/arm64/include/asm/asm-extable.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_ASM_EXTABLE_H
+#define __ASM_ASM_EXTABLE_H
+
+#ifdef __ASSEMBLY__
+
+#define __ASM_EXTABLE_RAW(insn, fixup)		\
+	.pushsection	__ex_table, "a";	\
+	.align		3;			\
+	.long		((insn) - .);		\
+	.long		((fixup) - .);		\
+	.popsection;
+
+/*
+ * Create an exception table entry for `insn`, which will branch to `fixup`
+ * when an unhandled fault is taken.
+ */
+	.macro		_asm_extable, insn, fixup
+	__ASM_EXTABLE_RAW(\insn, \fixup)
+	.endm
+
+/*
+ * Create an exception table entry for `insn` if `fixup` is provided. Otherwise
+ * do nothing.
+ */
+	.macro		_cond_extable, insn, fixup
+	.ifnc		\fixup,
+	_asm_extable	\insn, \fixup
+	.endif
+	.endm
+
+#else /* __ASSEMBLY__ */
+
+#include <linux/stringify.h>
+
+#define __ASM_EXTABLE_RAW(insn, fixup)		\
+	".pushsection	__ex_table, \"a\"\n"	\
+	".align		3\n"			\
+	".long		((" insn ") - .)\n"	\
+	".long		((" fixup ") - .)\n"	\
+	".popsection\n"
+
+#define _ASM_EXTABLE(insn, fixup) \
+	__ASM_EXTABLE_RAW(#insn, #fixup)
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_ASM_EXTABLE_H */
diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h
index ccedf548dac9..0557af834e03 100644
--- a/arch/arm64/include/asm/asm-uaccess.h
+++ b/arch/arm64/include/asm/asm-uaccess.h
@@ -3,10 +3,11 @@
 #define __ASM_ASM_UACCESS_H
 
 #include <asm/alternative-macros.h>
+#include <asm/asm-extable.h>
+#include <asm/assembler.h>
 #include <asm/kernel-pgtable.h>
 #include <asm/mmu.h>
 #include <asm/sysreg.h>
-#include <asm/assembler.h>
 
 /*
  * User access enabling/disabling macros.
@@ -58,6 +59,10 @@ alternative_else_nop_endif
 	.endm
 #endif
 
+#define USER(l, x...)				\
+9999:	x;					\
+	_asm_extable	9999b, l
+
 /*
  * Generate the assembly for LDTR/STTR with exception table entries.
  * This is complicated as there is no post-increment or pair versions of the
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index bfa58409a4d4..ec67480d55fb 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -14,9 +14,10 @@
 
 #include <asm-generic/export.h>
 
-#include <asm/asm-offsets.h>
 #include <asm/alternative.h>
 #include <asm/asm-bug.h>
+#include <asm/asm-extable.h>
+#include <asm/asm-offsets.h>
 #include <asm/cpufeature.h>
 #include <asm/cputype.h>
 #include <asm/debug-monitors.h>
@@ -130,32 +131,6 @@ alternative_endif
 	.endm
 
 /*
- * Create an exception table entry for `insn`, which will branch to `fixup`
- * when an unhandled fault is taken.
- */
-	.macro		_asm_extable, insn, fixup
-	.pushsection	__ex_table, "a"
-	.align		3
-	.long		(\insn - .), (\fixup - .)
-	.popsection
-	.endm
-
-/*
- * Create an exception table entry for `insn` if `fixup` is provided. Otherwise
- * do nothing.
- */
-	.macro		_cond_extable, insn, fixup
-	.ifnc		\fixup,
-	_asm_extable	\insn, \fixup
-	.endif
-	.endm
-
-
-#define USER(l, x...)				\
-9999:	x;					\
-	_asm_extable	9999b, l
-
-/*
  * Register aliases.
  */
 lr	.req	x30		// link register
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 190b494e22ab..759019523b85 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -18,6 +18,7 @@
 #include <linux/kasan-checks.h>
 #include <linux/string.h>
 
+#include <asm/asm-extable.h>
 #include <asm/cpufeature.h>
 #include <asm/mmu.h>
 #include <asm/mte.h>
@@ -70,12 +71,6 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
 
 #define access_ok(addr, size)	__range_ok(addr, size)
 
-#define _ASM_EXTABLE(from, to)						\
-	"	.pushsection	__ex_table, \"a\"\n"			\
-	"	.align		3\n"					\
-	"	.long		(" #from " - .), (" #to " - .)\n"	\
-	"	.popsection\n"
-
 /*
  * User access enabling/disabling.
  */
diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index dac13df4a1ed..a671097131b7 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -4,7 +4,7 @@
  */
 
 #include <linux/linkage.h>
-#include <asm/assembler.h>
+#include <asm/asm-uaccess.h>
 
 	.text
 
-- 
2.11.0


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

[PATCH 08/13] arm64: extable: make fixup_exception() return bool

[Mark Rutland]

The return values of fixup_exception() and arm64_bpf_fixup_exception()
represent a boolean condition rather than an error code, so for clarity
it would be better to return `bool` rather than `int`.

This patch adjusts the code accordingly. While we're modifying the
prototype, we also remove the unnecessary `extern` keyword, so that this
won't look out of place when we make subsequent additions to the header.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/extable.h | 10 +++++-----
 arch/arm64/mm/extable.c          |  6 +++---
 arch/arm64/net/bpf_jit_comp.c    |  6 +++---
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/extable.h b/arch/arm64/include/asm/extable.h
index b15eb4a3e6b2..1859b9fd566f 100644
--- a/arch/arm64/include/asm/extable.h
+++ b/arch/arm64/include/asm/extable.h
@@ -32,16 +32,16 @@ static inline bool in_bpf_jit(struct pt_regs *regs)
 }
 
 #ifdef CONFIG_BPF_JIT
-int arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
+bool arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
 			      struct pt_regs *regs);
 #else /* !CONFIG_BPF_JIT */
 static inline
-int arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
-			      struct pt_regs *regs)
+bool arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
+			       struct pt_regs *regs)
 {
-	return 0;
+	return false;
 }
 #endif /* !CONFIG_BPF_JIT */
 
-extern int fixup_exception(struct pt_regs *regs);
+bool fixup_exception(struct pt_regs *regs);
 #endif
diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
index aa0060178343..3ebc738870f5 100644
--- a/arch/arm64/mm/extable.c
+++ b/arch/arm64/mm/extable.c
@@ -6,17 +6,17 @@
 #include <linux/extable.h>
 #include <linux/uaccess.h>
 
-int fixup_exception(struct pt_regs *regs)
+bool fixup_exception(struct pt_regs *regs)
 {
 	const struct exception_table_entry *fixup;
 
 	fixup = search_exception_tables(instruction_pointer(regs));
 	if (!fixup)
-		return 0;
+		return false;
 
 	if (in_bpf_jit(regs))
 		return arm64_bpf_fixup_exception(fixup, regs);
 
 	regs->pc = (unsigned long)&fixup->fixup + fixup->fixup;
-	return 1;
+	return true;
 }
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 41c23f474ea6..956c841ef346 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -358,15 +358,15 @@ static void build_epilogue(struct jit_ctx *ctx)
 #define BPF_FIXUP_OFFSET_MASK	GENMASK(26, 0)
 #define BPF_FIXUP_REG_MASK	GENMASK(31, 27)
 
-int arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
-			      struct pt_regs *regs)
+bool arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
+			       struct pt_regs *regs)
 {
 	off_t offset = FIELD_GET(BPF_FIXUP_OFFSET_MASK, ex->fixup);
 	int dst_reg = FIELD_GET(BPF_FIXUP_REG_MASK, ex->fixup);
 
 	regs->regs[dst_reg] = 0;
 	regs->pc = (unsigned long)&ex->fixup - offset;
-	return 1;
+	return true;
 }
 
 /* For accesses to BTF pointers, add an entry to the exception table */
-- 
2.11.0


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

[PATCH 09/13] arm64: extable: use `ex` for `exception_table_entry`

[Mark Rutland]

Subsequent patches will extend `struct exception_table_entry` with more
fields, and the distinction between the entry and its `fixup` field will
become more important.

For clarity, let's consistently use `ex` to refer to refer to an entire
entry. In subsequent patches we'll use `fixup` to refer to the fixup
field specifically. This matches the naming convention used today in
arch/arm64/net/bpf_jit_comp.c.

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: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/mm/extable.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
index 3ebc738870f5..dba3d59f3eca 100644
--- a/arch/arm64/mm/extable.c
+++ b/arch/arm64/mm/extable.c
@@ -8,15 +8,15 @@
 
 bool fixup_exception(struct pt_regs *regs)
 {
-	const struct exception_table_entry *fixup;
+	const struct exception_table_entry *ex;
 
-	fixup = search_exception_tables(instruction_pointer(regs));
-	if (!fixup)
+	ex = search_exception_tables(instruction_pointer(regs));
+	if (!ex)
 		return false;
 
 	if (in_bpf_jit(regs))
-		return arm64_bpf_fixup_exception(fixup, regs);
+		return arm64_bpf_fixup_exception(ex, regs);
 
-	regs->pc = (unsigned long)&fixup->fixup + fixup->fixup;
+	regs->pc = (unsigned long)&ex->fixup + ex->fixup;
 	return true;
 }
-- 
2.11.0


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

[PATCH 10/13] arm64: extable: add `type` and `data` fields

[Mark Rutland]

Subsequent patches will add specialized handlers for fixups, in addition
to the simple PC fixup and BPF handlers we have today. In preparation,
this patch adds a new `type` field to struct exception_table_entry, and
uses this to distinguish the fixup and BPF cases. A `data` field is also
added so that subsequent patches can associate data specific to each
exception site (e.g. register numbers).

Handlers are named ex_handler_*() for consistency, following the exmaple
of x86. At the same time, get_ex_fixup() is split out into a helper so
that it can be used by other ex_handler_*() functions ins subsequent
patches.

This patch will increase the size of the exception tables, which will be
remedied by subsequent patches removing redundant fixup code. There
should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: James Morse <james.morse@arm.com>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
 arch/arm64/include/asm/extable.h     | 19 +++++++++++++++----
 arch/arm64/mm/extable.c              | 29 +++++++++++++++++++++++++----
 arch/arm64/net/bpf_jit_comp.c        |  7 +++++--
 scripts/sorttable.c                  | 30 ++++++++++++++++++++++++++++++
 5 files changed, 95 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
index 986b4c0d4792..5ee748edaef1 100644
--- a/arch/arm64/include/asm/asm-extable.h
+++ b/arch/arm64/include/asm/asm-extable.h
@@ -2,13 +2,19 @@
 #ifndef __ASM_ASM_EXTABLE_H
 #define __ASM_ASM_EXTABLE_H
 
+#define EX_TYPE_NONE			0
+#define EX_TYPE_FIXUP			1
+#define EX_TYPE_BPF			2
+
 #ifdef __ASSEMBLY__
 
-#define __ASM_EXTABLE_RAW(insn, fixup)		\
-	.pushsection	__ex_table, "a";	\
-	.align		3;			\
-	.long		((insn) - .);		\
-	.long		((fixup) - .);		\
+#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
+	.pushsection	__ex_table, "a";		\
+	.align		2;				\
+	.long		((insn) - .);			\
+	.long		((fixup) - .);			\
+	.short		(type);				\
+	.short		(data);				\
 	.popsection;
 
 /*
@@ -16,7 +22,7 @@
  * when an unhandled fault is taken.
  */
 	.macro		_asm_extable, insn, fixup
-	__ASM_EXTABLE_RAW(\insn, \fixup)
+	__ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0)
 	.endm
 
 /*
@@ -33,15 +39,17 @@
 
 #include <linux/stringify.h>
 
-#define __ASM_EXTABLE_RAW(insn, fixup)		\
-	".pushsection	__ex_table, \"a\"\n"	\
-	".align		3\n"			\
-	".long		((" insn ") - .)\n"	\
-	".long		((" fixup ") - .)\n"	\
+#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
+	".pushsection	__ex_table, \"a\"\n"		\
+	".align		2\n"				\
+	".long		((" insn ") - .)\n"		\
+	".long		((" fixup ") - .)\n"		\
+	".short		(" type ")\n"			\
+	".short		(" data ")\n"			\
 	".popsection\n"
 
 #define _ASM_EXTABLE(insn, fixup) \
-	__ASM_EXTABLE_RAW(#insn, #fixup)
+	__ASM_EXTABLE_RAW(#insn, #fixup, __stringify(EX_TYPE_FIXUP), "0")
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/arm64/include/asm/extable.h b/arch/arm64/include/asm/extable.h
index 1859b9fd566f..8b300dd28def 100644
--- a/arch/arm64/include/asm/extable.h
+++ b/arch/arm64/include/asm/extable.h
@@ -18,10 +18,21 @@
 struct exception_table_entry
 {
 	int insn, fixup;
+	short type, data;
 };
 
 #define ARCH_HAS_RELATIVE_EXTABLE
 
+#define swap_ex_entry_fixup(a, b, tmp, delta)		\
+do {							\
+	(a)->fixup = (b)->fixup + (delta);		\
+	(b)->fixup = (tmp).fixup - (delta);		\
+	(a)->type = (b)->type;				\
+	(b)->type = (tmp).type;				\
+	(a)->data = (b)->data;				\
+	(b)->data = (tmp).data;				\
+} while (0)
+
 static inline bool in_bpf_jit(struct pt_regs *regs)
 {
 	if (!IS_ENABLED(CONFIG_BPF_JIT))
@@ -32,12 +43,12 @@ static inline bool in_bpf_jit(struct pt_regs *regs)
 }
 
 #ifdef CONFIG_BPF_JIT
-bool arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
-			      struct pt_regs *regs);
+bool ex_handler_bpf(const struct exception_table_entry *ex,
+		    struct pt_regs *regs);
 #else /* !CONFIG_BPF_JIT */
 static inline
-bool arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
-			       struct pt_regs *regs)
+bool ex_handler_bpf(const struct exception_table_entry *ex,
+		    struct pt_regs *regs)
 {
 	return false;
 }
diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
index dba3d59f3eca..c2951b963335 100644
--- a/arch/arm64/mm/extable.c
+++ b/arch/arm64/mm/extable.c
@@ -6,6 +6,24 @@
 #include <linux/extable.h>
 #include <linux/uaccess.h>
 
+#include <asm/asm-extable.h>
+
+typedef bool (*ex_handler_t)(const struct exception_table_entry *,
+			     struct pt_regs *);
+
+static inline unsigned long
+get_ex_fixup(const struct exception_table_entry *ex)
+{
+	return ((unsigned long)&ex->fixup + ex->fixup);
+}
+
+static bool ex_handler_fixup(const struct exception_table_entry *ex,
+			     struct pt_regs *regs)
+{
+	regs->pc = get_ex_fixup(ex);
+	return true;
+}
+
 bool fixup_exception(struct pt_regs *regs)
 {
 	const struct exception_table_entry *ex;
@@ -14,9 +32,12 @@ bool fixup_exception(struct pt_regs *regs)
 	if (!ex)
 		return false;
 
-	if (in_bpf_jit(regs))
-		return arm64_bpf_fixup_exception(ex, regs);
+	switch (ex->type) {
+	case EX_TYPE_FIXUP:
+		return ex_handler_fixup(ex, regs);
+	case EX_TYPE_BPF:
+		return ex_handler_bpf(ex, regs);
+	}
 
-	regs->pc = (unsigned long)&ex->fixup + ex->fixup;
-	return true;
+	BUG();
 }
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 956c841ef346..7df7345e60d8 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -13,6 +13,7 @@
 #include <linux/printk.h>
 #include <linux/slab.h>
 
+#include <asm/asm-extable.h>
 #include <asm/byteorder.h>
 #include <asm/cacheflush.h>
 #include <asm/debug-monitors.h>
@@ -358,8 +359,8 @@ static void build_epilogue(struct jit_ctx *ctx)
 #define BPF_FIXUP_OFFSET_MASK	GENMASK(26, 0)
 #define BPF_FIXUP_REG_MASK	GENMASK(31, 27)
 
-bool arm64_bpf_fixup_exception(const struct exception_table_entry *ex,
-			       struct pt_regs *regs)
+bool ex_handler_bpf(const struct exception_table_entry *ex,
+		    struct pt_regs *regs)
 {
 	off_t offset = FIELD_GET(BPF_FIXUP_OFFSET_MASK, ex->fixup);
 	int dst_reg = FIELD_GET(BPF_FIXUP_REG_MASK, ex->fixup);
@@ -412,6 +413,8 @@ static int add_exception_handler(const struct bpf_insn *insn,
 	ex->fixup = FIELD_PREP(BPF_FIXUP_OFFSET_MASK, offset) |
 		    FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
 
+	ex->type = EX_TYPE_BPF;
+
 	ctx->exentry_idx++;
 	return 0;
 }
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index 6ee4fa882919..ee95bb47a50d 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)
 	}
 }
 
+static void arm64_sort_relative_table(char *extab_image, int image_size)
+{
+	int i = 0;
+
+	while (i < image_size) {
+		uint32_t *loc = (uint32_t *)(extab_image + i);
+
+		w(r(loc) + i, loc);
+		w(r(loc + 1) + i + 4, loc + 1);
+		/* Don't touch the fixup type or data */
+
+		i += sizeof(uint32_t) * 3;
+	}
+
+	qsort(extab_image, image_size / 12, 12, compare_relative_table);
+
+	i = 0;
+	while (i < image_size) {
+		uint32_t *loc = (uint32_t *)(extab_image + i);
+
+		w(r(loc) - i, loc);
+		w(r(loc + 1) - (i + 4), loc + 1);
+		/* Don't touch the fixup type or data */
+
+		i += sizeof(uint32_t) * 3;
+	}
+}
+
 static void x86_sort_relative_table(char *extab_image, int image_size)
 {
 	int i = 0;
@@ -343,6 +371,8 @@ static int do_file(char const *const fname, void *addr)
 		custom_sort = s390_sort_relative_table;
 		break;
 	case EM_AARCH64:
+		custom_sort = arm64_sort_relative_table;
+		break;
 	case EM_PARISC:
 	case EM_PPC:
 	case EM_PPC64:
-- 
2.11.0


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

[PATCH 11/13] arm64: extable: add a dedicated uaccess handler

[Mark Rutland]

For inline assembly, we place exception fixups out-of-line in the
`.fixup` section such that these are out of the way of the fast path.
This has a few drawbacks:

* Since the fixup code is anonymous, backtraces will symbolize fixups as
  offsets from the nearest prior symbol, currently
  `__entry_tramp_text_end`. This is confusing, and painful to debug
  without access to the relevant vmlinux.

* Since the exception handler adjusts the PC to execute the fixup, and
  the fixup uses a direct branch back into the function it fixes,
  backtraces of fixups miss the original function. This is confusing,
  and violates requirements for RELIABLE_STACKTRACE (and therefore
  LIVEPATCH).

* Inline assembly and associated fixups are generated from templates,
  and we have many copies of logically identical fixups which only
  differ in which specific registers are written to and which address is
  branched to at the end of the fixup. This is potentially wasteful of
  I-cache resources, and makes it hard to add additional logic to fixups
  without significant bloat.

This patch address all three concerns for inline uaccess fixups by
adding a dedicated exception handler which updates registers in
exception context and subsequent returns back into the function which
faulted, removing the need for fixups specialized to each faulting
instruction.

Other than backtracing, 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: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/asm-extable.h | 24 ++++++++++++++++++++++++
 arch/arm64/include/asm/futex.h       | 25 ++++++++-----------------
 arch/arm64/include/asm/uaccess.h     | 19 ++++---------------
 arch/arm64/kernel/armv8_deprecated.c | 12 +++---------
 arch/arm64/kernel/traps.c            |  9 ++-------
 arch/arm64/mm/extable.c              | 17 +++++++++++++++++
 6 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
index 5ee748edaef1..11209da19c62 100644
--- a/arch/arm64/include/asm/asm-extable.h
+++ b/arch/arm64/include/asm/asm-extable.h
@@ -5,6 +5,7 @@
 #define EX_TYPE_NONE			0
 #define EX_TYPE_FIXUP			1
 #define EX_TYPE_BPF			2
+#define EX_TYPE_UACCESS_ERR_ZERO	3
 
 #ifdef __ASSEMBLY__
 
@@ -37,8 +38,11 @@
 
 #else /* __ASSEMBLY__ */
 
+#include <linux/bits.h>
 #include <linux/stringify.h>
 
+#include <asm/gpr-num.h>
+
 #define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
 	".pushsection	__ex_table, \"a\"\n"		\
 	".align		2\n"				\
@@ -51,6 +55,26 @@
 #define _ASM_EXTABLE(insn, fixup) \
 	__ASM_EXTABLE_RAW(#insn, #fixup, __stringify(EX_TYPE_FIXUP), "0")
 
+#define EX_DATA_REG_ERR_SHIFT	0
+#define EX_DATA_REG_ERR		GENMASK(4, 0)
+#define EX_DATA_REG_ZERO_SHIFT	5
+#define EX_DATA_REG_ZERO	GENMASK(9, 5)
+
+#define EX_DATA_REG(reg, gpr)						\
+	"((.L__gpr_num_" #gpr ") << " __stringify(EX_DATA_REG_##reg##_SHIFT) ")"
+
+#define _ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, zero)		\
+	__DEFINE_ASM_GPR_NUMS						\
+	__ASM_EXTABLE_RAW(#insn, #fixup, 				\
+			  __stringify(EX_TYPE_UACCESS_ERR_ZERO),	\
+			  "("						\
+			    EX_DATA_REG(ERR, err) " | "			\
+			    EX_DATA_REG(ZERO, zero)			\
+			  ")")
+
+#define _ASM_EXTABLE_UACCESS_ERR(insn, fixup, err)			\
+	_ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, wzr)
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_ASM_EXTABLE_H */
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index 8e41faa37c69..bc06691d2062 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -25,19 +25,14 @@ do {									\
 "	cbz	%w0, 3f\n"						\
 "	sub	%w4, %w4, %w0\n"					\
 "	cbnz	%w4, 1b\n"						\
-"	mov	%w0, %w7\n"						\
+"	mov	%w0, %w6\n"						\
 "3:\n"									\
 "	dmb	ish\n"							\
-"	.pushsection .fixup,\"ax\"\n"					\
-"	.align	2\n"							\
-"4:	mov	%w0, %w6\n"						\
-"	b	3b\n"							\
-"	.popsection\n"							\
-	_ASM_EXTABLE(1b, 4b)						\
-	_ASM_EXTABLE(2b, 4b)						\
+	_ASM_EXTABLE_UACCESS_ERR(1b, 3b, %w0)				\
+	_ASM_EXTABLE_UACCESS_ERR(2b, 3b, %w0)				\
 	: "=&r" (ret), "=&r" (oldval), "+Q" (*uaddr), "=&r" (tmp),	\
 	  "+r" (loops)							\
-	: "r" (oparg), "Ir" (-EFAULT), "Ir" (-EAGAIN)			\
+	: "r" (oparg), "Ir" (-EAGAIN)					\
 	: "memory");							\
 	uaccess_disable_privileged();					\
 } while (0)
@@ -105,18 +100,14 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *_uaddr,
 "	cbz	%w3, 3f\n"
 "	sub	%w4, %w4, %w3\n"
 "	cbnz	%w4, 1b\n"
-"	mov	%w0, %w8\n"
+"	mov	%w0, %w7\n"
 "3:\n"
 "	dmb	ish\n"
 "4:\n"
-"	.pushsection .fixup,\"ax\"\n"
-"5:	mov	%w0, %w7\n"
-"	b	4b\n"
-"	.popsection\n"
-	_ASM_EXTABLE(1b, 5b)
-	_ASM_EXTABLE(2b, 5b)
+	_ASM_EXTABLE_UACCESS_ERR(1b, 4b, %w0)
+	_ASM_EXTABLE_UACCESS_ERR(2b, 4b, %w0)
 	: "+r" (ret), "=&r" (val), "+Q" (*uaddr), "=&r" (tmp), "+r" (loops)
-	: "r" (oldval), "r" (newval), "Ir" (-EFAULT), "Ir" (-EAGAIN)
+	: "r" (oldval), "r" (newval), "Ir" (-EAGAIN)
 	: "memory");
 	uaccess_disable_privileged();
 
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 759019523b85..9bc218991c5a 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -255,15 +255,9 @@ static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
 	asm volatile(							\
 	"1:	" load "	" reg "1, [%2]\n"			\
 	"2:\n"								\
-	"	.section .fixup, \"ax\"\n"				\
-	"	.align	2\n"						\
-	"3:	mov	%w0, %3\n"					\
-	"	mov	%1, #0\n"					\
-	"	b	2b\n"						\
-	"	.previous\n"						\
-	_ASM_EXTABLE(1b, 3b)						\
+	_ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %w0, %w1)			\
 	: "+r" (err), "=&r" (x)						\
-	: "r" (addr), "i" (-EFAULT))
+	: "r" (addr))
 
 #define __raw_get_mem(ldr, x, ptr, err)					\
 do {									\
@@ -332,14 +326,9 @@ do {									\
 	asm volatile(							\
 	"1:	" store "	" reg "1, [%2]\n"			\
 	"2:\n"								\
-	"	.section .fixup,\"ax\"\n"				\
-	"	.align	2\n"						\
-	"3:	mov	%w0, %3\n"					\
-	"	b	2b\n"						\
-	"	.previous\n"						\
-	_ASM_EXTABLE(1b, 3b)						\
+	_ASM_EXTABLE_UACCESS_ERR(1b, 2b, %w0)				\
 	: "+r" (err)							\
-	: "r" (x), "r" (addr), "i" (-EFAULT))
+	: "r" (x), "r" (addr))
 
 #define __raw_put_mem(str, x, ptr, err)					\
 do {									\
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 0e86e8b9cedd..6875a16b09d2 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -279,7 +279,7 @@ static void __init register_insn_emulation_sysctl(void)
 do {								\
 	uaccess_enable_privileged();				\
 	__asm__ __volatile__(					\
-	"	mov		%w3, %w7\n"			\
+	"	mov		%w3, %w6\n"			\
 	"0:	ldxr"B"		%w2, [%4]\n"			\
 	"1:	stxr"B"		%w0, %w1, [%4]\n"		\
 	"	cbz		%w0, 2f\n"			\
@@ -290,16 +290,10 @@ do {								\
 	"2:\n"							\
 	"	mov		%w1, %w2\n"			\
 	"3:\n"							\
-	"	.pushsection	 .fixup,\"ax\"\n"		\
-	"	.align		2\n"				\
-	"4:	mov		%w0, %w6\n"			\
-	"	b		3b\n"				\
-	"	.popsection"					\
-	_ASM_EXTABLE(0b, 4b)					\
-	_ASM_EXTABLE(1b, 4b)					\
+	_ASM_EXTABLE_UACCESS_ERR(0b, 3b, %w0)			\
+	_ASM_EXTABLE_UACCESS_ERR(1b, 3b, %w0)			\
 	: "=&r" (res), "+r" (data), "=&r" (temp), "=&r" (temp2)	\
 	: "r" ((unsigned long)addr), "i" (-EAGAIN),		\
-	  "i" (-EFAULT),					\
 	  "i" (__SWP_LL_SC_LOOPS)				\
 	: "memory");						\
 	uaccess_disable_privileged();				\
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index b03e383d944a..268a81a3006e 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -527,14 +527,9 @@ NOKPROBE_SYMBOL(do_ptrauth_fault);
 			"1:	" insn ", %1\n"			\
 			"	mov	%w0, #0\n"		\
 			"2:\n"					\
-			"	.pushsection .fixup,\"ax\"\n"	\
-			"	.align	2\n"			\
-			"3:	mov	%w0, %w2\n"		\
-			"	b	2b\n"			\
-			"	.popsection\n"			\
-			_ASM_EXTABLE(1b, 3b)			\
+			_ASM_EXTABLE_UACCESS_ERR(1b, 2b, %w0)	\
 			: "=r" (res)				\
-			: "r" (address), "i" (-EFAULT));	\
+			: "r" (address));			\
 		uaccess_ttbr0_disable();			\
 	}
 
diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
index c2951b963335..bbbc95313f2e 100644
--- a/arch/arm64/mm/extable.c
+++ b/arch/arm64/mm/extable.c
@@ -3,10 +3,12 @@
  * Based on arch/arm/mm/extable.c
  */
 
+#include <linux/bitfield.h>
 #include <linux/extable.h>
 #include <linux/uaccess.h>
 
 #include <asm/asm-extable.h>
+#include <asm/ptrace.h>
 
 typedef bool (*ex_handler_t)(const struct exception_table_entry *,
 			     struct pt_regs *);
@@ -24,6 +26,19 @@ static bool ex_handler_fixup(const struct exception_table_entry *ex,
 	return true;
 }
 
+static bool ex_handler_uaccess_err_zero(const struct exception_table_entry *ex,
+					struct pt_regs *regs)
+{
+	int reg_err = FIELD_GET(EX_DATA_REG_ERR, ex->data);
+	int reg_zero = FIELD_GET(EX_DATA_REG_ZERO, ex->data);
+
+	pt_regs_write_reg(regs, reg_err, -EFAULT);
+	pt_regs_write_reg(regs, reg_zero, 0);
+
+	regs->pc = get_ex_fixup(ex);
+	return true;
+}
+
 bool fixup_exception(struct pt_regs *regs)
 {
 	const struct exception_table_entry *ex;
@@ -37,6 +52,8 @@ bool fixup_exception(struct pt_regs *regs)
 		return ex_handler_fixup(ex, regs);
 	case EX_TYPE_BPF:
 		return ex_handler_bpf(ex, regs);
+	case EX_TYPE_UACCESS_ERR_ZERO:
+		return ex_handler_uaccess_err_zero(ex, regs);
 	}
 
 	BUG();
-- 
2.11.0


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

[PATCH 12/13] arm64: extable: add load_unaligned_zeropad() handler

[Mark Rutland]

For inline assembly, we place exception fixups out-of-line in the
`.fixup` section such that these are out of the way of the fast path.
This has a few drawbacks:

* Since the fixup code is anonymous, backtraces will symbolize fixups as
  offsets from the nearest prior symbol, currently
  `__entry_tramp_text_end`. This is confusing, and painful to debug
  without access to the relevant vmlinux.

* Since the exception handler adjusts the PC to execute the fixup, and
  the fixup uses a direct branch back into the function it fixes,
  backtraces of fixups miss the original function. This is confusing,
  and violates requirements for RELIABLE_STACKTRACE (and therefore
  LIVEPATCH).

* Inline assembly and associated fixups are generated from templates,
  and we have many copies of logically identical fixups which only
  differ in which specific registers are written to and which address is
  branched to at the end of the fixup. This is potentially wasteful of
  I-cache resources, and makes it hard to add additional logic to fixups
  without significant bloat.

* In the case of load_unaligned_zeropad(), the logic in the fixup
  requires a temporary register that we must allocate even in the
  fast-path where it will not be used.

This patch address all four concerns for load_unaligned_zeropad() fixups
by adding a dedicated exception handler which performs the fixup logic
in exception context and subsequent returns back after the faulting
instruction. For the moment, the fixup logic is identical to the old
assembly fixup logic, but in future we could enhance this by taking the
ESR and FAR into account to constrain the faults we try to fix up, or to
specialize fixups for MTE tag check faults.

Other than backtracing, 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: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/asm-extable.h    | 15 +++++++++++++++
 arch/arm64/include/asm/word-at-a-time.h | 21 ++++-----------------
 arch/arm64/mm/extable.c                 | 29 +++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
index 11209da19c62..c39f2437e08e 100644
--- a/arch/arm64/include/asm/asm-extable.h
+++ b/arch/arm64/include/asm/asm-extable.h
@@ -6,6 +6,7 @@
 #define EX_TYPE_FIXUP			1
 #define EX_TYPE_BPF			2
 #define EX_TYPE_UACCESS_ERR_ZERO	3
+#define EX_TYPE_LOAD_UNALIGNED_ZEROPAD	4
 
 #ifdef __ASSEMBLY__
 
@@ -75,6 +76,20 @@
 #define _ASM_EXTABLE_UACCESS_ERR(insn, fixup, err)			\
 	_ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, wzr)
 
+#define EX_DATA_REG_DATA_SHIFT	0
+#define EX_DATA_REG_DATA	GENMASK(4, 0)
+#define EX_DATA_REG_ADDR_SHIFT	5
+#define EX_DATA_REG_ADDR	GENMASK(9, 5)
+
+#define _ASM_EXTABLE_LOAD_UNALIGNED_ZEROPAD(insn, fixup, data, addr)		\
+	__DEFINE_ASM_GPR_NUMS							\
+	__ASM_EXTABLE_RAW(#insn, #fixup,					\
+			  __stringify(EX_TYPE_LOAD_UNALIGNED_ZEROPAD),		\
+			  "("							\
+			    EX_DATA_REG(DATA, data) " | "			\
+			    EX_DATA_REG(ADDR, addr)				\
+			  ")")
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_ASM_EXTABLE_H */
diff --git a/arch/arm64/include/asm/word-at-a-time.h b/arch/arm64/include/asm/word-at-a-time.h
index 2dcb104c645b..1c8e4f2490bf 100644
--- a/arch/arm64/include/asm/word-at-a-time.h
+++ b/arch/arm64/include/asm/word-at-a-time.h
@@ -53,29 +53,16 @@ static inline unsigned long find_zero(unsigned long mask)
  */
 static inline unsigned long load_unaligned_zeropad(const void *addr)
 {
-	unsigned long ret, tmp;
+	unsigned long ret;
 
 	__uaccess_enable_tco_async();
 
 	/* Load word from unaligned pointer addr */
 	asm(
-	"1:	ldr	%0, %3\n"
+	"1:	ldr	%0, %2\n"
 	"2:\n"
-	"	.pushsection .fixup,\"ax\"\n"
-	"	.align 2\n"
-	"3:	bic	%1, %2, #0x7\n"
-	"	ldr	%0, [%1]\n"
-	"	and	%1, %2, #0x7\n"
-	"	lsl	%1, %1, #0x3\n"
-#ifndef __AARCH64EB__
-	"	lsr	%0, %0, %1\n"
-#else
-	"	lsl	%0, %0, %1\n"
-#endif
-	"	b	2b\n"
-	"	.popsection\n"
-	_ASM_EXTABLE(1b, 3b)
-	: "=&r" (ret), "=&r" (tmp)
+	_ASM_EXTABLE_LOAD_UNALIGNED_ZEROPAD(1b, 2b, %0, %1)
+	: "=&r" (ret)
 	: "r" (addr), "Q" (*(unsigned long *)addr));
 
 	__uaccess_disable_tco_async();
diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c
index bbbc95313f2e..c3d53811a15e 100644
--- a/arch/arm64/mm/extable.c
+++ b/arch/arm64/mm/extable.c
@@ -39,6 +39,33 @@ static bool ex_handler_uaccess_err_zero(const struct exception_table_entry *ex,
 	return true;
 }
 
+static bool
+ex_handler_load_unaligned_zeropad(const struct exception_table_entry *ex,
+				  struct pt_regs *regs)
+{
+	int reg_data = FIELD_GET(EX_DATA_REG_DATA, ex->type);
+	int reg_addr = FIELD_GET(EX_DATA_REG_ADDR, ex->type);
+	unsigned long data, addr, offset;
+
+	addr = pt_regs_read_reg(regs, reg_addr);
+
+	offset = addr & 0x7UL;
+	addr &= ~0x7UL;
+
+	data = *(unsigned long*)addr;
+
+#ifndef __AARCH64EB__
+	data >>= 8 * offset;
+#else
+	data <<= 8 * offset;
+#endif
+
+	pt_regs_write_reg(regs, reg_data, data);
+
+	regs->pc = get_ex_fixup(ex);
+	return true;
+}
+
 bool fixup_exception(struct pt_regs *regs)
 {
 	const struct exception_table_entry *ex;
@@ -54,6 +81,8 @@ bool fixup_exception(struct pt_regs *regs)
 		return ex_handler_bpf(ex, regs);
 	case EX_TYPE_UACCESS_ERR_ZERO:
 		return ex_handler_uaccess_err_zero(ex, regs);
+	case EX_TYPE_LOAD_UNALIGNED_ZEROPAD:
+		return ex_handler_load_unaligned_zeropad(ex, regs);
 	}
 
 	BUG();
-- 
2.11.0


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

[PATCH 13/13] arm64: vmlinux.lds.S: remove `.fixup` section

[Mark Rutland]

We no longer place anything into a `.fixup` section, so we no longer
need to place those sections into the `.text` section in the main kernel
Image.

Remove the use of `.fixup`.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/vmlinux.lds.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index f6b1a88245db..d567c360d2e3 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -161,7 +161,6 @@ SECTIONS
 			IDMAP_TEXT
 			HIBERNATE_TEXT
 			TRAMP_TEXT
-			*(.fixup)
 			*(.gnu.warning)
 		. = ALIGN(16);
 		*(.got)			/* Global offset table		*/
-- 
2.11.0


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

Re: [PATCH 01/13] arm64: lib: __arch_clear_user(): fold fixups into body

[Robin Murphy]

On 2021-10-13 12:00, Mark Rutland wrote:
> Like other functions, __arch_clear_user() places its exception fixups in
> the `.fixup` section without any clear association with
> __arch_clear_user() itself. If we backtrace the fixup code, it will be
> symbolized as an offset from the nearest prior symbol, which happens to
> be `__entry_tramp_text_end`. Further, since the PC adjustment for the
> fixup is akin to a direct branch rather than a function call,
> __arch_clear_user() itself will be missing from the backtrace.
> 
> This is confusing and hinders debugging. In general this pattern will
> also be problematic for CONFIG_LIVEPATCH, since fixups often return to
> their associated function, but this isn't accurately captured in the
> stacktrace.
> 
> To solve these issues for assembly functions, we must move fixups into
> the body of the functions themselves, after the usual fast-path returns.
> This patch does so for __arch_clear_user().
> 
> Inline assembly will be dealt with in subsequent patches.
> 
> Other than the improved backtracing, there should be no functional
> change as a result of this patch.

Oh, I always assumed the .fixup section might have some special 
significance of its own. If not, all the better - modulo one possible 
nit below, this is fine by me. Also it's led me to see that 
copy_in_user() has finally left us, hooray! Guess I've got no more 
excuses to put off that promised usercopy rewrite other than finding the 
time now...

> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>   arch/arm64/lib/clear_user.S | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
> index a7efb2ad2a1c..dac13df4a1ed 100644
> --- a/arch/arm64/lib/clear_user.S
> +++ b/arch/arm64/lib/clear_user.S
> @@ -45,13 +45,10 @@ USER(9f, sttrh	wzr, [x0])
>   USER(7f, sttrb	wzr, [x2, #-1])
>   5:	mov	x0, #0
>   	ret
> -SYM_FUNC_END(__arch_clear_user)
> -EXPORT_SYMBOL(__arch_clear_user)
>   
> -	.section .fixup,"ax"

The one useful purpose this did serve is to provide a handy visual cue - 
if you have any more concrete reason to respin the series, would you 
mind sticking in a little comment like "// Exception fixup" here and in 
the other two assembly routines, just so it's harder to overlook that 
the preceding ret is the normal exit path?

Either way, for patches 1-3,

Acked-by: Robin Murphy <robin.murphy@arm.com>

I got totally lost trying to follow the _ASM_EXTABLE_UACCESS_* business, 
but I don't think the rest of the series gets in the way of any 
outstanding plans either (and FWIW I always thought "fixup->fixup" was 
pretty awful so feel free to have an ack for patch #9 as well). Indeed, 
I guess the new type field should mean that we can implement "proper" 
address-aware fault handlers without the Itanium trick if we still need to.

Cheers,
Robin

> -	.align	2
>   7:	sub	x0, x2, #5	// Adjust for faulting on the final byte...
>   8:	add	x0, x0, #4	// ...or the second word of the 4-7 byte case
>   9:	sub	x0, x2, x0
>   	ret
> -	.previous
> +SYM_FUNC_END(__arch_clear_user)
> +EXPORT_SYMBOL(__arch_clear_user)
> 

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

Re: [PATCH 01/13] arm64: lib: __arch_clear_user(): fold fixups into body

[Mark Rutland]

On Wed, Oct 13, 2021 at 08:55:31PM +0100, Robin Murphy wrote:
> On 2021-10-13 12:00, Mark Rutland wrote:
> > Like other functions, __arch_clear_user() places its exception fixups in
> > the `.fixup` section without any clear association with
> > __arch_clear_user() itself. If we backtrace the fixup code, it will be
> > symbolized as an offset from the nearest prior symbol, which happens to
> > be `__entry_tramp_text_end`. Further, since the PC adjustment for the
> > fixup is akin to a direct branch rather than a function call,
> > __arch_clear_user() itself will be missing from the backtrace.
> > 
> > This is confusing and hinders debugging. In general this pattern will
> > also be problematic for CONFIG_LIVEPATCH, since fixups often return to
> > their associated function, but this isn't accurately captured in the
> > stacktrace.
> > 
> > To solve these issues for assembly functions, we must move fixups into
> > the body of the functions themselves, after the usual fast-path returns.
> > This patch does so for __arch_clear_user().
> > 
> > Inline assembly will be dealt with in subsequent patches.
> > 
> > Other than the improved backtracing, there should be no functional
> > change as a result of this patch.
> 
> Oh, I always assumed the .fixup section might have some special significance
> of its own. If not, all the better - modulo one possible nit below, this is
> fine by me.

Cheers, I'll go fix those up.

> Also it's led me to see that copy_in_user() has finally left us,
> hooray! Guess I've got no more excuses to put off that promised usercopy
> rewrite other than finding the time now...

;)

> > diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
> > index a7efb2ad2a1c..dac13df4a1ed 100644
> > --- a/arch/arm64/lib/clear_user.S
> > +++ b/arch/arm64/lib/clear_user.S
> > @@ -45,13 +45,10 @@ USER(9f, sttrh	wzr, [x0])
> >   USER(7f, sttrb	wzr, [x2, #-1])
> >   5:	mov	x0, #0
> >   	ret
> > -SYM_FUNC_END(__arch_clear_user)
> > -EXPORT_SYMBOL(__arch_clear_user)
> > -	.section .fixup,"ax"
> 
> The one useful purpose this did serve is to provide a handy visual cue - if
> you have any more concrete reason to respin the series, would you mind
> sticking in a little comment like "// Exception fixup" here and in the other
> two assembly routines, just so it's harder to overlook that the preceding
> ret is the normal exit path?

I've added:

| // Exception fixups

... to the start of the fixups in patches 1-3.

> Either way, for patches 1-3,
> 
> Acked-by: Robin Murphy <robin.murphy@arm.com>

Thanks!
 
> I got totally lost trying to follow the _ASM_EXTABLE_UACCESS_* business, but
> I don't think the rest of the series gets in the way of any outstanding
> plans either (and FWIW I always thought "fixup->fixup" was pretty awful so
> feel free to have an ack for patch #9 as well).

Thanks again!

> Indeed, I guess the new type field should mean that we can implement
> "proper" address-aware fault handlers without the Itanium trick if we
> still need to.

Yes -- my thinking was that we can capture the address register(s) and
offset(s) in the data field, and *also* provide the ESR and/or FAR if
necessary in the exception handler.

What's the Itanium trick?

Thanks,
Mark.

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

Re: [PATCH 00/13] arm64: extable: remove anonymous out-of-line fixups

[Ard Biesheuvel]

On Wed, 13 Oct 2021 at 13:01, Mark Rutland <mark.rutland@arm.com> wrote:
>
> We recently realised that out-of-line extable fixups cause a number of problems
> for backtracing (mattering both for developers and for RELIABLE_STACKTRACE and
> LIVEPATCH). Dmitry spotted a confusing backtrace, which we identified was due
> to problems with unwinding fixups, as summarized in:
>
>   https://lore.kernel.org/linux-arm-kernel/20210927171812.GB9201@C02TD0UTHF1T.local/
>
> The gist is that while backtracing through a fixup, the fixup gets symmbolized
> as an offset from the nearest prior symbol (which happens to be
> `__entry_tramp_text_end`), and we the backtrace misses the function that was
> being fixed up (because the fixup handling adjusts the PC, then the fixup does
> a direct branch back to the original function). We can't reliably map from an
> arbitrary PC in the fixup text back to the original function.
>
> The way we create fixups is a bit unfortunate: most fixups are generated from
> common templates, and only differ in register to be poked and the address to
> branch back to, leading to redundant copies of the same logic that must pollute
> Since the fixups are all written in assembly, and duplicated for each fixup
> site, we can only perform very simple fixups, and can't handle any complex
> triage that we might need for some exceptions (e.g. MTE faults).
>
> This series address these concerns by getting rid of the out-of-line anonymous
> fixup logic:
>
> * For plain assembly functions, we move the fixup into the body of
>   the function, after the usual return, as we already do for our cache
>   routines. This simplifies the source code, and only adds a handful of
>   instructions to the main body of `.text`.
>
>   This is handled by the first three patches, which I think are trivial and
>   could be queued regardless of the rest of the series.
>
> * For inline assembly, we add specialised handlers which run in exception
>   context to update registers, then adjust the PC *within* the faulting
>   function. This requires some new glue to capture the handler and metadata in
>   struct exception_table_entry (costing 32 bits per fixup), but for any
>   non-trivial fixup (which is all of the inline asm cases), this removes at
>   least two instructions of out-of-line fixup.
>
> As the fixups are now handled from C code in exception context, we can more
> easily extend these in future with more complex triage if necessary.
>
> Overall, this doesn't have an appreciable impact on Image size (in local
> testing the size of the Image was identical before/after), but does shift the
> boundary between .text and .ordata, making .text smaller and .rodata bigger.
> .text somewhat while growing .rodata somewhat.
>
> I've tested this with both GCC and clang (including with clang CFI), and
> everything is working as expected.
>
> Other than changes to backtracing, there should be no functional change as a
> result of this series.
>
> Thanks
> Mark.
>
> Mark Rutland (13):
>   arm64: lib: __arch_clear_user(): fold fixups into body
>   arm64: lib: __arch_copy_from_user(): fold fixups into body
>   arm64: lib: __arch_copy_to_user(): fold fixups into body
>   arm64: kvm: use kvm_exception_table_entry
>   arm64: factor out GPR numbering helpers
>   arm64: gpr-num: support W registers
>   arm64: extable: consolidate definitions
>   arm64: extable: make fixup_exception() return bool
>   arm64: extable: use `ex` for `exception_table_entry`
>   arm64: extable: add `type` and `data` fields
>   arm64: extable: add a dedicated uaccess handler
>   arm64: extable: add load_unaligned_zeropad() handler
>   arm64: vmlinux.lds.S: remove `.fixup` section
>

This looks like a very worthwhile improvement to me, as it moves
complexity from the asm fault sites to the C handling code.

For the series,

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

>  arch/arm64/include/asm/asm-extable.h    | 95 +++++++++++++++++++++++++++++++++
>  arch/arm64/include/asm/asm-uaccess.h    |  7 ++-
>  arch/arm64/include/asm/assembler.h      | 29 +---------
>  arch/arm64/include/asm/extable.h        | 23 +++++---
>  arch/arm64/include/asm/futex.h          | 25 +++------
>  arch/arm64/include/asm/gpr-num.h        | 26 +++++++++
>  arch/arm64/include/asm/kvm_asm.h        |  7 +--
>  arch/arm64/include/asm/sysreg.h         | 25 +++------
>  arch/arm64/include/asm/uaccess.h        | 26 ++-------
>  arch/arm64/include/asm/word-at-a-time.h | 21 ++------
>  arch/arm64/kernel/armv8_deprecated.c    | 12 ++---
>  arch/arm64/kernel/traps.c               |  9 +---
>  arch/arm64/kernel/vmlinux.lds.S         |  1 -
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 10 ++--
>  arch/arm64/lib/clear_user.S             |  9 ++--
>  arch/arm64/lib/copy_from_user.S         |  7 +--
>  arch/arm64/lib/copy_to_user.S           |  7 +--
>  arch/arm64/mm/extable.c                 | 85 +++++++++++++++++++++++++----
>  arch/arm64/net/bpf_jit_comp.c           |  9 ++--
>  scripts/sorttable.c                     | 30 +++++++++++
>  20 files changed, 306 insertions(+), 157 deletions(-)
>  create mode 100644 arch/arm64/include/asm/asm-extable.h
>  create mode 100644 arch/arm64/include/asm/gpr-num.h
>
> --
> 2.11.0
>

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

Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields

[Will Deacon]

On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
> Subsequent patches will add specialized handlers for fixups, in addition
> to the simple PC fixup and BPF handlers we have today. In preparation,
> this patch adds a new `type` field to struct exception_table_entry, and
> uses this to distinguish the fixup and BPF cases. A `data` field is also
> added so that subsequent patches can associate data specific to each
> exception site (e.g. register numbers).
> 
> Handlers are named ex_handler_*() for consistency, following the exmaple
> of x86. At the same time, get_ex_fixup() is split out into a helper so
> that it can be used by other ex_handler_*() functions ins subsequent
> patches.
> 
> This patch will increase the size of the exception tables, which will be
> remedied by subsequent patches removing redundant fixup code. There
> should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: James Morse <james.morse@arm.com>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
>  arch/arm64/include/asm/extable.h     | 19 +++++++++++++++----
>  arch/arm64/mm/extable.c              | 29 +++++++++++++++++++++++++----
>  arch/arm64/net/bpf_jit_comp.c        |  7 +++++--
>  scripts/sorttable.c                  | 30 ++++++++++++++++++++++++++++++
>  5 files changed, 95 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
> index 986b4c0d4792..5ee748edaef1 100644
> --- a/arch/arm64/include/asm/asm-extable.h
> +++ b/arch/arm64/include/asm/asm-extable.h
> @@ -2,13 +2,19 @@
>  #ifndef __ASM_ASM_EXTABLE_H
>  #define __ASM_ASM_EXTABLE_H
>  
> +#define EX_TYPE_NONE			0
> +#define EX_TYPE_FIXUP			1
> +#define EX_TYPE_BPF			2
> +
>  #ifdef __ASSEMBLY__
>  
> -#define __ASM_EXTABLE_RAW(insn, fixup)		\
> -	.pushsection	__ex_table, "a";	\
> -	.align		3;			\
> -	.long		((insn) - .);		\
> -	.long		((fixup) - .);		\
> +#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
> +	.pushsection	__ex_table, "a";		\
> +	.align		2;				\
> +	.long		((insn) - .);			\
> +	.long		((fixup) - .);			\
> +	.short		(type);				\
> +	.short		(data);				\

Why are you reducing the alignment here?

> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> index 6ee4fa882919..ee95bb47a50d 100644
> --- a/scripts/sorttable.c
> +++ b/scripts/sorttable.c
> @@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)
>  	}
>  }
>  
> +static void arm64_sort_relative_table(char *extab_image, int image_size)
> +{
> +	int i = 0;
> +
> +	while (i < image_size) {
> +		uint32_t *loc = (uint32_t *)(extab_image + i);
> +
> +		w(r(loc) + i, loc);
> +		w(r(loc + 1) + i + 4, loc + 1);
> +		/* Don't touch the fixup type or data */
> +
> +		i += sizeof(uint32_t) * 3;
> +	}
> +
> +	qsort(extab_image, image_size / 12, 12, compare_relative_table);
> +
> +	i = 0;
> +	while (i < image_size) {
> +		uint32_t *loc = (uint32_t *)(extab_image + i);
> +
> +		w(r(loc) - i, loc);
> +		w(r(loc + 1) - (i + 4), loc + 1);
> +		/* Don't touch the fixup type or data */
> +
> +		i += sizeof(uint32_t) * 3;
> +	}
> +}

This is very nearly a direct copy of x86_sort_relative_table() (magic
numbers and all). It would be nice to tidy that up, but I couldn't
immediately see a good way to do it :(

Will

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

Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields

[Mark Rutland]

On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:
> On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
> > Subsequent patches will add specialized handlers for fixups, in addition
> > to the simple PC fixup and BPF handlers we have today. In preparation,
> > this patch adds a new `type` field to struct exception_table_entry, and
> > uses this to distinguish the fixup and BPF cases. A `data` field is also
> > added so that subsequent patches can associate data specific to each
> > exception site (e.g. register numbers).
> > 
> > Handlers are named ex_handler_*() for consistency, following the exmaple
> > of x86. At the same time, get_ex_fixup() is split out into a helper so
> > that it can be used by other ex_handler_*() functions ins subsequent
> > patches.
> > 
> > This patch will increase the size of the exception tables, which will be
> > remedied by subsequent patches removing redundant fixup code. There
> > should be no functional change as a result of this patch.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Andrii Nakryiko <andrii@kernel.org>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > ---
> >  arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
> >  arch/arm64/include/asm/extable.h     | 19 +++++++++++++++----
> >  arch/arm64/mm/extable.c              | 29 +++++++++++++++++++++++++----
> >  arch/arm64/net/bpf_jit_comp.c        |  7 +++++--
> >  scripts/sorttable.c                  | 30 ++++++++++++++++++++++++++++++
> >  5 files changed, 95 insertions(+), 22 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
> > index 986b4c0d4792..5ee748edaef1 100644
> > --- a/arch/arm64/include/asm/asm-extable.h
> > +++ b/arch/arm64/include/asm/asm-extable.h
> > @@ -2,13 +2,19 @@
> >  #ifndef __ASM_ASM_EXTABLE_H
> >  #define __ASM_ASM_EXTABLE_H
> >  
> > +#define EX_TYPE_NONE			0
> > +#define EX_TYPE_FIXUP			1
> > +#define EX_TYPE_BPF			2
> > +
> >  #ifdef __ASSEMBLY__
> >  
> > -#define __ASM_EXTABLE_RAW(insn, fixup)		\
> > -	.pushsection	__ex_table, "a";	\
> > -	.align		3;			\
> > -	.long		((insn) - .);		\
> > -	.long		((fixup) - .);		\
> > +#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
> > +	.pushsection	__ex_table, "a";		\
> > +	.align		2;				\
> > +	.long		((insn) - .);			\
> > +	.long		((fixup) - .);			\
> > +	.short		(type);				\
> > +	.short		(data);				\
> 
> Why are you reducing the alignment here?

That's because the size of each entry is now 12 bytes, and 
`.align 3` aligns to 8 bytes, which would leave a gap between entries.
We only require the fields are naturally aligned, so `.align 2` is
sufficient, and doesn't waste space.

I'll update the commit message to call that out.

> > diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> > index 6ee4fa882919..ee95bb47a50d 100644
> > --- a/scripts/sorttable.c
> > +++ b/scripts/sorttable.c
> > @@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)
> >  	}
> >  }
> >  
> > +static void arm64_sort_relative_table(char *extab_image, int image_size)
> > +{
> > +	int i = 0;
> > +
> > +	while (i < image_size) {
> > +		uint32_t *loc = (uint32_t *)(extab_image + i);
> > +
> > +		w(r(loc) + i, loc);
> > +		w(r(loc + 1) + i + 4, loc + 1);
> > +		/* Don't touch the fixup type or data */
> > +
> > +		i += sizeof(uint32_t) * 3;
> > +	}
> > +
> > +	qsort(extab_image, image_size / 12, 12, compare_relative_table);
> > +
> > +	i = 0;
> > +	while (i < image_size) {
> > +		uint32_t *loc = (uint32_t *)(extab_image + i);
> > +
> > +		w(r(loc) - i, loc);
> > +		w(r(loc + 1) - (i + 4), loc + 1);
> > +		/* Don't touch the fixup type or data */
> > +
> > +		i += sizeof(uint32_t) * 3;
> > +	}
> > +}
> 
> This is very nearly a direct copy of x86_sort_relative_table() (magic
> numbers and all). It would be nice to tidy that up, but I couldn't
> immediately see a good way to do it :(

Beware that's true in linux-next, but not mainline, as that changes in
commit:

  46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics")

A patch to unify the two is trivial, but will cause a cross-tree
dependency, so I'd suggest having this separate for now and sending a
unification patch come -rc1.

I can note something to that effect in the commit message, if that
helps?

Thanks,
Mark.

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

Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields

[Will Deacon]

On Tue, Oct 19, 2021 at 12:50:22PM +0100, Mark Rutland wrote:
> On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:
> > On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
> > > Subsequent patches will add specialized handlers for fixups, in addition
> > > to the simple PC fixup and BPF handlers we have today. In preparation,
> > > this patch adds a new `type` field to struct exception_table_entry, and
> > > uses this to distinguish the fixup and BPF cases. A `data` field is also
> > > added so that subsequent patches can associate data specific to each
> > > exception site (e.g. register numbers).
> > > 
> > > Handlers are named ex_handler_*() for consistency, following the exmaple
> > > of x86. At the same time, get_ex_fixup() is split out into a helper so
> > > that it can be used by other ex_handler_*() functions ins subsequent
> > > patches.
> > > 
> > > This patch will increase the size of the exception tables, which will be
> > > remedied by subsequent patches removing redundant fixup code. There
> > > should be no functional change as a result of this patch.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > Cc: Andrii Nakryiko <andrii@kernel.org>
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > Cc: James Morse <james.morse@arm.com>
> > > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > Cc: Robin Murphy <robin.murphy@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > ---
> > >  arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
> > >  arch/arm64/include/asm/extable.h     | 19 +++++++++++++++----
> > >  arch/arm64/mm/extable.c              | 29 +++++++++++++++++++++++++----
> > >  arch/arm64/net/bpf_jit_comp.c        |  7 +++++--
> > >  scripts/sorttable.c                  | 30 ++++++++++++++++++++++++++++++
> > >  5 files changed, 95 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
> > > index 986b4c0d4792..5ee748edaef1 100644
> > > --- a/arch/arm64/include/asm/asm-extable.h
> > > +++ b/arch/arm64/include/asm/asm-extable.h
> > > @@ -2,13 +2,19 @@
> > >  #ifndef __ASM_ASM_EXTABLE_H
> > >  #define __ASM_ASM_EXTABLE_H
> > >  
> > > +#define EX_TYPE_NONE			0
> > > +#define EX_TYPE_FIXUP			1
> > > +#define EX_TYPE_BPF			2
> > > +
> > >  #ifdef __ASSEMBLY__
> > >  
> > > -#define __ASM_EXTABLE_RAW(insn, fixup)		\
> > > -	.pushsection	__ex_table, "a";	\
> > > -	.align		3;			\
> > > -	.long		((insn) - .);		\
> > > -	.long		((fixup) - .);		\
> > > +#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
> > > +	.pushsection	__ex_table, "a";		\
> > > +	.align		2;				\
> > > +	.long		((insn) - .);			\
> > > +	.long		((fixup) - .);			\
> > > +	.short		(type);				\
> > > +	.short		(data);				\
> > 
> > Why are you reducing the alignment here?
> 
> That's because the size of each entry is now 12 bytes, and 
> `.align 3` aligns to 8 bytes, which would leave a gap between entries.
> We only require the fields are naturally aligned, so `.align 2` is
> sufficient, and doesn't waste space.
> 
> I'll update the commit message to call that out.

I think the part which is confusing me is that I would expect the alignment
here to match the alignment of the corresponding C type, but the old value
of '3' doesn't seem to do that, so is this patch fixing an earlier bug?

Without your patches in the picture, we're using a '.align 3' in
_asm_extable, but with:

struct exception_table_entry
{
	int insn, fixup;
};

I suppose it works out because that over-alignment doesn't result in any
additional padding, but I think we could reduce the current alignment
without any of these other changes, no?

> > > diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> > > index 6ee4fa882919..ee95bb47a50d 100644
> > > --- a/scripts/sorttable.c
> > > +++ b/scripts/sorttable.c
> > > @@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)
> > >  	}
> > >  }
> > >  
> > > +static void arm64_sort_relative_table(char *extab_image, int image_size)
> > > +{
> > > +	int i = 0;
> > > +
> > > +	while (i < image_size) {
> > > +		uint32_t *loc = (uint32_t *)(extab_image + i);
> > > +
> > > +		w(r(loc) + i, loc);
> > > +		w(r(loc + 1) + i + 4, loc + 1);
> > > +		/* Don't touch the fixup type or data */
> > > +
> > > +		i += sizeof(uint32_t) * 3;
> > > +	}
> > > +
> > > +	qsort(extab_image, image_size / 12, 12, compare_relative_table);
> > > +
> > > +	i = 0;
> > > +	while (i < image_size) {
> > > +		uint32_t *loc = (uint32_t *)(extab_image + i);
> > > +
> > > +		w(r(loc) - i, loc);
> > > +		w(r(loc + 1) - (i + 4), loc + 1);
> > > +		/* Don't touch the fixup type or data */
> > > +
> > > +		i += sizeof(uint32_t) * 3;
> > > +	}
> > > +}
> > 
> > This is very nearly a direct copy of x86_sort_relative_table() (magic
> > numbers and all). It would be nice to tidy that up, but I couldn't
> > immediately see a good way to do it :(
> 
> Beware that's true in linux-next, but not mainline, as that changes in
> commit:
> 
>   46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics")
> 
> A patch to unify the two is trivial, but will cause a cross-tree
> dependency, so I'd suggest having this separate for now and sending a
> unification patch come -rc1.
> 
> I can note something to that effect in the commit message, if that
> helps?

Yeah, I suppose. It's not worth tripping over the x86 changes, but we
should try to remember to come back and unify things.

Will

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

Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields

[Ard Biesheuvel]

On Tue, 19 Oct 2021 at 14:05, Will Deacon <will@kernel.org> wrote:
>
> On Tue, Oct 19, 2021 at 12:50:22PM +0100, Mark Rutland wrote:
> > On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:
> > > On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
> > > > Subsequent patches will add specialized handlers for fixups, in addition
> > > > to the simple PC fixup and BPF handlers we have today. In preparation,
> > > > this patch adds a new `type` field to struct exception_table_entry, and
> > > > uses this to distinguish the fixup and BPF cases. A `data` field is also
> > > > added so that subsequent patches can associate data specific to each
> > > > exception site (e.g. register numbers).
> > > >
> > > > Handlers are named ex_handler_*() for consistency, following the exmaple
> > > > of x86. At the same time, get_ex_fixup() is split out into a helper so
> > > > that it can be used by other ex_handler_*() functions ins subsequent
> > > > patches.
> > > >
> > > > This patch will increase the size of the exception tables, which will be
> > > > remedied by subsequent patches removing redundant fixup code. There
> > > > should be no functional change as a result of this patch.
> > > >
> > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > > Cc: Andrii Nakryiko <andrii@kernel.org>
> > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > > Cc: James Morse <james.morse@arm.com>
> > > > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > > Cc: Robin Murphy <robin.murphy@arm.com>
> > > > Cc: Will Deacon <will@kernel.org>
> > > > ---
> > > >  arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
> > > >  arch/arm64/include/asm/extable.h     | 19 +++++++++++++++----
> > > >  arch/arm64/mm/extable.c              | 29 +++++++++++++++++++++++++----
> > > >  arch/arm64/net/bpf_jit_comp.c        |  7 +++++--
> > > >  scripts/sorttable.c                  | 30 ++++++++++++++++++++++++++++++
> > > >  5 files changed, 95 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
> > > > index 986b4c0d4792..5ee748edaef1 100644
> > > > --- a/arch/arm64/include/asm/asm-extable.h
> > > > +++ b/arch/arm64/include/asm/asm-extable.h
> > > > @@ -2,13 +2,19 @@
> > > >  #ifndef __ASM_ASM_EXTABLE_H
> > > >  #define __ASM_ASM_EXTABLE_H
> > > >
> > > > +#define EX_TYPE_NONE                     0
> > > > +#define EX_TYPE_FIXUP                    1
> > > > +#define EX_TYPE_BPF                      2
> > > > +
> > > >  #ifdef __ASSEMBLY__
> > > >
> > > > -#define __ASM_EXTABLE_RAW(insn, fixup)           \
> > > > - .pushsection    __ex_table, "a";        \
> > > > - .align          3;                      \
> > > > - .long           ((insn) - .);           \
> > > > - .long           ((fixup) - .);          \
> > > > +#define __ASM_EXTABLE_RAW(insn, fixup, type, data)       \
> > > > + .pushsection    __ex_table, "a";                \
> > > > + .align          2;                              \
> > > > + .long           ((insn) - .);                   \
> > > > + .long           ((fixup) - .);                  \
> > > > + .short          (type);                         \
> > > > + .short          (data);                         \
> > >
> > > Why are you reducing the alignment here?
> >
> > That's because the size of each entry is now 12 bytes, and
> > `.align 3` aligns to 8 bytes, which would leave a gap between entries.
> > We only require the fields are naturally aligned, so `.align 2` is
> > sufficient, and doesn't waste space.
> >
> > I'll update the commit message to call that out.
>
> I think the part which is confusing me is that I would expect the alignment
> here to match the alignment of the corresponding C type, but the old value
> of '3' doesn't seem to do that, so is this patch fixing an earlier bug?
>
> Without your patches in the picture, we're using a '.align 3' in
> _asm_extable, but with:
>
> struct exception_table_entry
> {
>         int insn, fixup;
> };
>
> I suppose it works out because that over-alignment doesn't result in any
> additional padding, but I think we could reduce the current alignment
> without any of these other changes, no?
>

If a struct type's size happens to be a power of 2, it is perfectly
reasonable to align it to its size rather than use the minimum
alignment required by its members, as this will generally result in
more efficient placement wrt cachelines, pages, etc.

So the change is obviously needed here, but I wouldn't consider the
old alignment value to be a bug.

-- 
Ard.

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

Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields

[Mark Rutland]

On Tue, Oct 19, 2021 at 01:05:06PM +0100, Will Deacon wrote:
> On Tue, Oct 19, 2021 at 12:50:22PM +0100, Mark Rutland wrote:
> > On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:
> > > On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
> > > > -#define __ASM_EXTABLE_RAW(insn, fixup)		\
> > > > -	.pushsection	__ex_table, "a";	\
> > > > -	.align		3;			\
> > > > -	.long		((insn) - .);		\
> > > > -	.long		((fixup) - .);		\
> > > > +#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
> > > > +	.pushsection	__ex_table, "a";		\
> > > > +	.align		2;				\
> > > > +	.long		((insn) - .);			\
> > > > +	.long		((fixup) - .);			\
> > > > +	.short		(type);				\
> > > > +	.short		(data);				\
> > > 
> > > Why are you reducing the alignment here?
> > 
> > That's because the size of each entry is now 12 bytes, and 
> > `.align 3` aligns to 8 bytes, which would leave a gap between entries.
> > We only require the fields are naturally aligned, so `.align 2` is
> > sufficient, and doesn't waste space.
> > 
> > I'll update the commit message to call that out.
> 
> I think the part which is confusing me is that I would expect the alignment
> here to match the alignment of the corresponding C type, but the old value
> of '3' doesn't seem to do that, so is this patch fixing an earlier bug?
> 
> Without your patches in the picture, we're using a '.align 3' in
> _asm_extable, but with:
> 
> struct exception_table_entry
> {
> 	int insn, fixup;
> };
> 
> I suppose it works out because that over-alignment doesn't result in any
> additional padding, but I think we could reduce the current alignment
> without any of these other changes, no?

Yes, we could reduce that first, but no, it's not a bug -- there's no
functional issue today.

For context, today the `__ex_table` section as a whole and the
`__start___ex_table` symbol also got 8 byte alignment, since in
ARch/arm64/kernel/vmlinux.lds.S we have:

| #define RO_EXCEPTION_TABLE_ALIGN        8

... and so in include/asm-generic/vmlinux.lds.h when the exception table
gets output with:

| EXCEPTION_TABLE(RO_EXCEPTION_TABLE_ALIGN)

| #define EXCEPTION_TABLE(align)                                          \
|         . = ALIGN(align);                                               \
|         __ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) {               \
|                 __start___ex_table = .;                                 \
|                 KEEP(*(__ex_table))                                     \
|                 __stop___ex_table = .;                                  \
|         }

If you want, I can split out a preparatory patch which drops the
alignment to the minimum necessary, both in the asm and for
RO_EXCEPTION_TABLE_ALIGN?

[...]

> > > > +static void arm64_sort_relative_table(char *extab_image, int image_size)
> > > > +{
> > > > +	int i = 0;
> > > > +
> > > > +	while (i < image_size) {
> > > > +		uint32_t *loc = (uint32_t *)(extab_image + i);
> > > > +
> > > > +		w(r(loc) + i, loc);
> > > > +		w(r(loc + 1) + i + 4, loc + 1);
> > > > +		/* Don't touch the fixup type or data */
> > > > +
> > > > +		i += sizeof(uint32_t) * 3;
> > > > +	}
> > > > +
> > > > +	qsort(extab_image, image_size / 12, 12, compare_relative_table);
> > > > +
> > > > +	i = 0;
> > > > +	while (i < image_size) {
> > > > +		uint32_t *loc = (uint32_t *)(extab_image + i);
> > > > +
> > > > +		w(r(loc) - i, loc);
> > > > +		w(r(loc + 1) - (i + 4), loc + 1);
> > > > +		/* Don't touch the fixup type or data */
> > > > +
> > > > +		i += sizeof(uint32_t) * 3;
> > > > +	}
> > > > +}
> > > 
> > > This is very nearly a direct copy of x86_sort_relative_table() (magic
> > > numbers and all). It would be nice to tidy that up, but I couldn't
> > > immediately see a good way to do it :(
> > 
> > Beware that's true in linux-next, but not mainline, as that changes in
> > commit:
> > 
> >   46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics")
> > 
> > A patch to unify the two is trivial, but will cause a cross-tree
> > dependency, so I'd suggest having this separate for now and sending a
> > unification patch come -rc1.
> > 
> > I can note something to that effect in the commit message, if that
> > helps?
> 
> Yeah, I suppose. It's not worth tripping over the x86 changes, but we
> should try to remember to come back and unify things.

Sure; works for me.

Thanks,
Mark.

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