[PATCH 0/7] CFI for ARM32 using LLVM

[PATCH 0/7] CFI for ARM32 using LLVM

[Linus Walleij]

This is a first patch set to support CLANG CFI (Control Flow
Integrity) on ARM32.

For information about what CFI is, see:
https://clang.llvm.org/docs/ControlFlowIntegrity.html

For the kernel KCFI flavor, see:
https://lwn.net/Articles/898040/

The base changes required to bring up KCFI on ARM32 was mostly
related to the use of custom vtables in the kernel, combined
with defines to call into these vtable members directly from
sites where they are used.

The approach to all of these vtable+define issues has been
the same: instead of a define, wrap the call in a static inline
function that explicitly calls the vtable member.

To runtime-test the patches:
- Enable CONFIG_LKDTM
- echo CFI_FORWARD_PROTO > /sys/kernel/debug/provoke-crash/DIRECT

The patch set has been booted to userspace on the following
test platforms:

- Arm Versatile (QEMU)
- Arm Versatile Express (QEMU)
- multi_v7 booted on Versatile Express (QEMU)
- Footbridge Netwinder (SA110 ARMv4)
- Ux500 (ARMv7 SMP)

I am not saying there will not be corner cases that we need
to fix in addition to this, but it is enough to get started.
Looking at what was fixed for arm64 I am a bit weary that
e.g. BPF might need something to trampoline properly.

But hopefullt people can get to testing it and help me fix
remaining issues before the final version, or we can fix it
in-tree.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Linus Walleij (7):
      ARM: Support CLANG CFI
      ARM: tlbflush: Make TLB flushes into static inlines
      ARM: bugs: Check in the vtable instead of defined aliases
      ARM: proc: Use inlines instead of defines
      ARM: delay: Turn delay functions into static inlines
      ARM: turn CPU cache flush functions into static inlines
      ARM: page: Turn highpage accesses into static inlines

 arch/arm/Kconfig                  |  1 +
 arch/arm/common/mcpm_entry.c      | 10 ++-----
 arch/arm/include/asm/cacheflush.h | 45 ++++++++++++++++++++++++-------
 arch/arm/include/asm/delay.h      | 16 ++++++++---
 arch/arm/include/asm/page.h       | 36 ++++++++++++++++++++-----
 arch/arm/include/asm/proc-fns.h   | 57 ++++++++++++++++++++++++++++++++-------
 arch/arm/include/asm/tlbflush.h   | 18 ++++++++-----
 arch/arm/kernel/bugs.c            |  2 +-
 arch/arm/mach-sunxi/mc_smp.c      |  7 +----
 arch/arm/mm/dma.h                 | 28 ++++++++++++++-----
 arch/arm/mm/proc-syms.c           |  7 +----
 arch/arm/mm/proc-v7-bugs.c        |  4 +--
 12 files changed, 167 insertions(+), 64 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240115-arm32-cfi-65d60f201108

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH 1/7] ARM: Support CLANG CFI

[Linus Walleij]

Support Control Flow Integrity (CFI) when compiling with
CLANG.

In the as-of-writing LLVM CLANG implementation (v17)
the 32-bit ARM platform is supported by the generic CFI
implementation, which isn't tailored specifically for ARM32
but works well enough to enable the feature.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0af6709570d1..1216656a40bc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -34,6 +34,7 @@ config ARM
 	select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7
 	select ARCH_SUPPORTS_ATOMIC_RMW
+	select ARCH_SUPPORTS_CFI_CLANG
 	select ARCH_SUPPORTS_HUGETLBFS if ARM_LPAE
 	select ARCH_SUPPORTS_PER_VMA_LOCK
 	select ARCH_USE_BUILTIN_BSWAP

-- 
2.34.1

[PATCH 2/7] ARM: tlbflush: Make TLB flushes into static inlines

[Linus Walleij]

Instead of just using defines to define the TLB flush functions,
use static inlines.

This has the upside that we can tag those as __nocfi so we can
execute a CFI-enabled kernel.

Move the variables around a bit so the functions can find their
global variable cpu_tlb.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/include/asm/tlbflush.h | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h
index 38c6e4a2a0b6..7340518ee0e9 100644
--- a/arch/arm/include/asm/tlbflush.h
+++ b/arch/arm/include/asm/tlbflush.h
@@ -210,13 +210,23 @@ struct cpu_tlb_fns {
 	unsigned long tlb_flags;
 };
 
+extern struct cpu_tlb_fns cpu_tlb;
+
+#define __cpu_tlb_flags			cpu_tlb.tlb_flags
+
 /*
  * Select the calling method
  */
 #ifdef MULTI_TLB
 
