[PATCH 0/4] PAN for ARM32 using LPAE

[PATCH 0/4] PAN for ARM32 using LPAE

[Linus Walleij]

This is a patch set from Catalin that ended up on the back burner.

Since LPAE systems, i.e. ARM32 systems with a lot of physical memory,
will be with us for a while more, this is a pretty straight-forward
hardening measure that we should support.

The last patch explains the mechanism: since PAN using CPU domains
isn't available when using the LPAE MMU tables, we use the split
between the two translation base tables instead: TTBR0 is for
userspace pages and TTBR1 is for kernelspace tables. When executing
in kernelspace: we protect userspace by simply disabling page
walks in TTBR0.

This was tested by a simple hack in the ELF loader:

create_elf_tables()
+       unsigned char *test;
(...)
        if (copy_to_user(u_rand_bytes, k_rand_bytes, sizeof(k_rand_bytes)))
                return -EFAULT;
+       /* Cause a kernelspace access to userspace memory */
+       test = (char *)u_rand_bytes;
+       pr_info("Some byte: %02x\n", *test);

This tries to read a byte from userspace memory right after the
first unconditional copy_to_user(), a function that carefully
switches access permissions if we're using PAN.

Without LPAE PAN this will just happily print these bytes from
userspace but with LPAE PAN it will cause a predictable
crash:

Run /init as init process
Some byte: ac
8<--- cut here ---
Unable to handle kernel paging request at virtual address 7ec59f6b when read
[7ec59f6b] *pgd=82c3b003, *pmd=82863003, *pte=e00000882f6f5f
Internal error: Oops: 206 [#1] SMP ARM
CPU: 0 PID: 47 Comm: rc.init Not tainted 6.7.0-rc1+ #25
Hardware name: ARM-Versatile Express
PC is at create_elf_tables+0x13c/0x608

Thus we can show that LPAE PAN does its job.

Changes from Catalins initial patch set:

- Use IS_ENABLED() to avoid some ifdefs
- Create a uaccess_disabled() for classic CPU domains
  and reate a stub uaccess_disabled() for !PAN so we can
  always check this.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Catalin Marinas (4):
      ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h
      ARM: Move asm statements accessing TTBCR into C functions
      ARM: Reduce the number of #ifdef CONFIG_CPU_SW_DOMAIN_PAN
      ARM: Implement privileged no-access using TTBR0 page table walks disabling

 arch/arm/Kconfig                            | 22 ++++++++--
 arch/arm/include/asm/assembler.h            |  1 +
 arch/arm/include/asm/pgtable-3level-hwdef.h | 26 +++++++++++
 arch/arm/include/asm/proc-fns.h             | 12 +++++
 arch/arm/include/asm/uaccess-asm.h          | 58 ++++++++++++++++++++++--
 arch/arm/include/asm/uaccess.h              | 68 ++++++++++++++++++++++++++---
 arch/arm/kernel/suspend.c                   |  8 ++++
 arch/arm/lib/csumpartialcopyuser.S          | 20 ++++++++-
 arch/arm/mm/fault.c                         |  8 ++++
 arch/arm/mm/mmu.c                           |  7 ++-
 10 files changed, 212 insertions(+), 18 deletions(-)
---
base-commit: 8615ebf1370a798c403b4495f39de48270ad48f9
change-id: 20231216-arm32-lpae-pan-56125ab63d63

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


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

[PATCH 1/4] ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h

[Linus Walleij]

From: Catalin Marinas <catalin.marinas@arm.com>

These macros will be used in a subsequent patch.

At one point these were part of the ARM32 KVM but that is no
longer the case.

Since these macros are only relevant to LPAE kernel builds, they
are added to pgtable-3level-hwdef.h

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/include/asm/pgtable-3level-hwdef.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/include/asm/pgtable-3level-hwdef.h b/arch/arm/include/asm/pgtable-3level-hwdef.h
index 2f35b4eddaa8..19da7753a0b8 100644
--- a/arch/arm/include/asm/pgtable-3level-hwdef.h
+++ b/arch/arm/include/asm/pgtable-3level-hwdef.h
@@ -94,4 +94,21 @@
 
 #define TTBR1_SIZE	(((PAGE_OFFSET >> 30) - 1) << 16)
 
+/*
+ * TTBCR register bits.
+ */
+#define TTBCR_EAE		(1 << 31)
+#define TTBCR_IMP		(1 << 30)
+#define TTBCR_SH1_MASK		(3 << 28)
+#define TTBCR_ORGN1_MASK	(3 << 26)
+#define TTBCR_IRGN1_MASK	(3 << 24)
+#define TTBCR_EPD1		(1 << 23)
+#define TTBCR_A1		(1 << 22)
+#define TTBCR_T1SZ_MASK		(7 << 16)
+#define TTBCR_SH0_MASK		(3 << 12)
+#define TTBCR_ORGN0_MASK	(3 << 10)
+#define TTBCR_IRGN0_MASK	(3 << 8)
+#define TTBCR_EPD0		(1 << 7)
+#define TTBCR_T0SZ_MASK		(7 << 0)
+
 #endif

-- 
2.34.1


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

[PATCH 2/4] ARM: Move asm statements accessing TTBCR into C functions

[Linus Walleij]

From: Catalin Marinas <catalin.marinas@arm.com>

This patch implements cpu_get_ttbcr() and cpu_set_ttbcr() and replaces
the corresponding asm statements.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/include/asm/proc-fns.h | 12 ++++++++++++
 arch/arm/mm/mmu.c               |  7 +++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index 280396483f5d..a13e5cf3d1ff 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -178,6 +178,18 @@ extern void cpu_resume(void);
 	})
 #endif
 
+static inline unsigned int cpu_get_ttbcr(void)
+{
+	unsigned int ttbcr;
+	asm("mrc p15, 0, %0, c2, c0, 2" : "=r" (ttbcr));
+	return ttbcr;
+}
+
+static inline void cpu_set_ttbcr(unsigned int ttbcr)
+{
+	asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
+}
+
 #else	/*!CONFIG_MMU */
 
 #define cpu_switch_mm(pgd,mm)	{ }
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 674ed71573a8..9a780da6a4e1 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1687,9 +1687,8 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
 	 */
 	cr = get_cr();
 	set_cr(cr & ~(CR_I | CR_C));
-	asm("mrc p15, 0, %0, c2, c0, 2" : "=r" (ttbcr));
-	asm volatile("mcr p15, 0, %0, c2, c0, 2"
-		: : "r" (ttbcr & ~(3 << 8 | 3 << 10)));
+	ttbcr = cpu_get_ttbcr();
+	cpu_set_ttbcr(ttbcr & ~(3 << 8 | 3 << 10));
 	flush_cache_all();
 
 	/*
@@ -1701,7 +1700,7 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
 	lpae_pgtables_remap(offset, pa_pgd);
 
 	/* Re-enable the caches and cacheable TLB walks */
-	asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
+	cpu_set_ttbcr(ttbcr);
 	set_cr(cr);
 }
 

-- 
2.34.1


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

[PATCH 3/4] ARM: Reduce the number of #ifdef CONFIG_CPU_SW_DOMAIN_PAN

[Linus Walleij]

From: Catalin Marinas <catalin.marinas@arm.com>

This is a clean-up patch aimed at reducing the number of checks on
CONFIG_CPU_SW_DOMAIN_PAN, together with some empty lines for better
clarity once the CONFIG_CPU_TTBR0_PAN is introduced.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/include/asm/uaccess-asm.h | 16 ++++++++++++----
 arch/arm/include/asm/uaccess.h     | 21 +++++++++++++++------
 arch/arm/lib/csumpartialcopyuser.S |  6 +++++-
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/uaccess-asm.h b/arch/arm/include/asm/uaccess-asm.h
index 65da32e1f1c1..ea42ba25920f 100644
--- a/arch/arm/include/asm/uaccess-asm.h
+++ b/arch/arm/include/asm/uaccess-asm.h
@@ -39,8 +39,9 @@
 #endif
 	.endm
 
-	.macro	uaccess_disable, tmp, isb=1
 #ifdef CONFIG_CPU_SW_DOMAIN_PAN