-#define __cpu_flush_user_tlb_range	cpu_tlb.flush_user_range
-#define __cpu_flush_kern_tlb_range	cpu_tlb.flush_kern_range
+static inline void __nocfi __cpu_flush_user_tlb_range(unsigned long s, unsigned long e, struct vm_area_struct *vma)
+{
+	cpu_tlb.flush_user_range(s, e, vma);
+}
+static inline void __nocfi __cpu_flush_kern_tlb_range(unsigned long s, unsigned long e)
+{
+	cpu_tlb.flush_kern_range(s, e);
+}
 
 #else
 
@@ -228,10 +238,6 @@ extern void __cpu_flush_kern_tlb_range(unsigned long, unsigned long);
 
 #endif
 
-extern struct cpu_tlb_fns cpu_tlb;
-
-#define __cpu_tlb_flags			cpu_tlb.tlb_flags
-
 /*
  *	TLB Management
  *	==============

-- 
2.34.1

[PATCH 3/7] ARM: bugs: Check in the vtable instead of defined aliases

[Linus Walleij]

Instead of checking if cpu_check_bugs() exist, check for this
callback directly in the CPU vtable: this is better because the
function is just a define to the vtable entry and this is why
the code works. But we want to be able to specify a proper
function for cpu_check_bugs() so look into the vtable instead.

In bugs.c assign PROC_VTABLE(switch_mm) instead of
assigning cpu_do_switch_mm where again this is just a define
into the vtable: this makes it possible to make
cpu_do_switch_mm() into a real function.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/kernel/bugs.c     | 2 +-
 arch/arm/mm/proc-v7-bugs.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/bugs.c b/arch/arm/kernel/bugs.c
index 087bce6ec8e9..35d39efb51ed 100644
--- a/arch/arm/kernel/bugs.c
+++ b/arch/arm/kernel/bugs.c
@@ -7,7 +7,7 @@
 void check_other_bugs(void)
 {
 #ifdef MULTI_CPU
-	if (cpu_check_bugs)
+	if (PROC_VTABLE(check_bugs))
 		cpu_check_bugs();
 #endif
 }
diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 8bc7a2d6d6c7..ea3ee2bd7b56 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -87,14 +87,14 @@ static unsigned int spectre_v2_install_workaround(unsigned int method)
 	case SPECTRE_V2_METHOD_HVC:
 		per_cpu(harden_branch_predictor_fn, cpu) =
 			call_hvc_arch_workaround_1;
-		cpu_do_switch_mm = cpu_v7_hvc_switch_mm;
+		PROC_VTABLE(switch_mm) = cpu_v7_hvc_switch_mm;
 		spectre_v2_method = "hypervisor";
 		break;
 
 	case SPECTRE_V2_METHOD_SMC:
 		per_cpu(harden_branch_predictor_fn, cpu) =
 			call_smc_arch_workaround_1;
-		cpu_do_switch_mm = cpu_v7_smc_switch_mm;
+		PROC_VTABLE(switch_mm) = cpu_v7_smc_switch_mm;
 		spectre_v2_method = "firmware";
 		break;
 	}

-- 
2.34.1

[PATCH 4/7] ARM: proc: Use inlines instead of defines

[Linus Walleij]

We currently access the per-cpu vtable with defines such
as:

  define cpu_proc_init PROC_VTABLE(_proc_init)

Convert all of these instances to static inlines instead:

  static inline __nocfi void cpu_proc_init(void)
  {
        PROC_VTABLE(_proc_init)();
  }

This has the upside that we can add the __nocfi tag to
the inline function so CFI can skip over this and work,
and we can simplify some platform code that was looking
into the symbol table to be able to call cpu_reset(),
now we can just call it instead.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/common/mcpm_entry.c    | 10 ++------
 arch/arm/include/asm/proc-fns.h | 57 +++++++++++++++++++++++++++++++++--------
 arch/arm/mach-sunxi/mc_smp.c    |  7 +----
 3 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
index e013ff1168d3..3e19f246caff 100644
--- a/arch/arm/common/mcpm_entry.c
+++ b/arch/arm/common/mcpm_entry.c
@@ -234,13 +234,10 @@ int mcpm_cpu_power_up(unsigned int cpu, unsigned int cluster)
 	return ret;
 }
 
-typedef typeof(cpu_reset) phys_reset_t;
-
 void mcpm_cpu_power_down(void)
 {
 	unsigned int mpidr, cpu, cluster;
 	bool cpu_going_down, last_man;
-	phys_reset_t phys_reset;
 
 	mpidr = read_cpuid_mpidr();
 	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
@@ -298,8 +295,7 @@ void mcpm_cpu_power_down(void)
 	 * the kernel as if the power_up method just had deasserted reset
 	 * on the CPU.
 	 */
-	phys_reset = (phys_reset_t)(unsigned long)__pa_symbol(cpu_reset);
-	phys_reset(__pa_symbol(mcpm_entry_point), false);
+	cpu_reset(__pa_symbol(mcpm_entry_point), false);
 
 	/* should never get here */
 	BUG();