+
+	.macro	uaccess_disable, tmp, isb=1
 	/*
 	 * Whenever we re-enter userspace, the domains should always be
 	 * set appropriately.
@@ -50,11 +51,9 @@
 	.if	\isb
 	instr_sync
 	.endif
-#endif
 	.endm
 
 	.macro	uaccess_enable, tmp, isb=1
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
 	/*
 	 * Whenever we re-enter userspace, the domains should always be
 	 * set appropriately.
@@ -64,9 +63,18 @@
 	.if	\isb
 	instr_sync
 	.endif
-#endif
 	.endm
 
+#else
+
+	.macro	uaccess_disable, tmp, isb=1
+	.endm
+
+	.macro	uaccess_enable, tmp, isb=1
+	.endm
+
+#endif
+
 #if defined(CONFIG_CPU_SW_DOMAIN_PAN) || defined(CONFIG_CPU_USE_DOMAINS)
 #define DACR(x...)	x
 #else
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 9556d04387f7..9b9234d1bb6a 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -24,9 +24,10 @@
  * perform such accesses (eg, via list poison values) which could then
  * be exploited for priviledge escalation.
  */
+#if defined(CONFIG_CPU_SW_DOMAIN_PAN)
+
 static __always_inline unsigned int uaccess_save_and_enable(void)
 {
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
 	unsigned int old_domain = get_domain();
 
 	/* Set the current domain access to permit user accesses */
@@ -34,19 +35,27 @@ static __always_inline unsigned int uaccess_save_and_enable(void)
 		   domain_val(DOMAIN_USER, DOMAIN_CLIENT));
 
 	return old_domain;
-#else
-	return 0;
-#endif
 }
 
 static __always_inline void uaccess_restore(unsigned int flags)
 {
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
 	/* Restore the user access mask */
 	set_domain(flags);
-#endif
 }
 