@@ -376,7 +372,6 @@ static int __init nocache_trampoline(unsigned long _arg)
 	unsigned int mpidr = read_cpuid_mpidr();
 	unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
 	unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
-	phys_reset_t phys_reset;
 
 	mcpm_set_entry_vector(cpu, cluster, cpu_resume_no_hyp);
 	setup_mm_for_reboot();
@@ -387,8 +382,7 @@ static int __init nocache_trampoline(unsigned long _arg)
 	__mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
 	__mcpm_cpu_down(cpu, cluster);
 
-	phys_reset = (phys_reset_t)(unsigned long)__pa_symbol(cpu_reset);
-	phys_reset(__pa_symbol(mcpm_entry_point), false);
+	cpu_reset(__pa_symbol(mcpm_entry_point), false);
 	BUG();
 }
 
diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index 280396483f5d..9bd6bf5f901a 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -131,18 +131,55 @@ static inline void init_proc_vtable(const struct processor *p)
 }
 #endif
 
-#define cpu_proc_init			PROC_VTABLE(_proc_init)
-#define cpu_check_bugs			PROC_VTABLE(check_bugs)
-#define cpu_proc_fin			PROC_VTABLE(_proc_fin)
-#define cpu_reset			PROC_VTABLE(reset)
-#define cpu_do_idle			PROC_VTABLE(_do_idle)
-#define cpu_dcache_clean_area		PROC_TABLE(dcache_clean_area)
-#define cpu_set_pte_ext			PROC_TABLE(set_pte_ext)
-#define cpu_do_switch_mm		PROC_VTABLE(switch_mm)
+static inline void __nocfi cpu_proc_init(void)
+{
+	PROC_VTABLE(_proc_init)();
+}
+static inline void __nocfi cpu_check_bugs(void)
+{
+	PROC_VTABLE(check_bugs)();
+}
+static inline void __nocfi cpu_proc_fin(void)
+{
+	PROC_VTABLE(_proc_fin)();
+}
+static inline void __nocfi cpu_reset(unsigned long addr, bool hvc)
+{
+	PROC_VTABLE(reset)(addr, hvc);
+}
+static inline int __nocfi cpu_do_idle(void)
+{
+	return PROC_VTABLE(_do_idle)();
+}
+static inline void __nocfi cpu_dcache_clean_area(void *addr, int size)
+{
+	PROC_TABLE(dcache_clean_area)(addr, size);
+}
+#ifdef CONFIG_ARM_LPAE
+static inline void __nocfi cpu_set_pte_ext(pte_t *ptep, pte_t pte)
+{
+	PROC_TABLE(set_pte_ext)(ptep, pte);
+}
+#else
+static inline void __nocfi cpu_set_pte_ext(pte_t *ptep, pte_t pte, unsigned int ext)
+{
+	PROC_TABLE(set_pte_ext)(ptep, pte, ext);
+}
+#endif
+static inline void __nocfi cpu_do_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm)
+{
+	PROC_VTABLE(switch_mm)(pgd_phys, mm);
+}
 
 /* These two are private to arch/arm/kernel/suspend.c */
-#define cpu_do_suspend			PROC_VTABLE(do_suspend)
-#define cpu_do_resume			PROC_VTABLE(do_resume)
+static inline void __nocfi cpu_do_suspend(void *p)
+{
+	PROC_VTABLE(do_suspend)(p);
+}
+static inline void __nocfi cpu_do_resume(void *p)
+{
+	PROC_VTABLE(do_resume)(p);
+}
 #endif
 
 extern void cpu_resume(void);
diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index 277f6aa8e6c2..791eabb7d433 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -646,17 +646,12 @@ static bool __init sunxi_mc_smp_cpu_table_init(void)
  *
  * We need the trampoline code to enable CCI-400 on the first cluster
  */
-typedef typeof(cpu_reset) phys_reset_t;
-
 static int __init nocache_trampoline(unsigned long __unused)
 {
-	phys_reset_t phys_reset;
-
 	setup_mm_for_reboot();
 	sunxi_cluster_cache_disable_without_axi();
 
-	phys_reset = (phys_reset_t)(unsigned long)__pa_symbol(cpu_reset);
-	phys_reset(__pa_symbol(sunxi_mc_smp_resume), false);
+	cpu_reset(__pa_symbol(sunxi_mc_smp_resume), false);
 	BUG();
 }
 

-- 
2.34.1

[PATCH 5/7] ARM: delay: Turn delay functions into static inlines

[Linus Walleij]

The members of the vector table arm_delay_ops are called
directly using defines, but this is really confusing for
KCFI. Wrap the calls in static inlines and tag them with
__nocfi so things start to work.