+#else
+
+static inline unsigned int uaccess_save_and_enable(void)
+{
+	return 0;
+}
+
+static inline void uaccess_restore(unsigned int flags)
+{
+}
+
+#endif
+
 /*
  * These two are intentionally not defined anywhere - if the kernel
  * code generates any references to them, that's a bug.
diff --git a/arch/arm/lib/csumpartialcopyuser.S b/arch/arm/lib/csumpartialcopyuser.S
index 6928781e6bee..04d8d9d741c7 100644
--- a/arch/arm/lib/csumpartialcopyuser.S
+++ b/arch/arm/lib/csumpartialcopyuser.S
@@ -13,7 +13,8 @@
 
 		.text
 
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
+#if defined(CONFIG_CPU_SW_DOMAIN_PAN)
+
 		.macro	save_regs
 		mrc	p15, 0, ip, c3, c0, 0
 		stmfd	sp!, {r1, r2, r4 - r8, ip, lr}
@@ -25,7 +26,9 @@
 		mcr	p15, 0, ip, c3, c0, 0
 		ret	lr
 		.endm
+
 #else
+
 		.macro	save_regs
 		stmfd	sp!, {r1, r2, r4 - r8, lr}
 		.endm
@@ -33,6 +36,7 @@
 		.macro	load_regs
 		ldmfd	sp!, {r1, r2, r4 - r8, pc}
 		.endm
+
 #endif
 
 		.macro	load1b,	reg1

-- 
2.34.1


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

[PATCH 4/4] ARM: Implement privileged no-access using TTBR0 page table walks disabling

[Linus Walleij]

From: Catalin Marinas <catalin.marinas@arm.com>

With LPAE enabled, privileged no-access cannot be enforced using CPU
domains as such feature is not available. This patch implements PAN
by disabling TTBR0 page table walks while in kernel mode.

The ARM architecture allows page table walks to be split between TTBR0
and TTBR1. With LPAE enabled, the split is defined by a combination of
TTBCR T0SZ and T1SZ bits. Currently, an LPAE-enabled kernel uses TTBR0
for user addresses and TTBR1 for kernel addresses with the VMSPLIT_2G
and VMSPLIT_3G configurations. The main advantage for the 3:1 split is
that TTBR1 is reduced to 2 levels, so potentially faster TLB refill
(though usually the first level entries are already cached in the TLB).

The PAN support on LPAE-enabled kernels uses TTBR0 when running in user
space or in kernel space during user access routines (TTBCR T0SZ and
T1SZ are both 0). When running user accesses are disabled in kernel
mode, TTBR0 page table walks are disabled by setting TTBCR.EPD0. TTBR1
is used for kernel accesses (including loadable modules; anything
covered by swapper_pg_dir) by reducing the TTBCR.T0SZ to the minimum
(2^(32-7) = 32MB). To avoid user accesses potentially hitting stale TLB
entries, the ASID is switched to 0 (reserved) by setting TTBCR.A1 and
using the ASID value in TTBR1. The difference from a non-PAN kernel is
that with the 3:1 memory split, TTBR1 always uses 3 levels of page
tables.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/Kconfig                            | 22 ++++++++++++--
 arch/arm/include/asm/assembler.h            |  1 +
 arch/arm/include/asm/pgtable-3level-hwdef.h |  9 ++++++
 arch/arm/include/asm/uaccess-asm.h          | 42 ++++++++++++++++++++++++++
 arch/arm/include/asm/uaccess.h              | 47 +++++++++++++++++++++++++++++
 arch/arm/kernel/suspend.c                   |  8 +++++
 arch/arm/lib/csumpartialcopyuser.S          | 14 +++++++++
 arch/arm/mm/fault.c                         |  8 +++++
 8 files changed, 148 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0af6709570d1..3d97a15a3e2d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1231,9 +1231,9 @@ config HIGHPTE
 	  consumed by page tables.  Setting this option will allow
 	  user-space 2nd level page tables to reside in high memory.
 
-config CPU_SW_DOMAIN_PAN
-	bool "Enable use of CPU domains to implement privileged no-access"
-	depends on MMU && !ARM_LPAE
+config ARM_PAN
+	bool "Enable privileged no-access"
+	depends on MMU
 	default y
 	help
 	  Increase kernel security by ensuring that normal kernel accesses
@@ -1242,10 +1242,26 @@ config CPU_SW_DOMAIN_PAN
 	  by ensuring that magic values (such as LIST_POISON) will always
 	  fault when dereferenced.
 
+	  The implementation uses CPU domains when !CONFIG_ARM_LPAE and
+	  disabling of TTBR0 page table walks with CONFIG_ARM_LPAE.
+
+config CPU_SW_DOMAIN_PAN
+	def_bool y
+	depends on ARM_PAN && !ARM_LPAE
+	help
+	  Enable use of CPU domains to implement privileged no-access.
+
 	  CPUs with low-vector mappings use a best-efforts implementation.
 	  Their lower 1MB needs to remain accessible for the vectors, but
 	  the remainder of userspace will become appropriately inaccessible.
 
+config CPU_TTBR0_PAN
+	def_bool y
+	depends on ARM_PAN && ARM_LPAE
+	help
+	  Enable privileged no-access by disabling TTBR0 page table walks when
+	  running in kernel mode.
+
 config HW_PERF_EVENTS
 	def_bool y
 	depends on ARM_PMU
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index aebe2c8f6a68..d33c1e24e00b 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -21,6 +21,7 @@
 #include <asm/opcodes-virt.h>
 #include <asm/asm-offsets.h>
 #include <asm/page.h>
+#include <asm/pgtable.h>
 #include <asm/thread_info.h>
 #include <asm/uaccess-asm.h>
 
diff --git a/arch/arm/include/asm/pgtable-3level-hwdef.h b/arch/arm/include/asm/pgtable-3level-hwdef.h
index 19da7753a0b8..323ad811732e 100644
--- a/arch/arm/include/asm/pgtable-3level-hwdef.h
+++ b/arch/arm/include/asm/pgtable-3level-hwdef.h
@@ -74,6 +74,7 @@
 #define PHYS_MASK_SHIFT		(40)
 #define PHYS_MASK		((1ULL << PHYS_MASK_SHIFT) - 1)
 
+#ifndef CONFIG_CPU_TTBR0_PAN
 /*
  * TTBR0/TTBR1 split (PAGE_OFFSET):
  *   0x40000000: T0SZ = 2, T1SZ = 0 (not used)
@@ -93,6 +94,14 @@
 #endif
 
 #define TTBR1_SIZE	(((PAGE_OFFSET >> 30) - 1) << 16)
+#else
+/*
+ * With CONFIG_CPU_TTBR0_PAN enabled, TTBR1 is only used during uaccess
+ * disabled regions when TTBR0 is disabled.
+ */
+#define TTBR1_OFFSET	0			/* pointing to swapper_pg_dir */
+#define TTBR1_SIZE	0			/* TTBR1 size controlled via TTBCR.T0SZ */
+#endif
 
 /*
  * TTBCR register bits.
diff --git a/arch/arm/include/asm/uaccess-asm.h b/arch/arm/include/asm/uaccess-asm.h
index ea42ba25920f..f7acf4cabbdc 100644
--- a/arch/arm/include/asm/uaccess-asm.h
+++ b/arch/arm/include/asm/uaccess-asm.h
@@ -65,6 +65,37 @@
 	.endif
 	.endm
 
+#elif defined(CONFIG_CPU_TTBR0_PAN)
+
+	.macro	uaccess_disable, tmp, isb=1
+	/*
+	 * Disable TTBR0 page table walks (EDP0 = 1), use the reserved ASID
+	 * from TTBR1 (A1 = 1) and enable TTBR1 page table walks for kernel
+	 * addresses by reducing TTBR0 range to 32MB (T0SZ = 7).
+	 */
+	mrc	p15, 0, \tmp, c2, c0, 2		@ read TTBCR
+	orr	\tmp, \tmp, #TTBCR_EPD0 | TTBCR_T0SZ_MASK
+	orr	\tmp, \tmp, #TTBCR_A1
+	mcr	p15, 0, \tmp, c2, c0, 2		@ write TTBCR
+	.if	\isb
+	instr_sync
+	.endif
+	.endm
+
+	.macro	uaccess_enable, tmp, isb=1
+	/*
+	 * Enable TTBR0 page table walks (T0SZ = 0, EDP0 = 0) and ASID from
+	 * TTBR0 (A1 = 0).
+	 */
+	mrc	p15, 0, \tmp, c2, c0, 2		@ read TTBCR
+	bic	\tmp, \tmp, #TTBCR_EPD0 | TTBCR_T0SZ_MASK
+	bic	\tmp, \tmp, #TTBCR_A1
+	mcr	p15, 0, \tmp, c2, c0, 2		@ write TTBCR
+	.if	\isb
+	instr_sync
+	.endif
+	.endm
+
 #else
 
 	.macro	uaccess_disable, tmp, isb=1
@@ -79,6 +110,12 @@
 #define DACR(x...)	x
 #else
 #define DACR(x...)
+#endif
+
+#ifdef CONFIG_CPU_TTBR0_PAN
+#define PAN(x...)	x
+#else
+#define PAN(x...)
 #endif
 
 	/*
@@ -94,6 +131,8 @@
 	.macro	uaccess_entry, tsk, tmp0, tmp1, tmp2, disable
  DACR(	mrc	p15, 0, \tmp0, c3, c0, 0)
  DACR(	str	\tmp0, [sp, #SVC_DACR])
+ PAN(	mrc	p15, 0, \tmp0, c2, c0, 2)
+ PAN(	str	\tmp0, [sp, #SVC_DACR])
 	.if \disable && IS_ENABLED(CONFIG_CPU_SW_DOMAIN_PAN)
 	/* kernel=client, user=no access */
 	mov	\tmp2, #DACR_UACCESS_DISABLE
@@ -112,8 +151,11 @@
 	.macro	uaccess_exit, tsk, tmp0, tmp1
  DACR(	ldr	\tmp0, [sp, #SVC_DACR])
  DACR(	mcr	p15, 0, \tmp0, c3, c0, 0)
+ PAN(	ldr	\tmp0, [sp, #SVC_DACR])
+ PAN(	mcr	p15, 0, \tmp0, c2, c0, 2)
 	.endm
 
 #undef DACR
+#undef PAN
 
 #endif /* __ASM_UACCESS_ASM_H__ */
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 9b9234d1bb6a..5b542eab009f 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -14,6 +14,8 @@
 #include <asm/domain.h>
 #include <asm/unaligned.h>
 #include <asm/unified.h>
+#include <asm/pgtable.h>
+#include <asm/proc-fns.h>
 #include <asm/compiler.h>
 
 #include <asm/extable.h>
@@ -43,6 +45,45 @@ static __always_inline void uaccess_restore(unsigned int flags)
 	set_domain(flags);
 }
 
+static inline bool uaccess_disabled(struct pt_regs *regs)
+{
+	/*
+	 * This is handled by hardware domain checks but included for
+	 * completeness.
+	 */
+	return !(get_domain() & domain_mask(DOMAIN_USER));
+}
+
+#elif defined(CONFIG_CPU_TTBR0_PAN)
+
+static inline unsigned int uaccess_save_and_enable(void)
+{
+	unsigned int old_ttbcr = cpu_get_ttbcr();
+
+	/*
+	 * Enable TTBR0 page table walks (T0SZ = 0, EDP0 = 0) and ASID from
+	 * TTBR0 (A1 = 0).
+	 */
+	cpu_set_ttbcr(old_ttbcr & ~(TTBCR_A1 | TTBCR_EPD0 | TTBCR_T0SZ_MASK));
+	isb();
+
+	return old_ttbcr;
+}
+
+static inline void uaccess_restore(unsigned int flags)
+{
+	cpu_set_ttbcr(flags);
+	isb();
+}
+
+static inline bool uaccess_disabled(struct pt_regs *regs)
+{
+	/* uaccess state saved above pt_regs on SVC exception entry */
+	unsigned int ttbcr = *(unsigned int *)(regs + 1);
+
+	return ttbcr & TTBCR_EPD0;
+}
+
 #else
 
 static inline unsigned int uaccess_save_and_enable(void)
@@ -54,6 +95,12 @@ static inline void uaccess_restore(unsigned int flags)
 {
 }
 
+static inline bool uaccess_disabled(struct pt_regs *regs)
+{
+	/* Without PAN userspace is always available */
+	return false;
+}
+
 #endif
 
 /*
diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
index c3ec3861dd07..58a6441b58c4 100644
--- a/arch/arm/kernel/suspend.c
+++ b/arch/arm/kernel/suspend.c
@@ -12,6 +12,7 @@
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 #include <asm/tlbflush.h>
+#include <asm/uaccess.h>
 
 extern int __cpu_suspend(unsigned long, int (*)(unsigned long), u32 cpuid);
 extern void cpu_resume_mmu(void);
@@ -26,6 +27,13 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 	if (!idmap_pgd)
 		return -EINVAL;
 
+	/*
+	 * Needed for the MMU disabling/enabing code to be able to run from
+	 * TTBR0 addresses.
+	 */
+	if (IS_ENABLED(CONFIG_CPU_TTBR0_PAN))
+		uaccess_save_and_enable();
+
 	/*
 	 * Function graph tracer state gets incosistent when the kernel
 	 * calls functions that never return (aka suspend finishers) hence
diff --git a/arch/arm/lib/csumpartialcopyuser.S b/arch/arm/lib/csumpartialcopyuser.S
index 04d8d9d741c7..c289bde04743 100644
--- a/arch/arm/lib/csumpartialcopyuser.S
+++ b/arch/arm/lib/csumpartialcopyuser.S
@@ -27,6 +27,20 @@
 		ret	lr
 		.endm
 
+#elif defined(CONFIG_CPU_TTBR0_PAN)
+
+		.macro	save_regs
+		mrc	p15, 0, ip, c2, c0, 2		@ read TTBCR
+		stmfd	sp!, {r1, r2, r4 - r8, ip, lr}
+		uaccess_enable ip
+		.endm
+
+		.macro	load_regs
+		ldmfd	sp!, {r1, r2, r4 - r8, ip, lr}
+		mcr	p15, 0, ip, c2, c0, 2		@ restore TTBCR
+		ret	lr
+		.endm
+
 #else
 
 		.macro	save_regs
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index e96fb40b9cc3..de4abf9dfd6a 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -278,6 +278,14 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, addr);
 
+	/*
+	 * Privileged access aborts with CONFIG_CPU_TTBR0_PAN enabled are
+	 * routed via the translation fault mechanism. Check whether uaccess
+	 * is disabled while in kernel mode.
+	 */
+	if (IS_ENABLED(CONFIG_CPU_TTBR0_PAN) && !user_mode(regs) && uaccess_disabled(regs))
+		goto no_context;
+
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 

-- 
2.34.1


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

Re: [PATCH 0/4] PAN for ARM32 using LPAE

[Russell King (Oracle)]

Second posting within seconds?

On Tue, Jan 23, 2024 at 10:16:13PM +0100, Linus Walleij wrote:
> This is a patch set from Catalin that ended up on the back burner.
> 
> Since LPAE systems, i.e. ARM32 systems with a lot of physical memory,
> will be with us for a while more, this is a pretty straight-forward
> hardening measure that we should support.
> 
> The last patch explains the mechanism: since PAN using CPU domains
> isn't available when using the LPAE MMU tables, we use the split
> between the two translation base tables instead: TTBR0 is for
> userspace pages and TTBR1 is for kernelspace tables. When executing
> in kernelspace: we protect userspace by simply disabling page
> walks in TTBR0.
> 
> This was tested by a simple hack in the ELF loader:
> 
> create_elf_tables()
> +       unsigned char *test;
> (...)
>         if (copy_to_user(u_rand_bytes, k_rand_bytes, sizeof(k_rand_bytes)))
>                 return -EFAULT;
> +       /* Cause a kernelspace access to userspace memory */
> +       test = (char *)u_rand_bytes;
> +       pr_info("Some byte: %02x\n", *test);
> 
> This tries to read a byte from userspace memory right after the
> first unconditional copy_to_user(), a function that carefully
> switches access permissions if we're using PAN.
> 
> Without LPAE PAN this will just happily print these bytes from
> userspace but with LPAE PAN it will cause a predictable
> crash:
> 
> Run /init as init process
> Some byte: ac
> 8<--- cut here ---
> Unable to handle kernel paging request at virtual address 7ec59f6b when read
> [7ec59f6b] *pgd=82c3b003, *pmd=82863003, *pte=e00000882f6f5f
> Internal error: Oops: 206 [#1] SMP ARM
> CPU: 0 PID: 47 Comm: rc.init Not tainted 6.7.0-rc1+ #25
> Hardware name: ARM-Versatile Express
> PC is at create_elf_tables+0x13c/0x608
> 
> Thus we can show that LPAE PAN does its job.
> 
> Changes from Catalins initial patch set:
> 
> - Use IS_ENABLED() to avoid some ifdefs
> - Create a uaccess_disabled() for classic CPU domains
>   and reate a stub uaccess_disabled() for !PAN so we can
>   always check this.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Catalin Marinas (4):
>       ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h
>       ARM: Move asm statements accessing TTBCR into C functions
>       ARM: Reduce the number of #ifdef CONFIG_CPU_SW_DOMAIN_PAN
>       ARM: Implement privileged no-access using TTBR0 page table walks disabling
> 
>  arch/arm/Kconfig                            | 22 ++++++++--
>  arch/arm/include/asm/assembler.h            |  1 +
>  arch/arm/include/asm/pgtable-3level-hwdef.h | 26 +++++++++++
>  arch/arm/include/asm/proc-fns.h             | 12 +++++
>  arch/arm/include/asm/uaccess-asm.h          | 58 ++++++++++++++++++++++--
>  arch/arm/include/asm/uaccess.h              | 68 ++++++++++++++++++++++++++---
>  arch/arm/kernel/suspend.c                   |  8 ++++
>  arch/arm/lib/csumpartialcopyuser.S          | 20 ++++++++-
>  arch/arm/mm/fault.c                         |  8 ++++
>  arch/arm/mm/mmu.c                           |  7 ++-
>  10 files changed, 212 insertions(+), 18 deletions(-)
> ---
> base-commit: 8615ebf1370a798c403b4495f39de48270ad48f9
> change-id: 20231216-arm32-lpae-pan-56125ab63d63
> 
> Best regards,
> -- 
> Linus Walleij <linus.walleij@linaro.org>
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

Re: [PATCH 0/4] PAN for ARM32 using LPAE

[Kees Cook]

On Tue, Jan 23, 2024 at 10:16:13PM +0100, Linus Walleij wrote:
> This is a patch set from Catalin that ended up on the back burner.
>
> Since LPAE systems, i.e. ARM32 systems with a lot of physical memory,
> will be with us for a while more, this is a pretty straight-forward
> hardening measure that we should support.
>
> The last patch explains the mechanism: since PAN using CPU domains
> isn't available when using the LPAE MMU tables, we use the split
> between the two translation base tables instead: TTBR0 is for
> userspace pages and TTBR1 is for kernelspace tables. When executing
> in kernelspace: we protect userspace by simply disabling page
> walks in TTBR0.
>
> This was tested by a simple hack in the ELF loader:
>
> create_elf_tables()
> +       unsigned char *test;
> (...)
>         if (copy_to_user(u_rand_bytes, k_rand_bytes, sizeof(k_rand_bytes)))
>                 return -EFAULT;
> +       /* Cause a kernelspace access to userspace memory */
> +       test = (char *)u_rand_bytes;
> +       pr_info("Some byte: %02x\n", *test);
>
> This tries to read a byte from userspace memory right after the
> first unconditional copy_to_user(), a function that carefully
> switches access permissions if we're using PAN.

You can also use CONFIG_LKDTM to test, with:

# echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
...
lkdtm: Performing direct entry ACCESS_USERSPACE
lkdtm: attempting bad read at 76fe9000
8<--- cut here ---
Unable to handle kernel paging request at virtual address 76fe9000 when read
[76fe9000] *pgd=45e47003, *pmd=43fd3003, *pte=a0000048af7f5f
Internal error: Oops: 206 [#1] SMP ARM
...

# echo "EXEC_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
...
lkdtm: Performing direct entry EXEC_USERSPACE
lkdtm: attempting ok execution at 8083707c
lkdtm: attempting bad execution at 76f38000
8<--- cut here ---
Unable to handle kernel paging request at virtual address 76f38000 when execute
[76f38000] *pgd=49ed2003, *pmd=49e19003, *pte=a00000494a5f5f
Internal error: Oops: 80000206 [#2] SMP ARM
...

I can confirm it works as expected. :)

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

Thanks!

-Kees

--
Kees Cook

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

Re: [PATCH 0/4] PAN for ARM32 using LPAE

[Kees Cook]

On Tue, Jan 23, 2024 at 09:28:34PM +0000, Russell King (Oracle) wrote:
> Second posting within seconds?

It looked to me like the first missed the mailing list CC.

-- 
Kees Cook

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

Re: [PATCH 0/4] PAN for ARM32 using LPAE

[Linus Walleij]

On Tue, Jan 23, 2024 at 10:29 PM Kees Cook <keescook@chromium.org> wrote:
> On Tue, Jan 23, 2024 at 09:28:34PM +0000, Russell King (Oracle) wrote:

> > Second posting within seconds?
>
> It looked to me like the first missed the mailing list CC.

Yeah I screwed up, sorry :(

Yours,
Linus Walleij

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

Re: [PATCH 2/4] ARM: Move asm statements accessing TTBCR into C functions

[Ard Biesheuvel]

On Tue, 23 Jan 2024 at 22:16, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> This patch implements cpu_get_ttbcr() and cpu_set_ttbcr() and replaces
> the corresponding asm statements.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/arm/include/asm/proc-fns.h | 12 ++++++++++++
>  arch/arm/mm/mmu.c               |  7 +++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index 280396483f5d..a13e5cf3d1ff 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -178,6 +178,18 @@ extern void cpu_resume(void);
>         })
>  #endif
>
> +static inline unsigned int cpu_get_ttbcr(void)
> +{
> +       unsigned int ttbcr;
> +       asm("mrc p15, 0, %0, c2, c0, 2" : "=r" (ttbcr));
> +       return ttbcr;
> +}
> +
> +static inline void cpu_set_ttbcr(unsigned int ttbcr)
> +{
> +       asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));

Nit: the 'volatile' is unnecessary here - there are no output
operands, so the compiler has to assume that the statement has side
effects.

> +}
> +
>  #else  /*!CONFIG_MMU */
>
>  #define cpu_switch_mm(pgd,mm)  { }
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 674ed71573a8..9a780da6a4e1 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1687,9 +1687,8 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
>          */
>         cr = get_cr();
>         set_cr(cr & ~(CR_I | CR_C));
> -       asm("mrc p15, 0, %0, c2, c0, 2" : "=r" (ttbcr));
> -       asm volatile("mcr p15, 0, %0, c2, c0, 2"
> -               : : "r" (ttbcr & ~(3 << 8 | 3 << 10)));
> +       ttbcr = cpu_get_ttbcr();
> +       cpu_set_ttbcr(ttbcr & ~(3 << 8 | 3 << 10));
>         flush_cache_all();
>
>         /*
> @@ -1701,7 +1700,7 @@ static void __init early_paging_init(const struct machine_desc *mdesc)
>         lpae_pgtables_remap(offset, pa_pgd);
>
>         /* Re-enable the caches and cacheable TLB walks */
> -       asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> +       cpu_set_ttbcr(ttbcr);
>         set_cr(cr);
>  }
>
>
> --
> 2.34.1
>

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

Re: [PATCH 2/4] ARM: Move asm statements accessing TTBCR into C functions

[Linus Walleij]

On Wed, Feb 14, 2024 at 4:23 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Tue, 23 Jan 2024 at 22:16, Linus Walleij <linus.walleij@linaro.org> wrote:

> > +static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > +{
> > +       asm volatile("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
>
> Nit: the 'volatile' is unnecessary here - there are no output
> operands, so the compiler has to assume that the statement has side
> effects.

Somehow I missed this in my v2 patch set, I fixed it for v3 now.

Yours,
Linus Walleij

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