Without this patch, platforms without a delay timer will
not boot (sticks in calibrating loop etc).

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/include/asm/delay.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index 1d069e558d8d..7d611b810b6c 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -55,7 +55,10 @@ extern struct arm_delay_ops {
 	unsigned long ticks_per_jiffy;
 } arm_delay_ops;
 
-#define __delay(n)		arm_delay_ops.delay(n)
+static inline void __nocfi __delay(unsigned long n)
+{
+	arm_delay_ops.delay(n);
+}
 
 /*
  * This function intentionally does not exist; if you see references to
@@ -76,8 +79,15 @@ extern void __bad_udelay(void);
  * first constant multiplications gets optimized away if the delay is
  * a constant)
  */
-#define __udelay(n)		arm_delay_ops.udelay(n)
-#define __const_udelay(n)	arm_delay_ops.const_udelay(n)
+static inline void __nocfi __udelay(unsigned long n)
+{
+	arm_delay_ops.udelay(n);
+}
+
+static inline void __nocfi __const_udelay(unsigned long n)
+{
+	arm_delay_ops.const_udelay(n);
+}
 
 #define udelay(n)							\
 	(__builtin_constant_p(n) ?					\

-- 
2.34.1

[PATCH 6/7] ARM: turn CPU cache flush functions into static inlines

[Linus Walleij]

The members of the vector table struct cpu_cache_fns cpu_cache
are called directly using defines, but this is really confusing
for KCFI. Wrap the calls in static inlines and tag them with
__nocfi so things start to work.

Conversely a similar approach is used for the __glue() helpers
which define their way into an assembly ENTRY(symbol) for respective
CPU variant. We wrap these into static inlines and prefix them
with __nocfi as well. (This happens on !MULTI_CACHE systems.)
For this case we also need to invoke the __glue() macro to
provide a proper function prototype for the inner function.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/include/asm/cacheflush.h | 45 +++++++++++++++++++++++++++++++--------
 arch/arm/mm/dma.h                 | 28 ++++++++++++++++++------
 2 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index 1075534b0a2e..76fb665162a4 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -122,14 +122,38 @@ struct cpu_cache_fns {
 
 extern struct cpu_cache_fns cpu_cache;
 
-#define __cpuc_flush_icache_all		cpu_cache.flush_icache_all
-#define __cpuc_flush_kern_all		cpu_cache.flush_kern_all
-#define __cpuc_flush_kern_louis		cpu_cache.flush_kern_louis
-#define __cpuc_flush_user_all		cpu_cache.flush_user_all
-#define __cpuc_flush_user_range		cpu_cache.flush_user_range
-#define __cpuc_coherent_kern_range	cpu_cache.coherent_kern_range
-#define __cpuc_coherent_user_range	cpu_cache.coherent_user_range
-#define __cpuc_flush_dcache_area	cpu_cache.flush_kern_dcache_area
+static inline void __nocfi __cpuc_flush_icache_all(void)
+{
+	cpu_cache.flush_icache_all();
+}
+static inline void __nocfi __cpuc_flush_kern_all(void)
+{
+	cpu_cache.flush_icache_all();
+}
+static inline void __nocfi __cpuc_flush_kern_louis(void)
+{
+	cpu_cache.flush_kern_louis();
+}
+static inline void __nocfi __cpuc_flush_user_all(void)
+{
+	cpu_cache.flush_user_all();
+}
+static inline void __nocfi __cpuc_flush_user_range(unsigned long start, unsigned long end, unsigned int flags)
+{
+	cpu_cache.flush_user_range(start, end, flags);
+}
+static inline void __nocfi __cpuc_coherent_kern_range(unsigned long start, unsigned long end)
+{
+	cpu_cache.coherent_kern_range(start, end);
+}
+static inline int __nocfi __cpuc_coherent_user_range(unsigned long start, unsigned long end)
+{
+	return cpu_cache.coherent_user_range(start, end);
+}
+static inline void __nocfi __cpuc_flush_dcache_area(void *kaddr, size_t sz)
+{
+	cpu_cache.flush_kern_dcache_area(kaddr, sz);
+}
 
 /*
  * These are private to the dma-mapping API.  Do not use directly.
@@ -137,7 +161,10 @@ extern struct cpu_cache_fns cpu_cache;
  * is visible to DMA, or data written by DMA to system memory is
  * visible to the CPU.
  */
-#define dmac_flush_range		cpu_cache.dma_flush_range
+static inline void __nocfi dmac_flush_range(const void *start, const void *end)
+{
+	cpu_cache.dma_flush_range(start, end);
+}
 
 #else
 
diff --git a/arch/arm/mm/dma.h b/arch/arm/mm/dma.h
index aaef64b7f177..251b8a9fffc1 100644
--- a/arch/arm/mm/dma.h
+++ b/arch/arm/mm/dma.h
@@ -5,8 +5,6 @@
 #include <asm/glue-cache.h>
 
 #ifndef MULTI_CACHE
-#define dmac_map_area			__glue(_CACHE,_dma_map_area)
-#define dmac_unmap_area 		__glue(_CACHE,_dma_unmap_area)
 
 /*
  * These are private to the dma-mapping API.  Do not use directly.
@@ -14,8 +12,20 @@
  * is visible to DMA, or data written by DMA to system memory is
  * visible to the CPU.
  */
-extern void dmac_map_area(const void *, size_t, int);
-extern void dmac_unmap_area(const void *, size_t, int);
+
+/* These turn into function declarations for each per-CPU glue function */
+void __glue(_CACHE,_dma_map_area)(const void *, size_t, int);
+void __glue(_CACHE,_dma_unmap_area)(const void *, size_t, int);
+
+static inline void __nocfi dmac_map_area(const void *start, size_t sz, int flags)
+{
+	__glue(_CACHE,_dma_map_area)(start, sz, flags);
+}
+
+static inline void __nocfi dmac_unmap_area(const void *start, size_t sz, int flags)
+{
+	__glue(_CACHE,_dma_unmap_area)(start, sz, flags);
+}
 
 #else
 
@@ -25,8 +35,14 @@ extern void dmac_unmap_area(const void *, size_t, int);
  * is visible to DMA, or data written by DMA to system memory is
  * visible to the CPU.
  */
-#define dmac_map_area			cpu_cache.dma_map_area
-#define dmac_unmap_area 		cpu_cache.dma_unmap_area
+static inline void __nocfi dmac_map_area(const void *start, size_t sz, int flags)
+{
+	cpu_cache.dma_map_area(start, sz, flags);
+}
+static inline void __nocfi dmac_unmap_area(const void *start, size_t sz, int flags)
+{
+	cpu_cache.dma_unmap_area(start, sz, flags);
+}
 
 #endif
 

-- 
2.34.1

[PATCH 7/7] ARM: page: Turn highpage accesses into static inlines

[Linus Walleij]

Clearing and copying pages in highmem uses either the cpu_user
vtable or the __glue() assembler stubs to call into per-CPU
versions of these functions.

This is all really confusing for KCFI so wrap these into static
inlines and prefix each inline function with __nocfi.

__cpu_clear_user_highpage() and __cpu_copy_user_highpage() are
exported in arch/arm/mm/proc-syms.c which causes a problem with
using static inlines, but it turns out that these exports are
completely unused, so we can just delete them.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/include/asm/page.h | 36 +++++++++++++++++++++++++++++-------
 arch/arm/mm/proc-syms.c     |  7 +------
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index 119aa85d1feb..8bf297228627 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -138,17 +138,39 @@ void xscale_mc_clear_user_highpage(struct page *page, unsigned long vaddr);
 #ifdef MULTI_USER
 extern struct cpu_user_fns cpu_user;
 
-#define __cpu_clear_user_highpage	cpu_user.cpu_clear_user_highpage
-#define __cpu_copy_user_highpage	cpu_user.cpu_copy_user_highpage
+static inline void __nocfi __cpu_clear_user_highpage(struct page *page,
+						     unsigned long vaddr)
+{
+	cpu_user.cpu_clear_user_highpage(page, vaddr);
+}
+
+static inline void __nocfi __cpu_copy_user_highpage(struct page *to,
+			struct page *from, unsigned long vaddr,
+			struct vm_area_struct *vma)
+{
+	cpu_user.cpu_copy_user_highpage(to, from, vaddr, vma);
+}
 
 #else
 
-#define __cpu_clear_user_highpage	__glue(_USER,_clear_user_highpage)
-#define __cpu_copy_user_highpage	__glue(_USER,_copy_user_highpage)
+/* These turn into function declarations for each per-CPU glue function */
+void __glue(_USER,_clear_user_highpage)(struct page *page, unsigned long vaddr);
+void __glue(_USER,_copy_user_highpage)(struct page *to, struct page *from,
+		unsigned long vaddr, struct vm_area_struct *vma);
+
+static inline void __nocfi __cpu_clear_user_highpage(struct page *page,
+						     unsigned long vaddr)
+{
+	__glue(_USER,_clear_user_highpage)(page, vaddr);
+}
+
+static inline void __nocfi __cpu_copy_user_highpage(struct page *to,
+			struct page *from, unsigned long vaddr,
+			struct vm_area_struct *vma)
+{
+	__glue(_USER,_copy_user_highpage)(to, from, vaddr, vma);
+}
 
-extern void __cpu_clear_user_highpage(struct page *page, unsigned long vaddr);
-extern void __cpu_copy_user_highpage(struct page *to, struct page *from,
-			unsigned long vaddr, struct vm_area_struct *vma);
 #endif
 
 #define clear_user_highpage(page,vaddr)		\
diff --git a/arch/arm/mm/proc-syms.c b/arch/arm/mm/proc-syms.c
index e21249548e9f..c93fec38d9f4 100644
--- a/arch/arm/mm/proc-syms.c
+++ b/arch/arm/mm/proc-syms.c
@@ -31,14 +31,9 @@ EXPORT_SYMBOL(__cpuc_flush_dcache_area);
 EXPORT_SYMBOL(cpu_cache);
 #endif
 
-#ifdef CONFIG_MMU
-#ifndef MULTI_USER
-EXPORT_SYMBOL(__cpu_clear_user_highpage);
-EXPORT_SYMBOL(__cpu_copy_user_highpage);
-#else
+#if defined(CONFIG_MMU) && defined(MULTI_USER)
 EXPORT_SYMBOL(cpu_user);
 #endif
-#endif
 
 /*
  * No module should need to touch the TLB (and currently

-- 
2.34.1

Re: [PATCH 0/7] CFI for ARM32 using LLVM

[Kees Cook]

On Sun, Feb 25, 2024 at 09:08:09PM +0100, Linus Walleij wrote:
> This is a first patch set to support CLANG CFI (Control Flow
> Integrity) on ARM32.

Yay!

Is CONFIG_CFI_PERMISSIVE=y expected to work with this series?

I wasn't able to build with CONFIG_FTRACE=y; I got this link error:

ld.lld: error: undefined symbol: ftrace_stub_graph

(FWIW, I'm building against v6.8-rc2, maybe I need a different base?)

But yes, I can boot and prototype mismatches are caught. Whee! :)

Tested-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook

Re: [PATCH 0/7] CFI for ARM32 using LLVM

[Linus Walleij]

On Tue, Feb 27, 2024 at 2:06 AM Kees Cook <keescook@chromium.org> wrote:

> On Sun, Feb 25, 2024 at 09:08:09PM +0100, Linus Walleij wrote:
> > This is a first patch set to support CLANG CFI (Control Flow
> > Integrity) on ARM32.
>
> Yay!
>
> Is CONFIG_CFI_PERMISSIVE=y expected to work with this series?

I enable that and what happens when I trigger a crash is that the
process shell is killed and I return to login prompt (busybox).
I guess that is expected behaviour for permissive?

> I wasn't able to build with CONFIG_FTRACE=y; I got this link error:
> ld.lld: error: undefined symbol: ftrace_stub_graph
> (FWIW, I'm building against v6.8-rc2, maybe I need a different base?)

I get that too, from the buildbots, but I'm clueless about what it is.
All of it is in generic code, nothing ARM32-specific, maybe some bug
in v6.8-rc1 that was fixed (I wish...) Sami?

> But yes, I can boot and prototype mismatches are caught. Whee! :)

Sweet!

Yours,
Linus Walleij

Re: [PATCH 0/7] CFI for ARM32 using LLVM

[Sami Tolvanen]

On Tue, Feb 27, 2024 at 1:48 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Feb 27, 2024 at 2:06 AM Kees Cook <keescook@chromium.org> wrote:
>
> > I wasn't able to build with CONFIG_FTRACE=y; I got this link error:
> > ld.lld: error: undefined symbol: ftrace_stub_graph
> > (FWIW, I'm building against v6.8-rc2, maybe I need a different base?)
>
> I get that too, from the buildbots, but I'm clueless about what it is.
> All of it is in generic code, nothing ARM32-specific, maybe some bug
> in v6.8-rc1 that was fixed (I wish...) Sami?

You need to just define a separate stub for graph tracing with KCFI to
avoid type mismatches. Something like this:

https://lore.kernel.org/all/20230710183544.999540-11-samitolvanen@google.com/

Sami

Re: [PATCH 0/7] CFI for ARM32 using LLVM

[Linus Walleij]

On Tue, Feb 27, 2024 at 6:44 PM Sami Tolvanen <samitolvanen@google.com> wrote:
> On Tue, Feb 27, 2024 at 1:48 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Tue, Feb 27, 2024 at 2:06 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > > I wasn't able to build with CONFIG_FTRACE=y; I got this link error:
> > > ld.lld: error: undefined symbol: ftrace_stub_graph
> > > (FWIW, I'm building against v6.8-rc2, maybe I need a different base?)
> >
> > I get that too, from the buildbots, but I'm clueless about what it is.
> > All of it is in generic code, nothing ARM32-specific, maybe some bug
> > in v6.8-rc1 that was fixed (I wish...) Sami?
>
> You need to just define a separate stub for graph tracing with KCFI to
> avoid type mismatches. Something like this:
>
> https://lore.kernel.org/all/20230710183544.999540-11-samitolvanen@google.com/

I managed to fix this so all compiles work now! Thanks Sami.

Yours,
Linus Walleij

Re: [PATCH 0/7] CFI for ARM32 using LLVM

[Kees Cook]

On Tue, Feb 27, 2024 at 02:48:13PM +0100, Linus Walleij wrote:
> On Tue, Feb 27, 2024 at 2:06 AM Kees Cook <keescook@chromium.org> wrote:
> 
> > On Sun, Feb 25, 2024 at 09:08:09PM +0100, Linus Walleij wrote:
> > > This is a first patch set to support CLANG CFI (Control Flow
> > > Integrity) on ARM32.
> >
> > Yay!
> >
> > Is CONFIG_CFI_PERMISSIVE=y expected to work with this series?
> 
> I enable that and what happens when I trigger a crash is that the
> process shell is killed and I return to login prompt (busybox).
> I guess that is expected behaviour for permissive?

No, permissive should just issue a WARN and continue running. I think
you need wire up report_cfi_failure(), see cfi_handler() in arm64.

But non-permissive is working great! :)

-- 
Kees Cook

Re: [PATCH 0/7] CFI for ARM32 using LLVM

[Nathan Chancellor]

Hi Linus,

On Sun, Feb 25, 2024 at 09:08:09PM +0100, Linus Walleij wrote:
> This is a first patch set to support CLANG CFI (Control Flow
> Integrity) on ARM32.
> 
> For information about what CFI is, see:
> https://clang.llvm.org/docs/ControlFlowIntegrity.html
> 
> For the kernel KCFI flavor, see:
> https://lwn.net/Articles/898040/
> 
> The base changes required to bring up KCFI on ARM32 was mostly
> related to the use of custom vtables in the kernel, combined
> with defines to call into these vtable members directly from
> sites where they are used.
> 
> The approach to all of these vtable+define issues has been
> the same: instead of a define, wrap the call in a static inline
> function that explicitly calls the vtable member.
> 
> To runtime-test the patches:
> - Enable CONFIG_LKDTM
> - echo CFI_FORWARD_PROTO > /sys/kernel/debug/provoke-crash/DIRECT
> 
> The patch set has been booted to userspace on the following
> test platforms:
> 
> - Arm Versatile (QEMU)
> - Arm Versatile Express (QEMU)
> - multi_v7 booted on Versatile Express (QEMU)
> - Footbridge Netwinder (SA110 ARMv4)
> - Ux500 (ARMv7 SMP)
> 
> I am not saying there will not be corner cases that we need
> to fix in addition to this, but it is enough to get started.
> Looking at what was fixed for arm64 I am a bit weary that
> e.g. BPF might need something to trampoline properly.
> 
> But hopefullt people can get to testing it and help me fix
> remaining issues before the final version, or we can fix it
> in-tree.

This is awesome, thanks a lot for working on this!

I went to take this for a spin in QEMU using the virt platform. When
booting via EFI using edk2 in a manner similar to [1] but with 32-bit
ARM virtual firmware (which I implemented in our boot utility scripts
at [2]), I get a panic on boot even with CONFIG_CFI_PERMISSIVE=y:

  [    0.245260] Unhandled prefetch abort: breakpoint debug exception (0x002) at 0x00000000
  [    0.245924] Internal error: : 2 [#1] SMP ARM
  [    0.246211] Modules linked in:
  [    0.246486] CPU: 0 PID: 18 Comm: kworker/u2:1 Not tainted 6.8.0-rc6-00018-g7562486357bb #1
  [    0.246855] Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 2/2/2022
  [    0.247244] Workqueue: efi_rts_wq efi_call_rts
  [    0.247907] PC is at efi_call_rts+0x30c/0x330
  [    0.248158] LR is at efi_call_rts+0x1c/0x330
  [    0.248317] pc : [<c0fd9cb4>]    lr : [<c0fd99c4>]    psr: 000f0053
  [    0.248532] sp : e087df10  ip : 00000000  fp : c20f4205
  [    0.248728] r10: c20f4200  r9 : 600f0053  r8 : c036da10
  [    0.248906] r7 : 2017e01c  r6 : e427e123  r5 : e080dbc0  r4 : c1e1ac5c
  [    0.249182] r3 : 20101699  r2 : e080dbf8  r1 : e080dbf2  r0 : e080dbf4
  [    0.249468] Flags: nzcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
  [    0.249750] Control: 10c5387d  Table: 4206406a  DAC: 00000051
  [    0.250007] Register r0 information: 2-page vmalloc region starting at 0xe080c000 allocated at copy_process+0x14c/0xd24
  [    0.250574] Register r1 information: 2-page vmalloc region starting at 0xe080c000 allocated at copy_process+0x14c/0xd24
  [    0.250984] Register r2 information: 2-page vmalloc region starting at 0xe080c000 allocated at copy_process+0x14c/0xd24
  [    0.251387] Register r3 information: non-paged memory
  [    0.251671] Register r4 information: non-slab/vmalloc memory
  [    0.251963] Register r5 information: 2-page vmalloc region starting at 0xe080c000 allocated at copy_process+0x14c/0xd24
  [    0.252373] Register r6 information: vmalloc memory
  [    0.252575] Register r7 information: non-paged memory
  [    0.252777] Register r8 information: non-slab/vmalloc memory
  [    0.252993] Register r9 information: non-paged memory
  [    0.253189] Register r10 information: slab kmalloc-256 start c20f4200 pointer offset 0 size 256
  [    0.253832] Register r11 information: slab kmalloc-256 start c20f4200 pointer offset 5 size 256
  [    0.254199] Register r12 information: NULL pointer
  [    0.254404] Process kworker/u2:1 (pid: 18, stack limit = 0x(ptrval))
  [    0.254838] Stack: (0xe087df10 to 0xe087e000)
  [    0.255082] df00:                                     00000000 df7dec40 c20bb200 c20f4280
  [    0.255399] df20: c20f4205 c2005000 c20bb22c c1e1ac64 c20f4205 c036da10 c2005000 c20bb218
  [    0.255685] df40: c20bb250 00000040 c2005000 c20bb22c c20bb22c c2005020 c2049100 c20bb200
  [    0.255992] df60: 61c88647 c036e094 c20bb200 00000040 c2049784 c20b8654 c20b8640 c036dd54
  [    0.256289] df80: c20bb200 c2049100 00000000 c0374268 c20b8540 c0374130 00000000 00000000
  [    0.256581] dfa0: 00000000 00000000 00000000 c03001ac 00000000 00000000 00000000 00000000
  [    0.256883] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
  [    0.257169] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
  [    0.257986]  efi_call_rts from process_scheduled_works+0x298/0x3d0
  [    0.258441]  process_scheduled_works from worker_thread+0x340/0x514
  [    0.258688]  worker_thread from kthread+0x138/0x164
  [    0.258888]  kthread from ret_from_fork+0x14/0x28
  [    0.259096] Exception stack(0xe087dfb0 to 0xe087dff8)
  [    0.259284] dfa0:                                     00000000 00000000 00000000 00000000
  [    0.259581] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
  [    0.259875] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
  [    0.260271] Code: e3a0109b e3a03000 ebcdcf7a eaffffd7 (e1200070)
  [    0.260850] ---[ end trace 0000000000000000 ]---

It is not immediately obvious what EFI runtime call is triggering this,
I assume that has something to do with the lack of traps due to the
generic implementation. I'll see if I can dig more into this tomorrow
but don't hesitate to jump in too :P

[1]: https://mirrors.edge.kernel.org/pub/linux/kernel/people/will/docs/qemu/qemu-arm64-howto.html
[2]: https://github.com/ClangBuiltLinux/boot-utils/pull/118

Cheers,
Nathan

Re: [PATCH 0/7] CFI for ARM32 using LLVM

[Linus Walleij]

On Tue, Feb 27, 2024 at 5:21 AM Nathan Chancellor <nathan@kernel.org> wrote:

> I went to take this for a spin in QEMU using the virt platform. When
> booting via EFI using edk2 in a manner similar to [1] but with 32-bit
> ARM virtual firmware (which I implemented in our boot utility scripts
> at [2]), I get a panic on boot even with CONFIG_CFI_PERMISSIVE=y:
(...)
>   [    0.247907] PC is at efi_call_rts+0x30c/0x330
>   [    0.248158] LR is at efi_call_rts+0x1c/0x330
(...)
>   [    0.257986]  efi_call_rts from process_scheduled_works+0x298/0x3d0
(...)

> It is not immediately obvious what EFI runtime call is triggering this,

Not to me either...

> I assume that has something to do with the lack of traps due to the
> generic implementation. I'll see if I can dig more into this tomorrow
> but don't hesitate to jump in too :P

I think it's Ard Biesheuvel territory, he's always on top of EFI stuff :)

What I see is that efi_call_rts() calls
arch_efi_call_virt_setup/efi_call_virt_save_flags/
efi_call_virt (dispatch to arch_efi_call_virt)
/arch_efi_call_virt_teardown

arch_efi_call_virt_setup and arch_efi_call_virt_teardown
is there. So the default becomes arch_efi_call_virt from
include/linux/efi.h which is:

#define arch_efi_call_virt(p, f, args...)       ((p)->f(args))

CFI is not going to like that because it just dereferences this random
function pointer and calls it, I suppose?

I *think* that in arch/arm/include/asm/efi.h we need something
like...

#undef arch_efi_call_virt
#define arch_efi_call_virt(p, f, args...) \
        __efi_arm_wrapper((p)->f, #f, args)

Then __efi_arm_wrapper() can be a static inline to call the
function f, with the right arguments, tagged __nocfi?
The details beats me, I'm not good with variadic functions,
but if the solution is not obvious to you or Ard I can try to
dig into it.

Yours,
Linus Walleij