linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] PAN for ARM32 using LPAE
@ 2024-03-12 12:52 Linus Walleij
  2024-03-12 12:52 ` [PATCH v3 1/4] ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h Linus Walleij
                   ` (4 more replies)
  0 siblings, 5 replies; 49+ messages in thread
From: Linus Walleij @ 2024-03-12 12:52 UTC (permalink / raw)
  To: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, Geert Uytterhoeven
  Cc: linux-arm-kernel, Linus Walleij, Catalin Marinas

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.

The simplest way to test a PAN crash:
- Enable CONFIG_LKDTM
- echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
- echo "EXEC_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT

This was also 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.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v3:
- Drop leftover uaccess_disabled() stub.
- Drop unnecesary volatile from the asm(mcr) call.
- Move all changes of ifdef to if defined() to the last patch
  and keep the preceding patch to its subject.
- Link to v2: https://lore.kernel.org/r/20240221-arm32-lpae-pan-v2-0-991096bba5d8@linaro.org

Changes in v1 (from Catalins original patch set):
- Use IS_ENABLED() to avoid some ifdefs
Changes in v2:
- Make the TTBCR a separate field in struct svc_pt_regs as requested
  by Russell. Adjust code accordingly.
- Push the MM page fault permission check into a local function
  and avoid the too generic uaccess_disabled() as requested by Ard.
- Link to v1: https://lore.kernel.org/r/20240123-arm32-lpae-pan-v1-0-7ea98a20514c@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 PAN for LPAE by TTBR0 page table walks disablement

 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/ptrace.h               |  1 +
 arch/arm/include/asm/uaccess-asm.h          | 58 +++++++++++++++++++++++++++--
 arch/arm/include/asm/uaccess.h              | 45 +++++++++++++++++++---
 arch/arm/kernel/asm-offsets.c               |  1 +
 arch/arm/kernel/suspend.c                   |  8 ++++
 arch/arm/lib/csumpartialcopyuser.S          | 20 +++++++++-
 arch/arm/mm/fault.c                         | 29 +++++++++++++++
 arch/arm/mm/mmu.c                           |  7 ++--
 12 files changed, 212 insertions(+), 18 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [PATCH v3 1/4] ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h
  2024-03-12 12:52 [PATCH v3 0/4] PAN for ARM32 using LPAE Linus Walleij
@ 2024-03-12 12:52 ` Linus Walleij
  2024-03-12 12:52 ` [PATCH v3 2/4] ARM: Move asm statements accessing TTBCR into C functions Linus Walleij
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 49+ messages in thread
From: Linus Walleij @ 2024-03-12 12:52 UTC (permalink / raw)
  To: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, Geert Uytterhoeven
  Cc: linux-arm-kernel, Linus Walleij, Catalin Marinas

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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCH v3 2/4] ARM: Move asm statements accessing TTBCR into C functions
  2024-03-12 12:52 [PATCH v3 0/4] PAN for ARM32 using LPAE Linus Walleij
  2024-03-12 12:52 ` [PATCH v3 1/4] ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h Linus Walleij
@ 2024-03-12 12:52 ` Linus Walleij
  2024-03-12 12:52 ` [PATCH v3 3/4] ARM: Reduce the number of #ifdef CONFIG_CPU_SW_DOMAIN_PAN Linus Walleij
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 49+ messages in thread
From: Linus Walleij @ 2024-03-12 12:52 UTC (permalink / raw)
  To: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, Geert Uytterhoeven
  Cc: linux-arm-kernel, Linus Walleij, Catalin Marinas

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>
---
ChangeLog v1->v3:
- Drop unnecesary volatile from the asm(mcr) call.
---
 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..9b3105a2a5e0 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("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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCH v3 3/4] ARM: Reduce the number of #ifdef CONFIG_CPU_SW_DOMAIN_PAN
  2024-03-12 12:52 [PATCH v3 0/4] PAN for ARM32 using LPAE Linus Walleij
  2024-03-12 12:52 ` [PATCH v3 1/4] ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h Linus Walleij
  2024-03-12 12:52 ` [PATCH v3 2/4] ARM: Move asm statements accessing TTBCR into C functions Linus Walleij
@ 2024-03-12 12:52 ` Linus Walleij
  2024-03-12 12:52 ` [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement Linus Walleij
  2024-03-12 17:45 ` [PATCH v3 0/4] PAN for ARM32 using LPAE Florian Fainelli
  4 siblings, 0 replies; 49+ messages in thread
From: Linus Walleij @ 2024-03-12 12:52 UTC (permalink / raw)
  To: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, Geert Uytterhoeven
  Cc: linux-arm-kernel, Linus Walleij, Catalin Marinas

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>
---
ChangeLog v2->v3:
- Don't ramdomly change ifdef to if defined(), let's save that
  for the last patch.
---
 arch/arm/include/asm/uaccess-asm.h | 16 ++++++++++++----
 arch/arm/include/asm/uaccess.h     | 21 +++++++++++++++------
 2 files changed, 27 insertions(+), 10 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..2278769f1156 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.
  */
+#ifdef 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.

-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-03-12 12:52 [PATCH v3 0/4] PAN for ARM32 using LPAE Linus Walleij
                   ` (2 preceding siblings ...)
  2024-03-12 12:52 ` [PATCH v3 3/4] ARM: Reduce the number of #ifdef CONFIG_CPU_SW_DOMAIN_PAN Linus Walleij
@ 2024-03-12 12:52 ` Linus Walleij
  2024-05-07 13:10   ` Geert Uytterhoeven
  2024-03-12 17:45 ` [PATCH v3 0/4] PAN for ARM32 using LPAE Florian Fainelli
  4 siblings, 1 reply; 49+ messages in thread
From: Linus Walleij @ 2024-03-12 12:52 UTC (permalink / raw)
  To: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, Geert Uytterhoeven
  Cc: linux-arm-kernel, Linus Walleij, Catalin Marinas

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.

As part of the change we are using preprocessor elif definied() clauses
so balance these clauses by converting relevant precedingt ifdef
clauses to if defined() clauses.

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>
---
ChangeLog v2->v3:
- Drop leftover uaccess_disabled() stub.
- Consistently change ifdef to if defined() in this patch instead
  of the previous patch.
- Convert a missing ifdef over to if defined() for consistency.
ChangeLog v1->v2:
- Make the SVC mode TTBCR a separate field in struct svc_pt_regs
  as requested by Russell.
- Push the MM page fault permission check into a local function
  and avoid the too generic uaccess_disabled() as requested by Ard.
---
 arch/arm/Kconfig                            | 22 +++++++++++++--
 arch/arm/include/asm/assembler.h            |  1 +
 arch/arm/include/asm/pgtable-3level-hwdef.h |  9 ++++++
 arch/arm/include/asm/ptrace.h               |  1 +
 arch/arm/include/asm/uaccess-asm.h          | 44 ++++++++++++++++++++++++++++-
 arch/arm/include/asm/uaccess.h              | 26 ++++++++++++++++-
 arch/arm/kernel/asm-offsets.c               |  1 +
 arch/arm/kernel/suspend.c                   |  8 ++++++
 arch/arm/lib/csumpartialcopyuser.S          | 20 ++++++++++++-
 arch/arm/mm/fault.c                         | 29 +++++++++++++++++++
 10 files changed, 155 insertions(+), 6 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/ptrace.h b/arch/arm/include/asm/ptrace.h
index 7f44e88d1f25..f064252498c7 100644
--- a/arch/arm/include/asm/ptrace.h
+++ b/arch/arm/include/asm/ptrace.h
@@ -19,6 +19,7 @@ struct pt_regs {
 struct svc_pt_regs {
 	struct pt_regs regs;
 	u32 dacr;
+	u32 ttbcr;
 };
 
 #define to_svc_pt_regs(r) container_of(r, struct svc_pt_regs, regs)
diff --git a/arch/arm/include/asm/uaccess-asm.h b/arch/arm/include/asm/uaccess-asm.h
index ea42ba25920f..4bccd895d954 100644
--- a/arch/arm/include/asm/uaccess-asm.h
+++ b/arch/arm/include/asm/uaccess-asm.h
@@ -39,7 +39,7 @@
 #endif
 	.endm
 
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
+#if defined(CONFIG_CPU_SW_DOMAIN_PAN)
 
 	.macro	uaccess_disable, tmp, isb=1
 	/*
@@ -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_TTBCR])
 	.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_TTBCR])
+ 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 2278769f1156..25d21d7d6e3e 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>
@@ -24,7 +26,7 @@
  * perform such accesses (eg, via list poison values) which could then
  * be exploited for priviledge escalation.
  */
-#ifdef CONFIG_CPU_SW_DOMAIN_PAN
+#if defined(CONFIG_CPU_SW_DOMAIN_PAN)
 
 static __always_inline unsigned int uaccess_save_and_enable(void)
 {
@@ -43,6 +45,28 @@ static __always_inline void uaccess_restore(unsigned int flags)
 	set_domain(flags);
 }
 
+#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();
+}
+
 #else
 
 static inline unsigned int uaccess_save_and_enable(void)
diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 219cbc7e5d13..dd2567ba987f 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -83,6 +83,7 @@ int main(void)
   DEFINE(S_OLD_R0,		offsetof(struct pt_regs, ARM_ORIG_r0));
   DEFINE(PT_REGS_SIZE,		sizeof(struct pt_regs));
   DEFINE(SVC_DACR,		offsetof(struct svc_pt_regs, dacr));
+  DEFINE(SVC_TTBCR,		offsetof(struct svc_pt_regs, ttbcr));
   DEFINE(SVC_REGS_SIZE,		sizeof(struct svc_pt_regs));
   BLANK();
   DEFINE(SIGFRAME_RC3_OFFSET,	offsetof(struct sigframe, retcode[3]));
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 6928781e6bee..c289bde04743 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,23 @@
 		mcr	p15, 0, ip, c3, c0, 0
 		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
 		stmfd	sp!, {r1, r2, r4 - r8, lr}
 		.endm
@@ -33,6 +50,7 @@
 		.macro	load_regs
 		ldmfd	sp!, {r1, r2, r4 - r8, pc}
 		.endm
+
 #endif
 
 		.macro	load1b,	reg1
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index e96fb40b9cc3..7d262a819ad1 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -235,6 +235,27 @@ static inline bool is_permission_fault(unsigned int fsr)
 	return false;
 }
 
+#ifdef CONFIG_CPU_TTBR0_PAN
+static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
+{
+	struct svc_pt_regs *svcregs;
+
+	/* If we are in user mode: permission granted */
+	if (user_mode(regs))
+		return true;
+
+	/* uaccess state saved above pt_regs on SVC exception entry */
+	svcregs = to_svc_pt_regs(regs);
+
+	return !(svcregs->ttbcr & TTBCR_EPD0);
+}
+#else
+static inline bool ttbr0_usermode_access_allowed(struct pt_regs *regs)
+{
+	return true;
+}
+#endif
+
 static int __kprobes
 do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 {
@@ -278,6 +299,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 (!ttbr0_usermode_access_allowed(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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 0/4] PAN for ARM32 using LPAE
  2024-03-12 12:52 [PATCH v3 0/4] PAN for ARM32 using LPAE Linus Walleij
                   ` (3 preceding siblings ...)
  2024-03-12 12:52 ` [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement Linus Walleij
@ 2024-03-12 17:45 ` Florian Fainelli
  2024-03-13  8:13   ` Linus Walleij
  4 siblings, 1 reply; 49+ messages in thread
From: Florian Fainelli @ 2024-03-12 17:45 UTC (permalink / raw)
  To: Linus Walleij, Russell King, Ard Biesheuvel, Arnd Bergmann,
	Stefan Wahren, Kees Cook, Geert Uytterhoeven
  Cc: linux-arm-kernel, Catalin Marinas

Hi Linus,

On 3/12/24 05:52, 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.
> 
> The simplest way to test a PAN crash:
> - Enable CONFIG_LKDTM
> - echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> - echo "EXEC_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> 
> This was also 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.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>

On ARCH_BRCMSTB, there was no noticeable performance drop with:

stress-ng --fault 0 --perf -t 1m

thanks a bunch for getting those patches out!
-- 
Florian


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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 0/4] PAN for ARM32 using LPAE
  2024-03-12 17:45 ` [PATCH v3 0/4] PAN for ARM32 using LPAE Florian Fainelli
@ 2024-03-13  8:13   ` Linus Walleij
  0 siblings, 0 replies; 49+ messages in thread
From: Linus Walleij @ 2024-03-13  8:13 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, Geert Uytterhoeven, linux-arm-kernel, Catalin Marinas

On Tue, Mar 12, 2024 at 6:45 PM Florian Fainelli <f.fainelli@gmail.com> wrote:

> Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>
>
> On ARCH_BRCMSTB, there was no noticeable performance drop with:
>
> stress-ng --fault 0 --perf -t 1m
>
> thanks a bunch for getting those patches out!

Thanks a lot Florian!

I think actually both the mitigations I have in the pipe (this one and
CFI) have low performance impact and are just nice to have (TM),
albeit CFI requires LLVM CLANG. Hopefully it will be possible to turn
on both.

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-03-12 12:52 ` [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement Linus Walleij
@ 2024-05-07 13:10   ` Geert Uytterhoeven
  2024-05-13 19:23     ` Linus Walleij
  0 siblings, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-07 13:10 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Linus,

On Tue, Mar 12, 2024 at 1:52 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> 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.
>
> As part of the change we are using preprocessor elif definied() clauses
> so balance these clauses by converting relevant precedingt ifdef
> clauses to if defined() clauses.
>
> 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>

Thanks for your patch, which is now commit 7af5b901e84743c6 ("ARM:
9358/2: Implement PAN for LPAE by TTBR0 page table walks disablement")
in arm/for-next (next-20240502 and later).

On Koelsch (R-Car M2-W with dual Cortex A15) with LPAE enabled:

    Run /sbin/init as init process
      with arguments:
        /sbin/init
      with environment:
        HOME=/
        TERM=linux
    Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
    CPU: 1 PID: 1 Comm: init Tainted: G        W        N
6.9.0-rc1-koelsch-00004-g7af5b901e847 #1930
    Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
    Call trace:
     unwind_backtrace from show_stack+0x10/0x14
     show_stack from dump_stack_lvl+0x78/0xa8
     dump_stack_lvl from panic+0x118/0x398
     panic from do_exit+0x1ec/0x938
     do_exit from sys_exit_group+0x0/0x10
    ---[ end Kernel panic - not syncing: Attempted to kill init!
exitcode=0x00000004 ]---

Disabling LPAE fixes the issue.

Note that shmobile_defconfig has CONFIG_LPAE=n, and thus works fine.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-07 13:10   ` Geert Uytterhoeven
@ 2024-05-13 19:23     ` Linus Walleij
  2024-05-13 19:58       ` Geert Uytterhoeven
  0 siblings, 1 reply; 49+ messages in thread
From: Linus Walleij @ 2024-05-13 19:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Geert,

On Tue, May 7, 2024 at 3:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Thanks for your patch, which is now commit 7af5b901e84743c6 ("ARM:
> 9358/2: Implement PAN for LPAE by TTBR0 page table walks disablement")
> in arm/for-next (next-20240502 and later).
>
> On Koelsch (R-Car M2-W with dual Cortex A15) with LPAE enabled:
>
>     Run /sbin/init as init process
>       with arguments:
>         /sbin/init
>       with environment:
>         HOME=/
>         TERM=linux
>     Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
>     CPU: 1 PID: 1 Comm: init Tainted: G        W        N
> 6.9.0-rc1-koelsch-00004-g7af5b901e847 #1930
>     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
>     Call trace:
>      unwind_backtrace from show_stack+0x10/0x14
>      show_stack from dump_stack_lvl+0x78/0xa8
>      dump_stack_lvl from panic+0x118/0x398
>      panic from do_exit+0x1ec/0x938
>      do_exit from sys_exit_group+0x0/0x10
>     ---[ end Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x00000004 ]---
>
> Disabling LPAE fixes the issue.

How annoying. I guess it doesn't help you that it works like a charm on
Versatile Express in QEMU, and also actually tested on the real
hardware. (Dual Cortex-A9).

So init is not executing, which userspace is this? I was  just testing
with busybox so far, maybe I need to test on something
bigger?

Do you have your ARMv7 file system available in an image or so
I can test it on Versatile Express?

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-13 19:23     ` Linus Walleij
@ 2024-05-13 19:58       ` Geert Uytterhoeven
  2024-05-13 20:29         ` Linus Walleij
  0 siblings, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-13 19:58 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Linus,

On Mon, May 13, 2024 at 9:24 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, May 7, 2024 at 3:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > Thanks for your patch, which is now commit 7af5b901e84743c6 ("ARM:
> > 9358/2: Implement PAN for LPAE by TTBR0 page table walks disablement")
> > in arm/for-next (next-20240502 and later).
> >
> > On Koelsch (R-Car M2-W with dual Cortex A15) with LPAE enabled:
> >
> >     Run /sbin/init as init process
> >       with arguments:
> >         /sbin/init
> >       with environment:
> >         HOME=/
> >         TERM=linux
> >     Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
> >     CPU: 1 PID: 1 Comm: init Tainted: G        W        N
> > 6.9.0-rc1-koelsch-00004-g7af5b901e847 #1930
> >     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> >     Call trace:
> >      unwind_backtrace from show_stack+0x10/0x14
> >      show_stack from dump_stack_lvl+0x78/0xa8
> >      dump_stack_lvl from panic+0x118/0x398
> >      panic from do_exit+0x1ec/0x938
> >      do_exit from sys_exit_group+0x0/0x10
> >     ---[ end Kernel panic - not syncing: Attempted to kill init!
> > exitcode=0x00000004 ]---
> >
> > Disabling LPAE fixes the issue.
>
> How annoying. I guess it doesn't help you that it works like a charm on
> Versatile Express in QEMU, and also actually tested on the real
> hardware. (Dual Cortex-A9).

Interesting. AFAIK Cortex-A9 does not support LPAE?

> So init is not executing, which userspace is this? I was  just testing
> with busybox so far, maybe I need to test on something
> bigger?
>
> Do you have your ARMv7 file system available in an image or so
> I can test it on Versatile Express?

It's just a Debian nfsroot.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-13 19:58       ` Geert Uytterhoeven
@ 2024-05-13 20:29         ` Linus Walleij
  2024-05-14  3:56           ` Florian Fainelli
                             ` (2 more replies)
  0 siblings, 3 replies; 49+ messages in thread
From: Linus Walleij @ 2024-05-13 20:29 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On Mon, May 13, 2024 at 9:58 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Mon, May 13, 2024 at 9:24 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Tue, May 7, 2024 at 3:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > Thanks for your patch, which is now commit 7af5b901e84743c6 ("ARM:
> > > 9358/2: Implement PAN for LPAE by TTBR0 page table walks disablement")
> > > in arm/for-next (next-20240502 and later).
> > >
> > > On Koelsch (R-Car M2-W with dual Cortex A15) with LPAE enabled:
> > >
> > >     Run /sbin/init as init process
> > >       with arguments:
> > >         /sbin/init
> > >       with environment:
> > >         HOME=/
> > >         TERM=linux
> > >     Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
> > >     CPU: 1 PID: 1 Comm: init Tainted: G        W        N
> > > 6.9.0-rc1-koelsch-00004-g7af5b901e847 #1930
> > >     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > >     Call trace:
> > >      unwind_backtrace from show_stack+0x10/0x14
> > >      show_stack from dump_stack_lvl+0x78/0xa8
> > >      dump_stack_lvl from panic+0x118/0x398
> > >      panic from do_exit+0x1ec/0x938
> > >      do_exit from sys_exit_group+0x0/0x10
> > >     ---[ end Kernel panic - not syncing: Attempted to kill init!
> > > exitcode=0x00000004 ]---
> > >
> > > Disabling LPAE fixes the issue.
> >
> > How annoying. I guess it doesn't help you that it works like a charm on
> > Versatile Express in QEMU, and also actually tested on the real
> > hardware. (Dual Cortex-A9).
>
> Interesting. AFAIK Cortex-A9 does not support LPAE?

Allright I was rambling, what I used (albeit early on) was
STMicro STM32MP157 which has Cortex-A7.

> > So init is not executing, which userspace is this? I was  just testing
> > with busybox so far, maybe I need to test on something
> > bigger?
> >
> > Do you have your ARMv7 file system available in an image or so
> > I can test it on Versatile Express?
>
> It's just a Debian nfsroot.

OK I tried with a vanilla ArchLinuxARM rootfs which uses systemd
and all that hoopla and it boots just fine.

So I'm a bit lost here.

I guess I should bring out the STM32MP157 board again and retest
to verify that that one hardware works with this. Any other ideas?

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-13 20:29         ` Linus Walleij
@ 2024-05-14  3:56           ` Florian Fainelli
  2024-05-14  8:14             ` Russell King (Oracle)
  2024-05-14  6:41           ` Geert Uytterhoeven
  2024-05-14  7:37           ` Linus Walleij
  2 siblings, 1 reply; 49+ messages in thread
From: Florian Fainelli @ 2024-05-14  3:56 UTC (permalink / raw)
  To: Linus Walleij, Geert Uytterhoeven
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Geert, Linus,

On 5/13/24 13:29, Linus Walleij wrote:
> On Mon, May 13, 2024 at 9:58 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Mon, May 13, 2024 at 9:24 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>>> On Tue, May 7, 2024 at 3:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> Thanks for your patch, which is now commit 7af5b901e84743c6 ("ARM:
>>>> 9358/2: Implement PAN for LPAE by TTBR0 page table walks disablement")
>>>> in arm/for-next (next-20240502 and later).
>>>>
>>>> On Koelsch (R-Car M2-W with dual Cortex A15) with LPAE enabled:
>>>>
>>>>      Run /sbin/init as init process
>>>>        with arguments:
>>>>          /sbin/init
>>>>        with environment:
>>>>          HOME=/
>>>>          TERM=linux
>>>>      Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
>>>>      CPU: 1 PID: 1 Comm: init Tainted: G        W        N
>>>> 6.9.0-rc1-koelsch-00004-g7af5b901e847 #1930
>>>>      Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
>>>>      Call trace:
>>>>       unwind_backtrace from show_stack+0x10/0x14
>>>>       show_stack from dump_stack_lvl+0x78/0xa8
>>>>       dump_stack_lvl from panic+0x118/0x398
>>>>       panic from do_exit+0x1ec/0x938
>>>>       do_exit from sys_exit_group+0x0/0x10
>>>>      ---[ end Kernel panic - not syncing: Attempted to kill init!
>>>> exitcode=0x00000004 ]---
>>>>
>>>> Disabling LPAE fixes the issue.
>>>
>>> How annoying. I guess it doesn't help you that it works like a charm on
>>> Versatile Express in QEMU, and also actually tested on the real
>>> hardware. (Dual Cortex-A9).
>>
>> Interesting. AFAIK Cortex-A9 does not support LPAE?
> 
> Allright I was rambling, what I used (albeit early on) was
> STMicro STM32MP157 which has Cortex-A7.
> 
>>> So init is not executing, which userspace is this? I was  just testing
>>> with busybox so far, maybe I need to test on something
>>> bigger?
>>>
>>> Do you have your ARMv7 file system available in an image or so
>>> I can test it on Versatile Express?
>>
>> It's just a Debian nfsroot.
> 
> OK I tried with a vanilla ArchLinuxARM rootfs which uses systemd
> and all that hoopla and it boots just fine.
> 
> So I'm a bit lost here.
> 
> I guess I should bring out the STM32MP157 board again and retest
> to verify that that one hardware works with this. Any other ideas?

FWIW, my ARCH_BRCMSTB systems using Brahma-B15, Brahma-B53 and 
Cortex-A72 CPUs are all happily booting the kernel to users-space with 
linux-next/master and CONFIG_CPU_TTBR0_PAN=y.

However I am not able to get my Raspberry Pi 4B to boot 
linux-next/master with CONFIG_CPU_TTBR0_PAN=y, too.

Error is similar to Geert, see below.

[   11.299106] Freeing unused kernel image (initmem) memory: 79872K
[   11.305720] Run /init as init process
[   11.314070] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x00000004
[   11.321888] CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-next-20240513 #32
[   11.328709] Hardware name: BCM2711
[   11.332169] Call trace:
[   11.332179]  unwind_backtrace from show_stack+0x10/0x14
[   11.340087]  show_stack from panic+0x20c/0x55c
[   11.344615]  panic from do_exit+0x6b0/0x1e74
[   11.348972]  do_exit from do_group_exit+0xcc/0x280
[   11.353857]  do_group_exit from get_signal+0xfb4/0x1340
[   11.359182]  get_signal from do_work_pending+0x2c0/0x7bc
[   11.364590]  do_work_pending from slow_work_pending+0xc/0x24
[   11.370351] Exception stack(0xf082bfb0 to 0xf082bff8)
[   11.375492] bfa0:                                     b6bca568 
00000000 003fa0d6 aedf3d20
[   11.383811] bfc0: aedf4a28 b6bca6f8 b6bca73c b6bca710 b6bca748 
b6bca6f8 aedf4a28 b6bca6f8
[   11.392127] bfe0: b6bca590 b6bca548 aeddae15 aedeb660 200f0030 ffffffff
[   11.398954] ---[ end Kernel panic - not syncing: Attempted to kill 
init! exitcode=0x00000004 ]---

I am in the process of running a diff between configurations because 
nothing obvious jumps out as being problematic so far.
--
Florian


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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-13 20:29         ` Linus Walleij
  2024-05-14  3:56           ` Florian Fainelli
@ 2024-05-14  6:41           ` Geert Uytterhoeven
  2024-05-14  7:46             ` Linus Walleij
  2024-05-14  7:37           ` Linus Walleij
  2 siblings, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-14  6:41 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Linus,

On Mon, May 13, 2024 at 10:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, May 13, 2024 at 9:58 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, May 13, 2024 at 9:24 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Tue, May 7, 2024 at 3:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > Thanks for your patch, which is now commit 7af5b901e84743c6 ("ARM:
> > > > 9358/2: Implement PAN for LPAE by TTBR0 page table walks disablement")
> > > > in arm/for-next (next-20240502 and later).
> > > >
> > > > On Koelsch (R-Car M2-W with dual Cortex A15) with LPAE enabled:
> > > >
> > > >     Run /sbin/init as init process
> > > >       with arguments:
> > > >         /sbin/init
> > > >       with environment:
> > > >         HOME=/
> > > >         TERM=linux
> > > >     Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
> > > >     CPU: 1 PID: 1 Comm: init Tainted: G        W        N
> > > > 6.9.0-rc1-koelsch-00004-g7af5b901e847 #1930
> > > >     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > >     Call trace:
> > > >      unwind_backtrace from show_stack+0x10/0x14
> > > >      show_stack from dump_stack_lvl+0x78/0xa8
> > > >      dump_stack_lvl from panic+0x118/0x398
> > > >      panic from do_exit+0x1ec/0x938
> > > >      do_exit from sys_exit_group+0x0/0x10
> > > >     ---[ end Kernel panic - not syncing: Attempted to kill init!
> > > > exitcode=0x00000004 ]---
> > > >
> > > > Disabling LPAE fixes the issue.
> > >
> > > How annoying. I guess it doesn't help you that it works like a charm on
> > > Versatile Express in QEMU, and also actually tested on the real
> > > hardware. (Dual Cortex-A9).
> >
> > Interesting. AFAIK Cortex-A9 does not support LPAE?
>
> Allright I was rambling, what I used (albeit early on) was
> STMicro STM32MP157 which has Cortex-A7.

I can reproduce the issue on Cortex-A7 (R-Car E2), too.

CONFIG_ARM_LPAE=y
CONFIG_CPU_TTBR0_PAN=y

> > > So init is not executing, which userspace is this? I was  just testing
> > > with busybox so far, maybe I need to test on something
> > > bigger?
> > >
> > > Do you have your ARMv7 file system available in an image or so
> > > I can test it on Versatile Express?
> >
> > It's just a Debian nfsroot.
>
> OK I tried with a vanilla ArchLinuxARM rootfs which uses systemd
> and all that hoopla and it boots just fine.
>
> So I'm a bit lost here.

I sent you a small initramfs by PM.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-13 20:29         ` Linus Walleij
  2024-05-14  3:56           ` Florian Fainelli
  2024-05-14  6:41           ` Geert Uytterhoeven
@ 2024-05-14  7:37           ` Linus Walleij
  2024-05-14 14:39             ` Catalin Marinas
  2 siblings, 1 reply; 49+ messages in thread
From: Linus Walleij @ 2024-05-14  7:37 UTC (permalink / raw)
  To: Geert Uytterhoeven, Catalin Marinas
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel

On Mon, May 13, 2024 at 10:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:

> I guess I should bring out the STM32MP157 board again and retest
> to verify that that one hardware works with this. Any other ideas?

I got this Cortex-A7 board out and booted with LPAE and PAN
using TTBR0 and it boots fine (some kind of OpenEmbedded/YOCTO
system is in it with systemd and all.

Tested with LKDTM provoked crashes and it behaves as expected.

Given Florians report it clearly works on some LPAE:s and not on some
others for some reason! I suppose we need to figure it out.

Catalin do you have some ideas?

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  6:41           ` Geert Uytterhoeven
@ 2024-05-14  7:46             ` Linus Walleij
  2024-05-14  7:59               ` Ard Biesheuvel
  0 siblings, 1 reply; 49+ messages in thread
From: Linus Walleij @ 2024-05-14  7:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Russell King, Ard Biesheuvel, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> I sent you a small initramfs by PM.

Booted this just fine on Vexpress QEMU:

Run /init as init process
sysctl: error: 'kernel.hotplug' is an unknown key


boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
/ # mount -t debugfs none /sys/kernel/debug
/ # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
lkdtm: Performing direct entry ACCESS_USERSPACE
lkdtm: attempting bad read at 76fea000
8<--- cut here ---
Unable to handle kernel paging request at virtual address 76fea000 when read
[76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
Internal error: Oops: 206 [#1] SMP ARM
CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
Hardware name: ARM-Versatile Express
PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138

I'm starting to think it is something about different LPAE implementations here.

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  7:46             ` Linus Walleij
@ 2024-05-14  7:59               ` Ard Biesheuvel
  2024-05-14  8:04                 ` Geert Uytterhoeven
  0 siblings, 1 reply; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-14  7:59 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Geert Uytterhoeven, Russell King, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> > I sent you a small initramfs by PM.
>
> Booted this just fine on Vexpress QEMU:
>
> Run /init as init process
> sysctl: error: 'kernel.hotplug' is an unknown key
>
>
> boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
> / # mount -t debugfs none /sys/kernel/debug
> / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> lkdtm: Performing direct entry ACCESS_USERSPACE
> lkdtm: attempting bad read at 76fea000
> 8<--- cut here ---
> Unable to handle kernel paging request at virtual address 76fea000 when read
> [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
> Internal error: Oops: 206 [#1] SMP ARM
> CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
> Hardware name: ARM-Versatile Express
> PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
>
> I'm starting to think it is something about different LPAE implementations here.
>

I have built multi_v7_defconfig with the following enabled

CONFIG_ARM_LPAE=y
CONFIG_CPU_TTBR0_PAN=y
CONFIG_LKDTM=y

and the resulting kernel boots happily as a 32-bit VM running under a
Rpi4 KVM host.

Could someone post an actual .config that reproduces this? Rpi4 is
A72, which both works and doesn't work in Florian's testing, so I'd be
highly surprised if this is not a config issue.

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  7:59               ` Ard Biesheuvel
@ 2024-05-14  8:04                 ` Geert Uytterhoeven
  2024-05-14  8:25                   ` Ard Biesheuvel
  0 siblings, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-14  8:04 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linus Walleij, Russell King, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Ard,

On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > > I sent you a small initramfs by PM.
> >
> > Booted this just fine on Vexpress QEMU:
> >
> > Run /init as init process
> > sysctl: error: 'kernel.hotplug' is an unknown key
> >
> >
> > boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
> > / # mount -t debugfs none /sys/kernel/debug
> > / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> > lkdtm: Performing direct entry ACCESS_USERSPACE
> > lkdtm: attempting bad read at 76fea000
> > 8<--- cut here ---
> > Unable to handle kernel paging request at virtual address 76fea000 when read
> > [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
> > Internal error: Oops: 206 [#1] SMP ARM
> > CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
> > Hardware name: ARM-Versatile Express
> > PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> >
> > I'm starting to think it is something about different LPAE implementations here.
> >
>
> I have built multi_v7_defconfig with the following enabled
>
> CONFIG_ARM_LPAE=y
> CONFIG_CPU_TTBR0_PAN=y
> CONFIG_LKDTM=y
>
> and the resulting kernel boots happily as a 32-bit VM running under a
> Rpi4 KVM host.
>
> Could someone post an actual .config that reproduces this? Rpi4 is
> A72, which both works and doesn't work in Florian's testing, so I'd be
> highly surprised if this is not a config issue.

shmobile_defconfig with CONFIG_LPAE=y added failed for me before.

Building multi_v7_defconfig with the above enabled...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  3:56           ` Florian Fainelli
@ 2024-05-14  8:14             ` Russell King (Oracle)
  2024-05-14 11:22               ` Geert Uytterhoeven
  0 siblings, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-14  8:14 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Linus Walleij, Geert Uytterhoeven, Ard Biesheuvel, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

On Mon, May 13, 2024 at 08:56:20PM -0700, Florian Fainelli wrote:
> Hi Geert, Linus,
> 
> Error is similar to Geert, see below.
> 
> [   11.299106] Freeing unused kernel image (initmem) memory: 79872K
> [   11.305720] Run /init as init process
> [   11.314070] Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x00000004
> [   11.321888] CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-next-20240513 #32
> [   11.328709] Hardware name: BCM2711
> [   11.332169] Call trace:
> [   11.332179]  unwind_backtrace from show_stack+0x10/0x14
> [   11.340087]  show_stack from panic+0x20c/0x55c
> [   11.344615]  panic from do_exit+0x6b0/0x1e74
> [   11.348972]  do_exit from do_group_exit+0xcc/0x280
> [   11.353857]  do_group_exit from get_signal+0xfb4/0x1340
> [   11.359182]  get_signal from do_work_pending+0x2c0/0x7bc
> [   11.364590]  do_work_pending from slow_work_pending+0xc/0x24
> [   11.370351] Exception stack(0xf082bfb0 to 0xf082bff8)
> [   11.375492] bfa0:                                     b6bca568 00000000
> 003fa0d6 aedf3d20
> [   11.383811] bfc0: aedf4a28 b6bca6f8 b6bca73c b6bca710 b6bca748 b6bca6f8
> aedf4a28 b6bca6f8
> [   11.392127] bfe0: b6bca590 b6bca548 aeddae15 aedeb660 200f0030 ffffffff
> [   11.398954] ---[ end Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x00000004 ]---

You could enable CONFiG_DEBUG_USER, and then pass "user_debug=24" to
the kernel to get a report for the conditions that lead to SEGV/BUS
signals being delivered to a userspace processd.

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  8:04                 ` Geert Uytterhoeven
@ 2024-05-14  8:25                   ` Ard Biesheuvel
  2024-05-14  9:22                     ` Russell King (Oracle)
  2024-05-14 11:28                     ` Geert Uytterhoeven
  0 siblings, 2 replies; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-14  8:25 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linus Walleij, Russell King, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, 14 May 2024 at 10:04, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ard,
>
> On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >
> > > > I sent you a small initramfs by PM.
> > >
> > > Booted this just fine on Vexpress QEMU:
> > >
> > > Run /init as init process
> > > sysctl: error: 'kernel.hotplug' is an unknown key
> > >
> > >
> > > boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
> > > / # mount -t debugfs none /sys/kernel/debug
> > > / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> > > lkdtm: Performing direct entry ACCESS_USERSPACE
> > > lkdtm: attempting bad read at 76fea000
> > > 8<--- cut here ---
> > > Unable to handle kernel paging request at virtual address 76fea000 when read
> > > [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
> > > Internal error: Oops: 206 [#1] SMP ARM
> > > CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
> > > Hardware name: ARM-Versatile Express
> > > PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > >
> > > I'm starting to think it is something about different LPAE implementations here.
> > >
> >
> > I have built multi_v7_defconfig with the following enabled
> >
> > CONFIG_ARM_LPAE=y
> > CONFIG_CPU_TTBR0_PAN=y
> > CONFIG_LKDTM=y
> >
> > and the resulting kernel boots happily as a 32-bit VM running under a
> > Rpi4 KVM host.
> >
> > Could someone post an actual .config that reproduces this? Rpi4 is
> > A72, which both works and doesn't work in Florian's testing, so I'd be
> > highly surprised if this is not a config issue.
>
> shmobile_defconfig with CONFIG_LPAE=y added failed for me before.
>
> Building multi_v7_defconfig with the above enabled...
>

What are you passing as the command line?

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  8:25                   ` Ard Biesheuvel
@ 2024-05-14  9:22                     ` Russell King (Oracle)
  2024-05-14 11:40                       ` Linus Walleij
  2024-05-14 11:28                     ` Geert Uytterhoeven
  1 sibling, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-14  9:22 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Geert Uytterhoeven, Linus Walleij, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, May 14, 2024 at 10:25:28AM +0200, Ard Biesheuvel wrote:
> On Tue, 14 May 2024 at 10:04, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Ard,
> >
> > On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> > > > On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > >
> > > > > I sent you a small initramfs by PM.
> > > >
> > > > Booted this just fine on Vexpress QEMU:
> > > >
> > > > Run /init as init process
> > > > sysctl: error: 'kernel.hotplug' is an unknown key
> > > >
> > > >
> > > > boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
> > > > / # mount -t debugfs none /sys/kernel/debug
> > > > / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> > > > lkdtm: Performing direct entry ACCESS_USERSPACE
> > > > lkdtm: attempting bad read at 76fea000
> > > > 8<--- cut here ---
> > > > Unable to handle kernel paging request at virtual address 76fea000 when read
> > > > [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
> > > > Internal error: Oops: 206 [#1] SMP ARM
> > > > CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
> > > > Hardware name: ARM-Versatile Express
> > > > PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > > LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > >
> > > > I'm starting to think it is something about different LPAE implementations here.
> > > >
> > >
> > > I have built multi_v7_defconfig with the following enabled
> > >
> > > CONFIG_ARM_LPAE=y
> > > CONFIG_CPU_TTBR0_PAN=y
> > > CONFIG_LKDTM=y
> > >
> > > and the resulting kernel boots happily as a 32-bit VM running under a
> > > Rpi4 KVM host.
> > >
> > > Could someone post an actual .config that reproduces this? Rpi4 is
> > > A72, which both works and doesn't work in Florian's testing, so I'd be
> > > highly surprised if this is not a config issue.
> >
> > shmobile_defconfig with CONFIG_LPAE=y added failed for me before.
> >
> > Building multi_v7_defconfig with the above enabled...
> >
> 
> What are you passing as the command line?

A more relevant question at this point in time - what do people want me
to do with these patches given that we're now in the merge window?

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  8:14             ` Russell King (Oracle)
@ 2024-05-14 11:22               ` Geert Uytterhoeven
  2024-05-14 11:33                 ` Russell King (Oracle)
  0 siblings, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-14 11:22 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Florian Fainelli, Linus Walleij, Ard Biesheuvel, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Russell,

On Tue, May 14, 2024 at 10:15 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Mon, May 13, 2024 at 08:56:20PM -0700, Florian Fainelli wrote:
> > [   11.299106] Freeing unused kernel image (initmem) memory: 79872K
> > [   11.305720] Run /init as init process
> > [   11.314070] Kernel panic - not syncing: Attempted to kill init!
> > exitcode=0x00000004
> > [   11.321888] CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-next-20240513 #32
> > [   11.328709] Hardware name: BCM2711
> > [   11.332169] Call trace:
> > [   11.332179]  unwind_backtrace from show_stack+0x10/0x14
> > [   11.340087]  show_stack from panic+0x20c/0x55c
> > [   11.344615]  panic from do_exit+0x6b0/0x1e74
> > [   11.348972]  do_exit from do_group_exit+0xcc/0x280
> > [   11.353857]  do_group_exit from get_signal+0xfb4/0x1340
> > [   11.359182]  get_signal from do_work_pending+0x2c0/0x7bc
> > [   11.364590]  do_work_pending from slow_work_pending+0xc/0x24
> > [   11.370351] Exception stack(0xf082bfb0 to 0xf082bff8)
> > [   11.375492] bfa0:                                     b6bca568 00000000
> > 003fa0d6 aedf3d20
> > [   11.383811] bfc0: aedf4a28 b6bca6f8 b6bca73c b6bca710 b6bca748 b6bca6f8
> > aedf4a28 b6bca6f8
> > [   11.392127] bfe0: b6bca590 b6bca548 aeddae15 aedeb660 200f0030 ffffffff
> > [   11.398954] ---[ end Kernel panic - not syncing: Attempted to kill init!
> > exitcode=0x00000004 ]---
>
> You could enable CONFiG_DEBUG_USER, and then pass "user_debug=24" to
> the kernel to get a report for the conditions that lead to SEGV/BUS
> signals being delivered to a userspace processd.

That does not seem to make any difference for me, i.e. no report?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  8:25                   ` Ard Biesheuvel
  2024-05-14  9:22                     ` Russell King (Oracle)
@ 2024-05-14 11:28                     ` Geert Uytterhoeven
  2024-05-14 16:06                       ` Geert Uytterhoeven
  1 sibling, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-14 11:28 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linus Walleij, Russell King, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Ard,

On Tue, May 14, 2024 at 10:25 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Tue, 14 May 2024 at 10:04, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> > > > On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > >
> > > > > I sent you a small initramfs by PM.
> > > >
> > > > Booted this just fine on Vexpress QEMU:
> > > >
> > > > Run /init as init process
> > > > sysctl: error: 'kernel.hotplug' is an unknown key
> > > >
> > > >
> > > > boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
> > > > / # mount -t debugfs none /sys/kernel/debug
> > > > / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> > > > lkdtm: Performing direct entry ACCESS_USERSPACE
> > > > lkdtm: attempting bad read at 76fea000
> > > > 8<--- cut here ---
> > > > Unable to handle kernel paging request at virtual address 76fea000 when read
> > > > [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
> > > > Internal error: Oops: 206 [#1] SMP ARM
> > > > CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
> > > > Hardware name: ARM-Versatile Express
> > > > PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > > LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > >
> > > > I'm starting to think it is something about different LPAE implementations here.
> > >
> > > I have built multi_v7_defconfig with the following enabled
> > >
> > > CONFIG_ARM_LPAE=y
> > > CONFIG_CPU_TTBR0_PAN=y
> > > CONFIG_LKDTM=y
> > >
> > > and the resulting kernel boots happily as a 32-bit VM running under a
> > > Rpi4 KVM host.
> > >
> > > Could someone post an actual .config that reproduces this? Rpi4 is
> > > A72, which both works and doesn't work in Florian's testing, so I'd be
> > > highly surprised if this is not a config issue.
> >
> > shmobile_defconfig with CONFIG_LPAE=y added failed for me before.
> >
> > Building multi_v7_defconfig with the above enabled...

And that works, while shmobile_defconfig with the above does not!
So it is config-related.

FTR, plain multi_v7_defconfig also works.

> What are you passing as the command line?

ignore_loglevel ip=dhcp root=/dev/nfs rw nfsroot=<ip>:<path>/debian-armhf,tcp,v3

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 11:22               ` Geert Uytterhoeven
@ 2024-05-14 11:33                 ` Russell King (Oracle)
  2024-05-14 12:32                   ` Geert Uytterhoeven
  0 siblings, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-14 11:33 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Fainelli, Linus Walleij, Ard Biesheuvel, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, May 14, 2024 at 01:22:36PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Tue, May 14, 2024 at 10:15 AM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > On Mon, May 13, 2024 at 08:56:20PM -0700, Florian Fainelli wrote:
> > > [   11.299106] Freeing unused kernel image (initmem) memory: 79872K
> > > [   11.305720] Run /init as init process
> > > [   11.314070] Kernel panic - not syncing: Attempted to kill init!
> > > exitcode=0x00000004
> > > [   11.321888] CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-next-20240513 #32
> > > [   11.328709] Hardware name: BCM2711
> > > [   11.332169] Call trace:
> > > [   11.332179]  unwind_backtrace from show_stack+0x10/0x14
> > > [   11.340087]  show_stack from panic+0x20c/0x55c
> > > [   11.344615]  panic from do_exit+0x6b0/0x1e74
> > > [   11.348972]  do_exit from do_group_exit+0xcc/0x280
> > > [   11.353857]  do_group_exit from get_signal+0xfb4/0x1340
> > > [   11.359182]  get_signal from do_work_pending+0x2c0/0x7bc
> > > [   11.364590]  do_work_pending from slow_work_pending+0xc/0x24
> > > [   11.370351] Exception stack(0xf082bfb0 to 0xf082bff8)
> > > [   11.375492] bfa0:                                     b6bca568 00000000
> > > 003fa0d6 aedf3d20
> > > [   11.383811] bfc0: aedf4a28 b6bca6f8 b6bca73c b6bca710 b6bca748 b6bca6f8
> > > aedf4a28 b6bca6f8
> > > [   11.392127] bfe0: b6bca590 b6bca548 aeddae15 aedeb660 200f0030 ffffffff
> > > [   11.398954] ---[ end Kernel panic - not syncing: Attempted to kill init!
> > > exitcode=0x00000004 ]---
> >
> > You could enable CONFiG_DEBUG_USER, and then pass "user_debug=24" to
> > the kernel to get a report for the conditions that lead to SEGV/BUS
> > signals being delivered to a userspace processd.
> 
> That does not seem to make any difference for me, i.e. no report?

Then it's not a SEGV/BUS (iow page fault.) Please try user_debug=31
in that case. Thanks.

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  9:22                     ` Russell King (Oracle)
@ 2024-05-14 11:40                       ` Linus Walleij
  0 siblings, 0 replies; 49+ messages in thread
From: Linus Walleij @ 2024-05-14 11:40 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Ard Biesheuvel, Geert Uytterhoeven, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, May 14, 2024 at 11:22 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:

> A more relevant question at this point in time - what do people want me
> to do with these patches given that we're now in the merge window?

Let's see if it is as simple as it seems (such as a missing Kconfig select
or depends on).

If we can't figure it out let's revert the last patch in <timespan> where
it's best of you decide on the <timespan> I'd say, you will know when
the pull request needs to go off.

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 11:33                 ` Russell King (Oracle)
@ 2024-05-14 12:32                   ` Geert Uytterhoeven
  2024-05-14 12:38                     ` Russell King (Oracle)
  0 siblings, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-14 12:32 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Florian Fainelli, Linus Walleij, Ard Biesheuvel, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

Hi Russell,

On Tue, May 14, 2024 at 1:33 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Tue, May 14, 2024 at 01:22:36PM +0200, Geert Uytterhoeven wrote:
> > On Tue, May 14, 2024 at 10:15 AM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > > On Mon, May 13, 2024 at 08:56:20PM -0700, Florian Fainelli wrote:
> > > > [   11.299106] Freeing unused kernel image (initmem) memory: 79872K
> > > > [   11.305720] Run /init as init process
> > > > [   11.314070] Kernel panic - not syncing: Attempted to kill init!
> > > > exitcode=0x00000004
> > > > [   11.321888] CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-next-20240513 #32
> > > > [   11.328709] Hardware name: BCM2711
> > > > [   11.332169] Call trace:
> > > > [   11.332179]  unwind_backtrace from show_stack+0x10/0x14
> > > > [   11.340087]  show_stack from panic+0x20c/0x55c
> > > > [   11.344615]  panic from do_exit+0x6b0/0x1e74
> > > > [   11.348972]  do_exit from do_group_exit+0xcc/0x280
> > > > [   11.353857]  do_group_exit from get_signal+0xfb4/0x1340
> > > > [   11.359182]  get_signal from do_work_pending+0x2c0/0x7bc
> > > > [   11.364590]  do_work_pending from slow_work_pending+0xc/0x24
> > > > [   11.370351] Exception stack(0xf082bfb0 to 0xf082bff8)
> > > > [   11.375492] bfa0:                                     b6bca568 00000000
> > > > 003fa0d6 aedf3d20
> > > > [   11.383811] bfc0: aedf4a28 b6bca6f8 b6bca73c b6bca710 b6bca748 b6bca6f8
> > > > aedf4a28 b6bca6f8
> > > > [   11.392127] bfe0: b6bca590 b6bca548 aeddae15 aedeb660 200f0030 ffffffff
> > > > [   11.398954] ---[ end Kernel panic - not syncing: Attempted to kill init!
> > > > exitcode=0x00000004 ]---
> > >
> > > You could enable CONFiG_DEBUG_USER, and then pass "user_debug=24" to
> > > the kernel to get a report for the conditions that lead to SEGV/BUS
> > > signals being delivered to a userspace processd.
> >
> > That does not seem to make any difference for me, i.e. no report?
>
> Then it's not a SEGV/BUS (iow page fault.) Please try user_debug=31
> in that case. Thanks.

Thanks, much better:

    init (1): undefined instruction: pc=b6f4feda
    CPU: 1 PID: 1 Comm: init Not tainted
6.9.0-shmobile-09158-g1218ffc3659e #1820
    Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
    PC is at 0xb6f4feda
    LR is at 0xb6f4ed31
    pc : [<b6f4feda>]    lr : [<b6f4ed31>]    psr: 60000030
    sp : be970630  ip : be970678  fp : b6f67978
    r10: 00000000  r9 : 004d48ff  r8 : be970844
    r7 : be9707f8  r6 : b6f67978  r5 : be970850  r4 : be970844
    r3 : b6f669b0  r2 : 003fb0d6  r1 : 00000000  r0 : be970650
    Flags: nZCv  IRQs on  FIQs on  Mode USER_32  ISA Thumb  Segment user
    Control: 30c5387d  Table: 41f6cac0  DAC: 55555555
    Code: bad PC value

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 12:32                   ` Geert Uytterhoeven
@ 2024-05-14 12:38                     ` Russell King (Oracle)
  2024-05-14 15:03                       ` Catalin Marinas
  0 siblings, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-14 12:38 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Fainelli, Linus Walleij, Ard Biesheuvel, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, May 14, 2024 at 02:32:23PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Tue, May 14, 2024 at 1:33 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > On Tue, May 14, 2024 at 01:22:36PM +0200, Geert Uytterhoeven wrote:
> > > On Tue, May 14, 2024 at 10:15 AM Russell King (Oracle)
> > > <linux@armlinux.org.uk> wrote:
> > > > On Mon, May 13, 2024 at 08:56:20PM -0700, Florian Fainelli wrote:
> > > > > [   11.299106] Freeing unused kernel image (initmem) memory: 79872K
> > > > > [   11.305720] Run /init as init process
> > > > > [   11.314070] Kernel panic - not syncing: Attempted to kill init!
> > > > > exitcode=0x00000004
> > > > > [   11.321888] CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-next-20240513 #32
> > > > > [   11.328709] Hardware name: BCM2711
> > > > > [   11.332169] Call trace:
> > > > > [   11.332179]  unwind_backtrace from show_stack+0x10/0x14
> > > > > [   11.340087]  show_stack from panic+0x20c/0x55c
> > > > > [   11.344615]  panic from do_exit+0x6b0/0x1e74
> > > > > [   11.348972]  do_exit from do_group_exit+0xcc/0x280
> > > > > [   11.353857]  do_group_exit from get_signal+0xfb4/0x1340
> > > > > [   11.359182]  get_signal from do_work_pending+0x2c0/0x7bc
> > > > > [   11.364590]  do_work_pending from slow_work_pending+0xc/0x24
> > > > > [   11.370351] Exception stack(0xf082bfb0 to 0xf082bff8)
> > > > > [   11.375492] bfa0:                                     b6bca568 00000000
> > > > > 003fa0d6 aedf3d20
> > > > > [   11.383811] bfc0: aedf4a28 b6bca6f8 b6bca73c b6bca710 b6bca748 b6bca6f8
> > > > > aedf4a28 b6bca6f8
> > > > > [   11.392127] bfe0: b6bca590 b6bca548 aeddae15 aedeb660 200f0030 ffffffff
> > > > > [   11.398954] ---[ end Kernel panic - not syncing: Attempted to kill init!
> > > > > exitcode=0x00000004 ]---
> > > >
> > > > You could enable CONFiG_DEBUG_USER, and then pass "user_debug=24" to
> > > > the kernel to get a report for the conditions that lead to SEGV/BUS
> > > > signals being delivered to a userspace processd.
> > >
> > > That does not seem to make any difference for me, i.e. no report?
> >
> > Then it's not a SEGV/BUS (iow page fault.) Please try user_debug=31
> > in that case. Thanks.
> 
> Thanks, much better:
> 
>     init (1): undefined instruction: pc=b6f4feda
>     CPU: 1 PID: 1 Comm: init Not tainted
> 6.9.0-shmobile-09158-g1218ffc3659e #1820
>     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
>     PC is at 0xb6f4feda
>     LR is at 0xb6f4ed31
>     pc : [<b6f4feda>]    lr : [<b6f4ed31>]    psr: 60000030
>     sp : be970630  ip : be970678  fp : b6f67978
>     r10: 00000000  r9 : 004d48ff  r8 : be970844
>     r7 : be9707f8  r6 : b6f67978  r5 : be970850  r4 : be970844
>     r3 : b6f669b0  r2 : 003fb0d6  r1 : 00000000  r0 : be970650
>     Flags: nZCv  IRQs on  FIQs on  Mode USER_32  ISA Thumb  Segment user
>     Control: 30c5387d  Table: 41f6cac0  DAC: 55555555
>     Code: bad PC value

Well, that points to another issue... get_user() appears to be unable
to access userspace. Userspace can, however, as we wouldn't get an
undefined instruction abort unless it can successfully access the
address.

This points to something being very wrong with this implementation.

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14  7:37           ` Linus Walleij
@ 2024-05-14 14:39             ` Catalin Marinas
  0 siblings, 0 replies; 49+ messages in thread
From: Catalin Marinas @ 2024-05-14 14:39 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Geert Uytterhoeven, Russell King, Ard Biesheuvel, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel

On Tue, May 14, 2024 at 09:37:21AM +0200, Linus Walleij wrote:
> On Mon, May 13, 2024 at 10:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > I guess I should bring out the STM32MP157 board again and retest
> > to verify that that one hardware works with this. Any other ideas?
> 
> I got this Cortex-A7 board out and booted with LPAE and PAN
> using TTBR0 and it boots fine (some kind of OpenEmbedded/YOCTO
> system is in it with systemd and all.
> 
> Tested with LKDTM provoked crashes and it behaves as expected.
> 
> Given Florians report it clearly works on some LPAE:s and not on some
> others for some reason! I suppose we need to figure it out.
> 
> Catalin do you have some ideas?

Since EPD0 is allowed to be cached in the TLB, we need to change the
ASID as well (assuming that it's at least tagged by ASID). But, to keep
the uaccess enable/disable code simple, patch 4 short-circuits this by
toggling the TTBCR.A1 bit. At the time (~2015) this was fine on the
32-bit CPUs. Since the A1 bit is theoretically allowed to be cached in
the TLB, toggling it may not have any effect without a TLB invalidation.
I recall some old documentation stating that the EPD0 value is only
cached when 0, not 1 (IOW, toggling it to 1 would not prevent existing
TLB entries from being hit). This wouldn't have been a significant
issue, most likely the PAN protection not always working but here it's
the other way around, it looks like a value of 1 persisting even after
being toggled.

I'd say the weird behaviour is caused by different microarchitectures,
especially if you run this on ARMv8 hardware (the original patch was
never intended to).

It would be worth trying to do a full TLBI invalidation after uaccess
enable/disable just to check this theory.

-- 
Catalin

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 12:38                     ` Russell King (Oracle)
@ 2024-05-14 15:03                       ` Catalin Marinas
  0 siblings, 0 replies; 49+ messages in thread
From: Catalin Marinas @ 2024-05-14 15:03 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Geert Uytterhoeven, Florian Fainelli, Linus Walleij,
	Ard Biesheuvel, Arnd Bergmann, Stefan Wahren, Kees Cook,
	linux-arm-kernel

On Tue, May 14, 2024 at 01:38:07PM +0100, Russell King wrote:
> On Tue, May 14, 2024 at 02:32:23PM +0200, Geert Uytterhoeven wrote:
> > On Tue, May 14, 2024 at 1:33 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > > On Tue, May 14, 2024 at 01:22:36PM +0200, Geert Uytterhoeven wrote:
> > > > On Tue, May 14, 2024 at 10:15 AM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:
> > > > > On Mon, May 13, 2024 at 08:56:20PM -0700, Florian Fainelli wrote:
> > > > > > [   11.299106] Freeing unused kernel image (initmem) memory: 79872K
> > > > > > [   11.305720] Run /init as init process
> > > > > > [   11.314070] Kernel panic - not syncing: Attempted to kill init!
> > > > > > exitcode=0x00000004
> > > > > > [   11.321888] CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-next-20240513 #32
> > > > > > [   11.328709] Hardware name: BCM2711
> > > > > > [   11.332169] Call trace:
> > > > > > [   11.332179]  unwind_backtrace from show_stack+0x10/0x14
> > > > > > [   11.340087]  show_stack from panic+0x20c/0x55c
> > > > > > [   11.344615]  panic from do_exit+0x6b0/0x1e74
> > > > > > [   11.348972]  do_exit from do_group_exit+0xcc/0x280
> > > > > > [   11.353857]  do_group_exit from get_signal+0xfb4/0x1340
> > > > > > [   11.359182]  get_signal from do_work_pending+0x2c0/0x7bc
> > > > > > [   11.364590]  do_work_pending from slow_work_pending+0xc/0x24
> > > > > > [   11.370351] Exception stack(0xf082bfb0 to 0xf082bff8)
> > > > > > [   11.375492] bfa0:                                     b6bca568 00000000
> > > > > > 003fa0d6 aedf3d20
> > > > > > [   11.383811] bfc0: aedf4a28 b6bca6f8 b6bca73c b6bca710 b6bca748 b6bca6f8
> > > > > > aedf4a28 b6bca6f8
> > > > > > [   11.392127] bfe0: b6bca590 b6bca548 aeddae15 aedeb660 200f0030 ffffffff
> > > > > > [   11.398954] ---[ end Kernel panic - not syncing: Attempted to kill init!
> > > > > > exitcode=0x00000004 ]---
> > > > >
> > > > > You could enable CONFiG_DEBUG_USER, and then pass "user_debug=24" to
> > > > > the kernel to get a report for the conditions that lead to SEGV/BUS
> > > > > signals being delivered to a userspace processd.
> > > >
> > > > That does not seem to make any difference for me, i.e. no report?
> > >
> > > Then it's not a SEGV/BUS (iow page fault.) Please try user_debug=31
> > > in that case. Thanks.
> > 
> > Thanks, much better:
> > 
> >     init (1): undefined instruction: pc=b6f4feda
> >     CPU: 1 PID: 1 Comm: init Not tainted
> > 6.9.0-shmobile-09158-g1218ffc3659e #1820
> >     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> >     PC is at 0xb6f4feda
> >     LR is at 0xb6f4ed31
> >     pc : [<b6f4feda>]    lr : [<b6f4ed31>]    psr: 60000030
> >     sp : be970630  ip : be970678  fp : b6f67978
> >     r10: 00000000  r9 : 004d48ff  r8 : be970844
> >     r7 : be9707f8  r6 : b6f67978  r5 : be970850  r4 : be970844
> >     r3 : b6f669b0  r2 : 003fb0d6  r1 : 00000000  r0 : be970650
> >     Flags: nZCv  IRQs on  FIQs on  Mode USER_32  ISA Thumb  Segment user
> >     Control: 30c5387d  Table: 41f6cac0  DAC: 55555555
> >     Code: bad PC value
> 
> Well, that points to another issue... get_user() appears to be unable
> to access userspace. Userspace can, however, as we wouldn't get an
> undefined instruction abort unless it can successfully access the
> address.
> 
> This points to something being very wrong with this implementation.

Yeah, it doesn't look great. Let's see if TLBIALLIS solves anything,
though not as an upstream solution as it's expensive, just to understand
the problem a bit better. So maybe revert the last patch from the
series, the first three seem inoffensive.

For the flush_tlb_all(), I think the mcr incantation is:

	mov	r0, #0
	mcr	p15, 0, r0, c8, c7, 0

Linus, if you attempt this in the uaccess_enable/disable macros, also
force the ISB to be always on just in case the TTBRC update does not
take place before the MCR.

-- 
Catalin

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 11:28                     ` Geert Uytterhoeven
@ 2024-05-14 16:06                       ` Geert Uytterhoeven
  2024-05-14 16:54                         ` Florian Fainelli
  0 siblings, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-14 16:06 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linus Walleij, Russell King, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, May 14, 2024 at 1:28 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, May 14, 2024 at 10:25 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Tue, 14 May 2024 at 10:04, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> > > > > On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > >
> > > > > > I sent you a small initramfs by PM.
> > > > >
> > > > > Booted this just fine on Vexpress QEMU:
> > > > >
> > > > > Run /init as init process
> > > > > sysctl: error: 'kernel.hotplug' is an unknown key
> > > > >
> > > > >
> > > > > boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
> > > > > / # mount -t debugfs none /sys/kernel/debug
> > > > > / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> > > > > lkdtm: Performing direct entry ACCESS_USERSPACE
> > > > > lkdtm: attempting bad read at 76fea000
> > > > > 8<--- cut here ---
> > > > > Unable to handle kernel paging request at virtual address 76fea000 when read
> > > > > [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
> > > > > Internal error: Oops: 206 [#1] SMP ARM
> > > > > CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
> > > > > Hardware name: ARM-Versatile Express
> > > > > PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > > > LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > > >
> > > > > I'm starting to think it is something about different LPAE implementations here.
> > > >
> > > > I have built multi_v7_defconfig with the following enabled
> > > >
> > > > CONFIG_ARM_LPAE=y
> > > > CONFIG_CPU_TTBR0_PAN=y
> > > > CONFIG_LKDTM=y
> > > >
> > > > and the resulting kernel boots happily as a 32-bit VM running under a
> > > > Rpi4 KVM host.
> > > >
> > > > Could someone post an actual .config that reproduces this? Rpi4 is
> > > > A72, which both works and doesn't work in Florian's testing, so I'd be
> > > > highly surprised if this is not a config issue.
> > >
> > > shmobile_defconfig with CONFIG_LPAE=y added failed for me before.
> > >
> > > Building multi_v7_defconfig with the above enabled...
>
> And that works, while shmobile_defconfig with the above does not!
> So it is config-related.

I ran tools/testing/ktest/config-bisect.pl and found the "offending"
config option: CONFIG_CC_OPTIMIZE_FOR_SIZE=y gives a broken
kernel, CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y works.
(verified by just enabling CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE in my
 broken config)....

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 16:06                       ` Geert Uytterhoeven
@ 2024-05-14 16:54                         ` Florian Fainelli
  2024-05-14 17:03                           ` Russell King (Oracle)
  0 siblings, 1 reply; 49+ messages in thread
From: Florian Fainelli @ 2024-05-14 16:54 UTC (permalink / raw)
  To: Geert Uytterhoeven, Ard Biesheuvel
  Cc: Linus Walleij, Russell King, Arnd Bergmann, Stefan Wahren,
	Kees Cook, linux-arm-kernel, Catalin Marinas

On 5/14/24 09:06, Geert Uytterhoeven wrote:
> On Tue, May 14, 2024 at 1:28 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Tue, May 14, 2024 at 10:25 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>> On Tue, 14 May 2024 at 10:04, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>> On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>>>> On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>>>
>>>>>>> I sent you a small initramfs by PM.
>>>>>>
>>>>>> Booted this just fine on Vexpress QEMU:
>>>>>>
>>>>>> Run /init as init process
>>>>>> sysctl: error: 'kernel.hotplug' is an unknown key
>>>>>>
>>>>>>
>>>>>> boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
>>>>>> / # mount -t debugfs none /sys/kernel/debug
>>>>>> / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
>>>>>> lkdtm: Performing direct entry ACCESS_USERSPACE
>>>>>> lkdtm: attempting bad read at 76fea000
>>>>>> 8<--- cut here ---
>>>>>> Unable to handle kernel paging request at virtual address 76fea000 when read
>>>>>> [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
>>>>>> Internal error: Oops: 206 [#1] SMP ARM
>>>>>> CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
>>>>>> Hardware name: ARM-Versatile Express
>>>>>> PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
>>>>>> LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
>>>>>>
>>>>>> I'm starting to think it is something about different LPAE implementations here.
>>>>>
>>>>> I have built multi_v7_defconfig with the following enabled
>>>>>
>>>>> CONFIG_ARM_LPAE=y
>>>>> CONFIG_CPU_TTBR0_PAN=y
>>>>> CONFIG_LKDTM=y
>>>>>
>>>>> and the resulting kernel boots happily as a 32-bit VM running under a
>>>>> Rpi4 KVM host.
>>>>>
>>>>> Could someone post an actual .config that reproduces this? Rpi4 is
>>>>> A72, which both works and doesn't work in Florian's testing, so I'd be
>>>>> highly surprised if this is not a config issue.
>>>>
>>>> shmobile_defconfig with CONFIG_LPAE=y added failed for me before.
>>>>
>>>> Building multi_v7_defconfig with the above enabled...
>>
>> And that works, while shmobile_defconfig with the above does not!
>> So it is config-related.
> 
> I ran tools/testing/ktest/config-bisect.pl and found the "offending"
> config option: CONFIG_CC_OPTIMIZE_FOR_SIZE=y gives a broken
> kernel, CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y works.
> (verified by just enabling CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE in my
>   broken config)....

I second that, my working configuration has 
CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y whereas my
broken one has CONFIG_CC_OPTIMIZE_FOR_SIZE=y, sure enough, changing the 
broken configuration to have CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y turns 
it into a "working" configuration on the Raspberry Pi 4B in AArch32 mode.

That makes more sense because BCM7211 and BCM2711 are exactly the same 
CPU, thanks for restoring sanity!
-- 
Florian


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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 16:54                         ` Florian Fainelli
@ 2024-05-14 17:03                           ` Russell King (Oracle)
  2024-05-14 18:26                             ` Florian Fainelli
  0 siblings, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-14 17:03 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Geert Uytterhoeven, Ard Biesheuvel, Linus Walleij, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

On Tue, May 14, 2024 at 09:54:07AM -0700, Florian Fainelli wrote:
> On 5/14/24 09:06, Geert Uytterhoeven wrote:
> > On Tue, May 14, 2024 at 1:28 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, May 14, 2024 at 10:25 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > On Tue, 14 May 2024 at 10:04, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> > > > > > > On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > 
> > > > > > > > I sent you a small initramfs by PM.
> > > > > > > 
> > > > > > > Booted this just fine on Vexpress QEMU:
> > > > > > > 
> > > > > > > Run /init as init process
> > > > > > > sysctl: error: 'kernel.hotplug' is an unknown key
> > > > > > > 
> > > > > > > 
> > > > > > > boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
> > > > > > > / # mount -t debugfs none /sys/kernel/debug
> > > > > > > / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
> > > > > > > lkdtm: Performing direct entry ACCESS_USERSPACE
> > > > > > > lkdtm: attempting bad read at 76fea000
> > > > > > > 8<--- cut here ---
> > > > > > > Unable to handle kernel paging request at virtual address 76fea000 when read
> > > > > > > [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
> > > > > > > Internal error: Oops: 206 [#1] SMP ARM
> > > > > > > CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
> > > > > > > Hardware name: ARM-Versatile Express
> > > > > > > PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > > > > > LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
> > > > > > > 
> > > > > > > I'm starting to think it is something about different LPAE implementations here.
> > > > > > 
> > > > > > I have built multi_v7_defconfig with the following enabled
> > > > > > 
> > > > > > CONFIG_ARM_LPAE=y
> > > > > > CONFIG_CPU_TTBR0_PAN=y
> > > > > > CONFIG_LKDTM=y
> > > > > > 
> > > > > > and the resulting kernel boots happily as a 32-bit VM running under a
> > > > > > Rpi4 KVM host.
> > > > > > 
> > > > > > Could someone post an actual .config that reproduces this? Rpi4 is
> > > > > > A72, which both works and doesn't work in Florian's testing, so I'd be
> > > > > > highly surprised if this is not a config issue.
> > > > > 
> > > > > shmobile_defconfig with CONFIG_LPAE=y added failed for me before.
> > > > > 
> > > > > Building multi_v7_defconfig with the above enabled...
> > > 
> > > And that works, while shmobile_defconfig with the above does not!
> > > So it is config-related.
> > 
> > I ran tools/testing/ktest/config-bisect.pl and found the "offending"
> > config option: CONFIG_CC_OPTIMIZE_FOR_SIZE=y gives a broken
> > kernel, CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y works.
> > (verified by just enabling CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE in my
> >   broken config)....
> 
> I second that, my working configuration has
> CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y whereas my
> broken one has CONFIG_CC_OPTIMIZE_FOR_SIZE=y, sure enough, changing the
> broken configuration to have CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y turns it
> into a "working" configuration on the Raspberry Pi 4B in AArch32 mode.
> 
> That makes more sense because BCM7211 and BCM2711 are exactly the same CPU,
> thanks for restoring sanity!

I would imagine that the problem is cpu_set_ttbcr(). Please try adding
a "memory" clobber to the asm() instruction in there.

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 17:03                           ` Russell King (Oracle)
@ 2024-05-14 18:26                             ` Florian Fainelli
  2024-05-14 20:33                               ` Linus Walleij
  2024-05-15  8:10                               ` Geert Uytterhoeven
  0 siblings, 2 replies; 49+ messages in thread
From: Florian Fainelli @ 2024-05-14 18:26 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Geert Uytterhoeven, Ard Biesheuvel, Linus Walleij, Arnd Bergmann,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

On 5/14/24 10:03, Russell King (Oracle) wrote:
> On Tue, May 14, 2024 at 09:54:07AM -0700, Florian Fainelli wrote:
>> On 5/14/24 09:06, Geert Uytterhoeven wrote:
>>> On Tue, May 14, 2024 at 1:28 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> On Tue, May 14, 2024 at 10:25 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>> On Tue, 14 May 2024 at 10:04, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>>> On Tue, May 14, 2024 at 9:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>>>> On Tue, 14 May 2024 at 09:46, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>>>>>> On Tue, May 14, 2024 at 8:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>>>>>
>>>>>>>>> I sent you a small initramfs by PM.
>>>>>>>>
>>>>>>>> Booted this just fine on Vexpress QEMU:
>>>>>>>>
>>>>>>>> Run /init as init process
>>>>>>>> sysctl: error: 'kernel.hotplug' is an unknown key
>>>>>>>>
>>>>>>>>
>>>>>>>> boot (Linux 6.9.0-rc1+, BusyBox v1.16.0.git, kexec-tools 2.0.1-git)
>>>>>>>> / # mount -t debugfs none /sys/kernel/debug
>>>>>>>> / # echo "ACCESS_USERSPACE" | cat >/sys/kernel/debug/provoke-crash/DIRECT
>>>>>>>> lkdtm: Performing direct entry ACCESS_USERSPACE
>>>>>>>> lkdtm: attempting bad read at 76fea000
>>>>>>>> 8<--- cut here ---
>>>>>>>> Unable to handle kernel paging request at virtual address 76fea000 when read
>>>>>>>> [76fea000] *pgd=82c93003, *pmd=82c94003, *pte=a00000811e2f5f
>>>>>>>> Internal error: Oops: 206 [#1] SMP ARM
>>>>>>>> CPU: 1 PID: 86 Comm: cat Not tainted 6.9.0-rc1+ #46
>>>>>>>> Hardware name: ARM-Versatile Express
>>>>>>>> PC is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
>>>>>>>> LR is at lkdtm_ACCESS_USERSPACE+0xc0/0x138
>>>>>>>>
>>>>>>>> I'm starting to think it is something about different LPAE implementations here.
>>>>>>>
>>>>>>> I have built multi_v7_defconfig with the following enabled
>>>>>>>
>>>>>>> CONFIG_ARM_LPAE=y
>>>>>>> CONFIG_CPU_TTBR0_PAN=y
>>>>>>> CONFIG_LKDTM=y
>>>>>>>
>>>>>>> and the resulting kernel boots happily as a 32-bit VM running under a
>>>>>>> Rpi4 KVM host.
>>>>>>>
>>>>>>> Could someone post an actual .config that reproduces this? Rpi4 is
>>>>>>> A72, which both works and doesn't work in Florian's testing, so I'd be
>>>>>>> highly surprised if this is not a config issue.
>>>>>>
>>>>>> shmobile_defconfig with CONFIG_LPAE=y added failed for me before.
>>>>>>
>>>>>> Building multi_v7_defconfig with the above enabled...
>>>>
>>>> And that works, while shmobile_defconfig with the above does not!
>>>> So it is config-related.
>>>
>>> I ran tools/testing/ktest/config-bisect.pl and found the "offending"
>>> config option: CONFIG_CC_OPTIMIZE_FOR_SIZE=y gives a broken
>>> kernel, CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y works.
>>> (verified by just enabling CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE in my
>>>    broken config)....
>>
>> I second that, my working configuration has
>> CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y whereas my
>> broken one has CONFIG_CC_OPTIMIZE_FOR_SIZE=y, sure enough, changing the
>> broken configuration to have CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y turns it
>> into a "working" configuration on the Raspberry Pi 4B in AArch32 mode.
>>
>> That makes more sense because BCM7211 and BCM2711 are exactly the same CPU,
>> thanks for restoring sanity!
> 
> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> a "memory" clobber to the asm() instruction in there.
> 

I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:

diff --git a/arch/arm/include/asm/proc-fns.h 
b/arch/arm/include/asm/proc-fns.h
index 9b3105a2a5e0..1087bd2af433 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)

  static inline void cpu_set_ttbcr(unsigned int ttbcr)
  {
-       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
+       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
  }

  #else  /*!CONFIG_MMU */

my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.

Thanks a lot Russell!
-- 
Florian


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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 18:26                             ` Florian Fainelli
@ 2024-05-14 20:33                               ` Linus Walleij
  2024-05-14 20:34                                 ` Florian Fainelli
  2024-05-15  8:10                               ` Geert Uytterhoeven
  1 sibling, 1 reply; 49+ messages in thread
From: Linus Walleij @ 2024-05-14 20:33 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Russell King (Oracle), Geert Uytterhoeven, Ard Biesheuvel,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 5/14/24 10:03, Russell King (Oracle) wrote:

> > I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > a "memory" clobber to the asm() instruction in there.
> >
>
> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
>
> diff --git a/arch/arm/include/asm/proc-fns.h
> b/arch/arm/include/asm/proc-fns.h
> index 9b3105a2a5e0..1087bd2af433 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
>
>   static inline void cpu_set_ttbcr(unsigned int ttbcr)
>   {
> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
>   }
>
>   #else  /*!CONFIG_MMU */
>
> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
>
> Thanks a lot Russell!

Second that, very nicely pinpointed Russell!

Florian, do you want to send a patch or should I?

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 20:33                               ` Linus Walleij
@ 2024-05-14 20:34                                 ` Florian Fainelli
  2024-05-15  8:36                                   ` Ard Biesheuvel
  0 siblings, 1 reply; 49+ messages in thread
From: Florian Fainelli @ 2024-05-14 20:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Russell King (Oracle), Geert Uytterhoeven, Ard Biesheuvel,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On 5/14/24 13:33, Linus Walleij wrote:
> On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 5/14/24 10:03, Russell King (Oracle) wrote:
> 
>>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
>>> a "memory" clobber to the asm() instruction in there.
>>>
>>
>> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
>>
>> diff --git a/arch/arm/include/asm/proc-fns.h
>> b/arch/arm/include/asm/proc-fns.h
>> index 9b3105a2a5e0..1087bd2af433 100644
>> --- a/arch/arm/include/asm/proc-fns.h
>> +++ b/arch/arm/include/asm/proc-fns.h
>> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
>>
>>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
>>    {
>> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
>> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
>>    }
>>
>>    #else  /*!CONFIG_MMU */
>>
>> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
>>
>> Thanks a lot Russell!
> 
> Second that, very nicely pinpointed Russell!
> 
> Florian, do you want to send a patch or should I?

I was wondering if Russell was able to fold this directly into patch #2 
where cpu_set_ttbr() is added, so as to not break functionality across 
bisection.
-- 
Florian


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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 18:26                             ` Florian Fainelli
  2024-05-14 20:33                               ` Linus Walleij
@ 2024-05-15  8:10                               ` Geert Uytterhoeven
  1 sibling, 0 replies; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-15  8:10 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Russell King (Oracle), Ard Biesheuvel, Linus Walleij,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > On Tue, May 14, 2024 at 09:54:07AM -0700, Florian Fainelli wrote:
> >> On 5/14/24 09:06, Geert Uytterhoeven wrote:
> >>> On Tue, May 14, 2024 at 1:28 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >>> I ran tools/testing/ktest/config-bisect.pl and found the "offending"
> >>> config option: CONFIG_CC_OPTIMIZE_FOR_SIZE=y gives a broken
> >>> kernel, CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y works.
> >>> (verified by just enabling CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE in my
> >>>    broken config)....
> >>
> >> I second that, my working configuration has
> >> CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y whereas my
> >> broken one has CONFIG_CC_OPTIMIZE_FOR_SIZE=y, sure enough, changing the
> >> broken configuration to have CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y turns it
> >> into a "working" configuration on the Raspberry Pi 4B in AArch32 mode.
> >>
> >> That makes more sense because BCM7211 and BCM2711 are exactly the same CPU,
> >> thanks for restoring sanity!
> >
> > I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > a "memory" clobber to the asm() instruction in there.
> >
>
> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
>
> diff --git a/arch/arm/include/asm/proc-fns.h
> b/arch/arm/include/asm/proc-fns.h
> index 9b3105a2a5e0..1087bd2af433 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
>
>   static inline void cpu_set_ttbcr(unsigned int ttbcr)
>   {
> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
>   }
>
>   #else  /*!CONFIG_MMU */
>
> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
>
> Thanks a lot Russell!

Unfortunately this does not fix the issue for me.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-14 20:34                                 ` Florian Fainelli
@ 2024-05-15  8:36                                   ` Ard Biesheuvel
  2024-05-15  8:45                                     ` Geert Uytterhoeven
  2024-05-15  8:48                                     ` Russell King (Oracle)
  0 siblings, 2 replies; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-15  8:36 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Linus Walleij, Russell King (Oracle), Geert Uytterhoeven,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 5/14/24 13:33, Linus Walleij wrote:
> > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> >
> >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> >>> a "memory" clobber to the asm() instruction in there.
> >>>
> >>
> >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> >>
> >> diff --git a/arch/arm/include/asm/proc-fns.h
> >> b/arch/arm/include/asm/proc-fns.h
> >> index 9b3105a2a5e0..1087bd2af433 100644
> >> --- a/arch/arm/include/asm/proc-fns.h
> >> +++ b/arch/arm/include/asm/proc-fns.h
> >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> >>
> >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> >>    {
> >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> >>    }
> >>
> >>    #else  /*!CONFIG_MMU */
> >>
> >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> >>
> >> Thanks a lot Russell!
> >
> > Second that, very nicely pinpointed Russell!
> >
> > Florian, do you want to send a patch or should I?
>
> I was wondering if Russell was able to fold this directly into patch #2
> where cpu_set_ttbr() is added, so as to not break functionality across
> bisection.

Sadly, I can still reproduce this with the above fix.

I included TTBCR in the DEBUG_USER output, and (as expected), it has
A1, EPD0 and T0SZ set to the 'uaccess disabled' values.

Using __always_inline on uaccess_save_and_enable() and
uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
around it. The "memory" clobber seems unnecessary in my case, but it
is needed for correctness in any case.

It is unclear to me why placing these helpers out of line should make
any difference, and I am not convinced it is a problem in the code
(IIRC we've had other issues with -Os in the past)






Run /init as init process
init (1): undefined instruction: pc=0015f450 ttbcr=b5403587
CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-rc1-00037-gb7e329ac0464-dirty #8
Hardware name: Generic DT based system
PC is at 0x15f450
LR is at 0x1548b0
pc : [<0015f450>]    lr : [<001548b0>]    psr: 20000010
sp : bec97da0  ip : bec97ee8  fp : 00000000
r10: 00000000  r9 : 00000000  r8 : 00000000
r7 : 00000000  r6 : bec97f0c  r5 : 00000000  r4 : 00000040
r3 : 00000000  r2 : 00000040  r1 : 00000040  r0 : 00000000
Flags: nzCv  IRQs on  FIQs on  Mode USER_32  ISA ARM  Segment user
Control: 30c5383d  Table: 427122c0  DAC: fffffffd
Code: bad PC value
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
CPU: 0 PID: 1 Comm: init Not tainted 6.9.0-rc1-00037-gb7e329ac0464-dirty #8
Hardware name: Generic DT based system
Call trace:
 unwind_backtrace from show_stack+0x10/0x14
 show_stack from dump_stack_lvl+0x54/0x68
 dump_stack_lvl from panic+0xf8/0x34c
 panic from do_exit+0x1dc/0x880
 do_exit from sys_exit_group+0x0/0x10

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  8:36                                   ` Ard Biesheuvel
@ 2024-05-15  8:45                                     ` Geert Uytterhoeven
  2024-05-15  8:49                                       ` Ard Biesheuvel
  2024-05-15  8:48                                     ` Russell King (Oracle)
  1 sibling, 1 reply; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-15  8:45 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Florian Fainelli, Linus Walleij, Russell King (Oracle),
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

Hi Ard,

On Wed, May 15, 2024 at 10:37 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > On 5/14/24 13:33, Linus Walleij wrote:
> > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > >
> > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > >>> a "memory" clobber to the asm() instruction in there.
> > >>>
> > >>
> > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > >>
> > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > >> b/arch/arm/include/asm/proc-fns.h
> > >> index 9b3105a2a5e0..1087bd2af433 100644
> > >> --- a/arch/arm/include/asm/proc-fns.h
> > >> +++ b/arch/arm/include/asm/proc-fns.h
> > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > >>
> > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > >>    {
> > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > >>    }
> > >>
> > >>    #else  /*!CONFIG_MMU */
> > >>
> > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > >>
> > >> Thanks a lot Russell!
> > >
> > > Second that, very nicely pinpointed Russell!
> > >
> > > Florian, do you want to send a patch or should I?
> >
> > I was wondering if Russell was able to fold this directly into patch #2
> > where cpu_set_ttbr() is added, so as to not break functionality across
> > bisection.
>
> Sadly, I can still reproduce this with the above fix.
>
> I included TTBCR in the DEBUG_USER output, and (as expected), it has
> A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
>
> Using __always_inline on uaccess_save_and_enable() and
> uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> around it. The "memory" clobber seems unnecessary in my case, but it
> is needed for correctness in any case.
>
> It is unclear to me why placing these helpers out of line should make
> any difference, and I am not convinced it is a problem in the code
> (IIRC we've had other issues with -Os in the past)

Commit 66abdd3b5d4e53bc ("ARM: 9356/2: Move asm statements accessing
TTBCR into C functions") also removed the "volatile" from the mcr
inline asm statement.  I tried adding it back, but that didn't help.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  8:36                                   ` Ard Biesheuvel
  2024-05-15  8:45                                     ` Geert Uytterhoeven
@ 2024-05-15  8:48                                     ` Russell King (Oracle)
  2024-05-15  8:53                                       ` Ard Biesheuvel
  1 sibling, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-15  8:48 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Florian Fainelli, Linus Walleij, Geert Uytterhoeven,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, May 15, 2024 at 10:36:48AM +0200, Ard Biesheuvel wrote:
> On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> >
> > On 5/14/24 13:33, Linus Walleij wrote:
> > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > >
> > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > >>> a "memory" clobber to the asm() instruction in there.
> > >>>
> > >>
> > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > >>
> > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > >> b/arch/arm/include/asm/proc-fns.h
> > >> index 9b3105a2a5e0..1087bd2af433 100644
> > >> --- a/arch/arm/include/asm/proc-fns.h
> > >> +++ b/arch/arm/include/asm/proc-fns.h
> > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > >>
> > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > >>    {
> > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > >>    }
> > >>
> > >>    #else  /*!CONFIG_MMU */
> > >>
> > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > >>
> > >> Thanks a lot Russell!
> > >
> > > Second that, very nicely pinpointed Russell!
> > >
> > > Florian, do you want to send a patch or should I?
> >
> > I was wondering if Russell was able to fold this directly into patch #2
> > where cpu_set_ttbr() is added, so as to not break functionality across
> > bisection.
> 
> Sadly, I can still reproduce this with the above fix.
> 
> I included TTBCR in the DEBUG_USER output, and (as expected), it has
> A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
> 
> Using __always_inline on uaccess_save_and_enable() and
> uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> around it. The "memory" clobber seems unnecessary in my case, but it
> is needed for correctness in any case.
> 
> It is unclear to me why placing these helpers out of line should make
> any difference, and I am not convinced it is a problem in the code
> (IIRC we've had other issues with -Os in the past)

Time to start comparing compilers / compiler versions?

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  8:45                                     ` Geert Uytterhoeven
@ 2024-05-15  8:49                                       ` Ard Biesheuvel
  2024-05-15  9:21                                         ` Geert Uytterhoeven
  0 siblings, 1 reply; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-15  8:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Fainelli, Linus Walleij, Russell King (Oracle),
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, 15 May 2024 at 10:45, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ard,
>
> On Wed, May 15, 2024 at 10:37 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > On 5/14/24 13:33, Linus Walleij wrote:
> > > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > > >
> > > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > > >>> a "memory" clobber to the asm() instruction in there.
> > > >>>
> > > >>
> > > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > > >>
> > > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > > >> b/arch/arm/include/asm/proc-fns.h
> > > >> index 9b3105a2a5e0..1087bd2af433 100644
> > > >> --- a/arch/arm/include/asm/proc-fns.h
> > > >> +++ b/arch/arm/include/asm/proc-fns.h
> > > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > > >>
> > > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > > >>    {
> > > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > > >>    }
> > > >>
> > > >>    #else  /*!CONFIG_MMU */
> > > >>
> > > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > > >>
> > > >> Thanks a lot Russell!
> > > >
> > > > Second that, very nicely pinpointed Russell!
> > > >
> > > > Florian, do you want to send a patch or should I?
> > >
> > > I was wondering if Russell was able to fold this directly into patch #2
> > > where cpu_set_ttbr() is added, so as to not break functionality across
> > > bisection.
> >
> > Sadly, I can still reproduce this with the above fix.
> >
> > I included TTBCR in the DEBUG_USER output, and (as expected), it has
> > A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
> >
> > Using __always_inline on uaccess_save_and_enable() and
> > uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> > around it. The "memory" clobber seems unnecessary in my case, but it
> > is needed for correctness in any case.
> >
> > It is unclear to me why placing these helpers out of line should make
> > any difference, and I am not convinced it is a problem in the code
> > (IIRC we've had other issues with -Os in the past)
>
> Commit 66abdd3b5d4e53bc ("ARM: 9356/2: Move asm statements accessing
> TTBCR into C functions") also removed the "volatile" from the mcr
> inline asm statement.  I tried adding it back, but that didn't help.
>

Does using __always_inline make any difference in your case?

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  8:48                                     ` Russell King (Oracle)
@ 2024-05-15  8:53                                       ` Ard Biesheuvel
  2024-05-15 12:27                                         ` Russell King (Oracle)
  0 siblings, 1 reply; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-15  8:53 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Florian Fainelli, Linus Walleij, Geert Uytterhoeven,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, 15 May 2024 at 10:48, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Wed, May 15, 2024 at 10:36:48AM +0200, Ard Biesheuvel wrote:
> > On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > >
> > > On 5/14/24 13:33, Linus Walleij wrote:
> > > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > > >
> > > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > > >>> a "memory" clobber to the asm() instruction in there.
> > > >>>
> > > >>
> > > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > > >>
> > > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > > >> b/arch/arm/include/asm/proc-fns.h
> > > >> index 9b3105a2a5e0..1087bd2af433 100644
> > > >> --- a/arch/arm/include/asm/proc-fns.h
> > > >> +++ b/arch/arm/include/asm/proc-fns.h
> > > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > > >>
> > > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > > >>    {
> > > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > > >>    }
> > > >>
> > > >>    #else  /*!CONFIG_MMU */
> > > >>
> > > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > > >>
> > > >> Thanks a lot Russell!
> > > >
> > > > Second that, very nicely pinpointed Russell!
> > > >
> > > > Florian, do you want to send a patch or should I?
> > >
> > > I was wondering if Russell was able to fold this directly into patch #2
> > > where cpu_set_ttbr() is added, so as to not break functionality across
> > > bisection.
> >
> > Sadly, I can still reproduce this with the above fix.
> >
> > I included TTBCR in the DEBUG_USER output, and (as expected), it has
> > A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
> >
> > Using __always_inline on uaccess_save_and_enable() and
> > uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> > around it. The "memory" clobber seems unnecessary in my case, but it
> > is needed for correctness in any case.
> >
> > It is unclear to me why placing these helpers out of line should make
> > any difference, and I am not convinced it is a problem in the code
> > (IIRC we've had other issues with -Os in the past)
>
> Time to start comparing compilers / compiler versions?
>

I guess.

I'll try to dig a bit deeper this afternoon - AIUI, there is never a
valid reason to return to user space with uaccess disabled, right? So
we should be able to catch that case using a BUG() or similar, and
crash in the kernel rather than in user space when this occurs. That
should make it a bit easier to deal with in the debugger etc.

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  8:49                                       ` Ard Biesheuvel
@ 2024-05-15  9:21                                         ` Geert Uytterhoeven
  2024-05-15  9:39                                           ` Ard Biesheuvel
  2024-05-15 11:58                                           ` Linus Walleij
  0 siblings, 2 replies; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-15  9:21 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Florian Fainelli, Linus Walleij, Russell King (Oracle),
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

Hi Ard,

On Wed, May 15, 2024 at 10:49 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> On Wed, 15 May 2024 at 10:45, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, May 15, 2024 at 10:37 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > On 5/14/24 13:33, Linus Walleij wrote:
> > > > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > > > >
> > > > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > > > >>> a "memory" clobber to the asm() instruction in there.
> > > > >>>
> > > > >>
> > > > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > > > >>
> > > > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > > > >> b/arch/arm/include/asm/proc-fns.h
> > > > >> index 9b3105a2a5e0..1087bd2af433 100644
> > > > >> --- a/arch/arm/include/asm/proc-fns.h
> > > > >> +++ b/arch/arm/include/asm/proc-fns.h
> > > > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > > > >>
> > > > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > > > >>    {
> > > > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > > > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > > > >>    }
> > > > >>
> > > > >>    #else  /*!CONFIG_MMU */
> > > > >>
> > > > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > > > >>
> > > > >> Thanks a lot Russell!
> > > > >
> > > > > Second that, very nicely pinpointed Russell!
> > > > >
> > > > > Florian, do you want to send a patch or should I?
> > > >
> > > > I was wondering if Russell was able to fold this directly into patch #2
> > > > where cpu_set_ttbr() is added, so as to not break functionality across
> > > > bisection.
> > >
> > > Sadly, I can still reproduce this with the above fix.
> > >
> > > I included TTBCR in the DEBUG_USER output, and (as expected), it has
> > > A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
> > >
> > > Using __always_inline on uaccess_save_and_enable() and
> > > uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> > > around it. The "memory" clobber seems unnecessary in my case, but it
> > > is needed for correctness in any case.
> > >
> > > It is unclear to me why placing these helpers out of line should make
> > > any difference, and I am not convinced it is a problem in the code
> > > (IIRC we've had other issues with -Os in the past)
> >
> > Commit 66abdd3b5d4e53bc ("ARM: 9356/2: Move asm statements accessing
> > TTBCR into C functions") also removed the "volatile" from the mcr
> > inline asm statement.  I tried adding it back, but that didn't help.
>
> Does using __always_inline make any difference in your case?

Marking  uaccess_save_and_enable() __always_inline is sufficient
to fix the issue.

gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  9:21                                         ` Geert Uytterhoeven
@ 2024-05-15  9:39                                           ` Ard Biesheuvel
  2024-05-15 11:58                                           ` Linus Walleij
  1 sibling, 0 replies; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-15  9:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Florian Fainelli, Linus Walleij, Russell King (Oracle),
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, 15 May 2024 at 11:21, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ard,
>
> On Wed, May 15, 2024 at 10:49 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Wed, 15 May 2024 at 10:45, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Wed, May 15, 2024 at 10:37 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > > On 5/14/24 13:33, Linus Walleij wrote:
> > > > > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > > > > >
> > > > > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > > > > >>> a "memory" clobber to the asm() instruction in there.
> > > > > >>>
> > > > > >>
> > > > > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > > > > >>
> > > > > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > > > > >> b/arch/arm/include/asm/proc-fns.h
> > > > > >> index 9b3105a2a5e0..1087bd2af433 100644
> > > > > >> --- a/arch/arm/include/asm/proc-fns.h
> > > > > >> +++ b/arch/arm/include/asm/proc-fns.h
> > > > > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > > > > >>
> > > > > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > > > > >>    {
> > > > > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > > > > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > > > > >>    }
> > > > > >>
> > > > > >>    #else  /*!CONFIG_MMU */
> > > > > >>
> > > > > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > > > > >>
> > > > > >> Thanks a lot Russell!
> > > > > >
> > > > > > Second that, very nicely pinpointed Russell!
> > > > > >
> > > > > > Florian, do you want to send a patch or should I?
> > > > >
> > > > > I was wondering if Russell was able to fold this directly into patch #2
> > > > > where cpu_set_ttbr() is added, so as to not break functionality across
> > > > > bisection.
> > > >
> > > > Sadly, I can still reproduce this with the above fix.
> > > >
> > > > I included TTBCR in the DEBUG_USER output, and (as expected), it has
> > > > A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
> > > >
> > > > Using __always_inline on uaccess_save_and_enable() and
> > > > uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> > > > around it. The "memory" clobber seems unnecessary in my case, but it
> > > > is needed for correctness in any case.
> > > >
> > > > It is unclear to me why placing these helpers out of line should make
> > > > any difference, and I am not convinced it is a problem in the code
> > > > (IIRC we've had other issues with -Os in the past)
> > >
> > > Commit 66abdd3b5d4e53bc ("ARM: 9356/2: Move asm statements accessing
> > > TTBCR into C functions") also removed the "volatile" from the mcr
> > > inline asm statement.  I tried adding it back, but that didn't help.
> >
> > Does using __always_inline make any difference in your case?
>
> Marking  uaccess_save_and_enable() __always_inline is sufficient
> to fix the issue.
>
> gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)
>

Thanks for confirming.

For the record, I am using

gcc version 13.2.0 (Debian 13.2.0-12)

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  9:21                                         ` Geert Uytterhoeven
  2024-05-15  9:39                                           ` Ard Biesheuvel
@ 2024-05-15 11:58                                           ` Linus Walleij
  2024-05-15 14:05                                             ` Geert Uytterhoeven
  1 sibling, 1 reply; 49+ messages in thread
From: Linus Walleij @ 2024-05-15 11:58 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Ard Biesheuvel, Florian Fainelli, Russell King (Oracle),
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, May 15, 2024 at 11:21 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Ard:

> > Does using __always_inline make any difference in your case?
>
> Marking  uaccess_save_and_enable() __always_inline is sufficient
> to fix the issue.
>
> gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)

Can you make a combined patch with __always_inline and the
"memory" clobber both and send so Russell has something he
can fold in to stabilize his tree? Like a "this makes my machine
work" patch.

I'm using
$ arm-none-eabi-gcc --version
arm-none-eabi-gcc (Arm GNU Toolchain 12.2.MPACBTI-Bet1 (Build
arm-12-mpacbti.16)) 12.2.0

I'm almost always using LLVM in parallel as well, also for this work,
because I work on CFI and forget to switch compiler between different
projects all of the time.

Yours,
Linus Walleij

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15  8:53                                       ` Ard Biesheuvel
@ 2024-05-15 12:27                                         ` Russell King (Oracle)
  2024-05-15 15:41                                           ` Ard Biesheuvel
  0 siblings, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-15 12:27 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Florian Fainelli, Linus Walleij, Geert Uytterhoeven,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, May 15, 2024 at 10:53:47AM +0200, Ard Biesheuvel wrote:
> On Wed, 15 May 2024 at 10:48, Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, May 15, 2024 at 10:36:48AM +0200, Ard Biesheuvel wrote:
> > > On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > >
> > > > On 5/14/24 13:33, Linus Walleij wrote:
> > > > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > > > >
> > > > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > > > >>> a "memory" clobber to the asm() instruction in there.
> > > > >>>
> > > > >>
> > > > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > > > >>
> > > > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > > > >> b/arch/arm/include/asm/proc-fns.h
> > > > >> index 9b3105a2a5e0..1087bd2af433 100644
> > > > >> --- a/arch/arm/include/asm/proc-fns.h
> > > > >> +++ b/arch/arm/include/asm/proc-fns.h
> > > > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > > > >>
> > > > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > > > >>    {
> > > > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > > > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > > > >>    }
> > > > >>
> > > > >>    #else  /*!CONFIG_MMU */
> > > > >>
> > > > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > > > >>
> > > > >> Thanks a lot Russell!
> > > > >
> > > > > Second that, very nicely pinpointed Russell!
> > > > >
> > > > > Florian, do you want to send a patch or should I?
> > > >
> > > > I was wondering if Russell was able to fold this directly into patch #2
> > > > where cpu_set_ttbr() is added, so as to not break functionality across
> > > > bisection.
> > >
> > > Sadly, I can still reproduce this with the above fix.
> > >
> > > I included TTBCR in the DEBUG_USER output, and (as expected), it has
> > > A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
> > >
> > > Using __always_inline on uaccess_save_and_enable() and
> > > uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> > > around it. The "memory" clobber seems unnecessary in my case, but it
> > > is needed for correctness in any case.
> > >
> > > It is unclear to me why placing these helpers out of line should make
> > > any difference, and I am not convinced it is a problem in the code
> > > (IIRC we've had other issues with -Os in the past)
> >
> > Time to start comparing compilers / compiler versions?
> >
> 
> I guess.
> 
> I'll try to dig a bit deeper this afternoon - AIUI, there is never a
> valid reason to return to user space with uaccess disabled, right? So
> we should be able to catch that case using a BUG() or similar, and
> crash in the kernel rather than in user space when this occurs. That
> should make it a bit easier to deal with in the debugger etc.

That _should_ be handled on the kernel exit to userspace path. The
kernel itself should be running with user access disabled except for
the explicit accesses.

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15 11:58                                           ` Linus Walleij
@ 2024-05-15 14:05                                             ` Geert Uytterhoeven
  0 siblings, 0 replies; 49+ messages in thread
From: Geert Uytterhoeven @ 2024-05-15 14:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Ard Biesheuvel, Florian Fainelli, Russell King (Oracle),
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

Hi Linus,

On Wed, May 15, 2024 at 1:58 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, May 15, 2024 at 11:21 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > Ard:
>
> > > Does using __always_inline make any difference in your case?
> >
> > Marking  uaccess_save_and_enable() __always_inline is sufficient
> > to fix the issue.
> >
> > gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)
>
> Can you make a combined patch with __always_inline and the
> "memory" clobber both and send so Russell has something he
> can fold in to stabilize his tree? Like a "this makes my machine
> work" patch.

Done, also submitted to rmk's patch tracker:
https://lore.kernel.org/all/200d273a83906a68a1c4a9298c415980737be811.1715781469.git.geert+renesas@glider.be/
https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9398/1

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15 12:27                                         ` Russell King (Oracle)
@ 2024-05-15 15:41                                           ` Ard Biesheuvel
  2024-05-15 16:18                                             ` Russell King (Oracle)
  0 siblings, 1 reply; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-15 15:41 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Florian Fainelli, Linus Walleij, Geert Uytterhoeven,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, 15 May 2024 at 14:27, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Wed, May 15, 2024 at 10:53:47AM +0200, Ard Biesheuvel wrote:
> > On Wed, 15 May 2024 at 10:48, Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Wed, May 15, 2024 at 10:36:48AM +0200, Ard Biesheuvel wrote:
> > > > On Tue, 14 May 2024 at 22:34, Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > >
> > > > > On 5/14/24 13:33, Linus Walleij wrote:
> > > > > > On Tue, May 14, 2024 at 8:26 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > > > >> On 5/14/24 10:03, Russell King (Oracle) wrote:
> > > > > >
> > > > > >>> I would imagine that the problem is cpu_set_ttbcr(). Please try adding
> > > > > >>> a "memory" clobber to the asm() instruction in there.
> > > > > >>>
> > > > > >>
> > > > > >> I can confirm that with CONFIG_CC_OPTIMIZE_FOR_SIZE=y and the hunk below:
> > > > > >>
> > > > > >> diff --git a/arch/arm/include/asm/proc-fns.h
> > > > > >> b/arch/arm/include/asm/proc-fns.h
> > > > > >> index 9b3105a2a5e0..1087bd2af433 100644
> > > > > >> --- a/arch/arm/include/asm/proc-fns.h
> > > > > >> +++ b/arch/arm/include/asm/proc-fns.h
> > > > > >> @@ -187,7 +187,7 @@ static inline unsigned int cpu_get_ttbcr(void)
> > > > > >>
> > > > > >>    static inline void cpu_set_ttbcr(unsigned int ttbcr)
> > > > > >>    {
> > > > > >> -       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr));
> > > > > >> +       asm("mcr p15, 0, %0, c2, c0, 2" : : "r" (ttbcr) : "memory");
> > > > > >>    }
> > > > > >>
> > > > > >>    #else  /*!CONFIG_MMU */
> > > > > >>
> > > > > >> my Raspberry Pi 4B in AArch32 mode boots and runs user-space properly.
> > > > > >>
> > > > > >> Thanks a lot Russell!
> > > > > >
> > > > > > Second that, very nicely pinpointed Russell!
> > > > > >
> > > > > > Florian, do you want to send a patch or should I?
> > > > >
> > > > > I was wondering if Russell was able to fold this directly into patch #2
> > > > > where cpu_set_ttbr() is added, so as to not break functionality across
> > > > > bisection.
> > > >
> > > > Sadly, I can still reproduce this with the above fix.
> > > >
> > > > I included TTBCR in the DEBUG_USER output, and (as expected), it has
> > > > A1, EPD0 and T0SZ set to the 'uaccess disabled' values.
> > > >
> > > > Using __always_inline on uaccess_save_and_enable() and
> > > > uaccess_restore() (as the CONFIG_CPU_SW_DOMAIN_PAN does) seems to work
> > > > around it. The "memory" clobber seems unnecessary in my case, but it
> > > > is needed for correctness in any case.
> > > >
> > > > It is unclear to me why placing these helpers out of line should make
> > > > any difference, and I am not convinced it is a problem in the code
> > > > (IIRC we've had other issues with -Os in the past)
> > >
> > > Time to start comparing compilers / compiler versions?
> > >
> >
> > I guess.
> >
> > I'll try to dig a bit deeper this afternoon - AIUI, there is never a
> > valid reason to return to user space with uaccess disabled, right? So
> > we should be able to catch that case using a BUG() or similar, and
> > crash in the kernel rather than in user space when this occurs. That
> > should make it a bit easier to deal with in the debugger etc.
>
> That _should_ be handled on the kernel exit to userspace path. The
> kernel itself should be running with user access disabled except for
> the explicit accesses.
>

This definitely looks like some pathological compiler behavior:

The faulting instruction in question is in busybox

   15f450:       eef11a10        vmrs    r1, fpscr

which [as expected] triggers an UNDEF exception as FP is disabled
after a context switch.

The get_user() call in do_undefinstr() [arch/arm/kernel/traps.c:482]

    if (get_user(instr, (u32 __user *)pc))

gets compiled to the below, where the thing to note is that the
out-of-line version of uaccess_save_and_enable() returns the old ttbcr
value in R0. This value gets recorded in R3, but it also gets happily
passed on to __get_user_4(), which expects the user address in R0, not
the old value of TTBCR.

│    0xc040a5e4 <do_undefinstr+228>  bl      0xc0409fe4
<uaccess_save_and_enable>
│    0xc040a5e8 <do_undefinstr+232>  mov     r3, r0
│  > 0xc040a5ec <do_undefinstr+236>  bl      0xc10443a8 <__get_user_4>
│    0xc040a5f0 <do_undefinstr+240>  mcr     15, 0, r3, cr2, cr0, {2}
│    0xc040a5f4 <do_undefinstr+244>  isb     sy

With __always_inline, this part is emitted as

 5fc:   ee124f50        mrc     15, 0, r4, cr2, cr0, {2}
 600:   e0033004        and     r3, r3, r4
 604:   ee023f50        mcr     15, 0, r3, cr2, cr0, {2}
 608:   f57ff06f        isb     sy
 60c:   ebfffffe        bl      0 <__get_user_4>
                        60c: R_ARM_CALL __get_user_4
 610:   ee024f50        mcr     15, 0, r4, cr2, cr0, {2}
 614:   f57ff06f        isb     sy

and so R0 is preserved, and the issue does not happen.

Not sure how to reduce this to a reproducer that can be used to report
the issue to the GCC folks, but it is most definitely a compiler
problem, as far as I can tell.

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15 15:41                                           ` Ard Biesheuvel
@ 2024-05-15 16:18                                             ` Russell King (Oracle)
  2024-05-15 16:36                                               ` Ard Biesheuvel
  0 siblings, 1 reply; 49+ messages in thread
From: Russell King (Oracle) @ 2024-05-15 16:18 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Florian Fainelli, Linus Walleij, Geert Uytterhoeven,
	Arnd Bergmann, Stefan Wahren, Kees Cook, linux-arm-kernel,
	Catalin Marinas

On Wed, May 15, 2024 at 05:41:45PM +0200, Ard Biesheuvel wrote:
> This definitely looks like some pathological compiler behavior:
> 
> The faulting instruction in question is in busybox
> 
>    15f450:       eef11a10        vmrs    r1, fpscr
> 
> which [as expected] triggers an UNDEF exception as FP is disabled
> after a context switch.
> 
> The get_user() call in do_undefinstr() [arch/arm/kernel/traps.c:482]
> 
>     if (get_user(instr, (u32 __user *)pc))
> 
> gets compiled to the below, where the thing to note is that the
> out-of-line version of uaccess_save_and_enable() returns the old ttbcr
> value in R0. This value gets recorded in R3, but it also gets happily
> passed on to __get_user_4(), which expects the user address in R0, not
> the old value of TTBCR.
> 
> │    0xc040a5e4 <do_undefinstr+228>  bl      0xc0409fe4
> <uaccess_save_and_enable>
> │    0xc040a5e8 <do_undefinstr+232>  mov     r3, r0
> │  > 0xc040a5ec <do_undefinstr+236>  bl      0xc10443a8 <__get_user_4>
> │    0xc040a5f0 <do_undefinstr+240>  mcr     15, 0, r3, cr2, cr0, {2}
> │    0xc040a5f4 <do_undefinstr+244>  isb     sy
> 
> With __always_inline, this part is emitted as
> 
>  5fc:   ee124f50        mrc     15, 0, r4, cr2, cr0, {2}
>  600:   e0033004        and     r3, r3, r4
>  604:   ee023f50        mcr     15, 0, r3, cr2, cr0, {2}
>  608:   f57ff06f        isb     sy
>  60c:   ebfffffe        bl      0 <__get_user_4>
>                         60c: R_ARM_CALL __get_user_4
>  610:   ee024f50        mcr     15, 0, r4, cr2, cr0, {2}
>  614:   f57ff06f        isb     sy
> 
> and so R0 is preserved, and the issue does not happen.
> 
> Not sure how to reduce this to a reproducer that can be used to report
> the issue to the GCC folks, but it is most definitely a compiler
> problem, as far as I can tell.

Well this has come up before...

commit 851140ab0d083c78e5723a8b1cbd258f567a7aff
Author: Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Wed Oct 2 11:28:02 2019 +0100

    ARM: 8908/1: add __always_inline to functions called from __get_user_check()

I assume it's a wontfix on the GCC side.

-- 
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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15 16:18                                             ` Russell King (Oracle)
@ 2024-05-15 16:36                                               ` Ard Biesheuvel
  2024-05-15 21:51                                                 ` Arnd Bergmann
  0 siblings, 1 reply; 49+ messages in thread
From: Ard Biesheuvel @ 2024-05-15 16:36 UTC (permalink / raw)
  To: Russell King (Oracle), Arnd Bergmann
  Cc: Florian Fainelli, Linus Walleij, Geert Uytterhoeven,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

On Wed, 15 May 2024 at 18:18, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Wed, May 15, 2024 at 05:41:45PM +0200, Ard Biesheuvel wrote:
> > This definitely looks like some pathological compiler behavior:
> >
> > The faulting instruction in question is in busybox
> >
> >    15f450:       eef11a10        vmrs    r1, fpscr
> >
> > which [as expected] triggers an UNDEF exception as FP is disabled
> > after a context switch.
> >
> > The get_user() call in do_undefinstr() [arch/arm/kernel/traps.c:482]
> >
> >     if (get_user(instr, (u32 __user *)pc))
> >
> > gets compiled to the below, where the thing to note is that the
> > out-of-line version of uaccess_save_and_enable() returns the old ttbcr
> > value in R0. This value gets recorded in R3, but it also gets happily
> > passed on to __get_user_4(), which expects the user address in R0, not
> > the old value of TTBCR.
> >
> > │    0xc040a5e4 <do_undefinstr+228>  bl      0xc0409fe4
> > <uaccess_save_and_enable>
> > │    0xc040a5e8 <do_undefinstr+232>  mov     r3, r0
> > │  > 0xc040a5ec <do_undefinstr+236>  bl      0xc10443a8 <__get_user_4>
> > │    0xc040a5f0 <do_undefinstr+240>  mcr     15, 0, r3, cr2, cr0, {2}
> > │    0xc040a5f4 <do_undefinstr+244>  isb     sy
> >
> > With __always_inline, this part is emitted as
> >
> >  5fc:   ee124f50        mrc     15, 0, r4, cr2, cr0, {2}
> >  600:   e0033004        and     r3, r3, r4
> >  604:   ee023f50        mcr     15, 0, r3, cr2, cr0, {2}
> >  608:   f57ff06f        isb     sy
> >  60c:   ebfffffe        bl      0 <__get_user_4>
> >                         60c: R_ARM_CALL __get_user_4
> >  610:   ee024f50        mcr     15, 0, r4, cr2, cr0, {2}
> >  614:   f57ff06f        isb     sy
> >
> > and so R0 is preserved, and the issue does not happen.
> >
> > Not sure how to reduce this to a reproducer that can be used to report
> > the issue to the GCC folks, but it is most definitely a compiler
> > problem, as far as I can tell.
>
> Well this has come up before...
>
> commit 851140ab0d083c78e5723a8b1cbd258f567a7aff
> Author: Masahiro Yamada <yamada.masahiro@socionext.com>
> Date:   Wed Oct 2 11:28:02 2019 +0100
>
>     ARM: 8908/1: add __always_inline to functions called from __get_user_check()
>
> I assume it's a wontfix on the GCC side.
>

Yes, that is the exact same issue.

Not sure whether it has been reported - the GCC side might not even be aware.

I managed to reproduce this in godbolt - it happens even with -O2 not
just with -Os

https://godbolt.org/z/do56voKsa

As far as I can tell, functions that use asm("r#") cannot safely call
other functions at all unless those are __always_inline. The fact that
it triggers with -Os first is just because it inlines much less
aggressively than other optimization levels.

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement
  2024-05-15 16:36                                               ` Ard Biesheuvel
@ 2024-05-15 21:51                                                 ` Arnd Bergmann
  0 siblings, 0 replies; 49+ messages in thread
From: Arnd Bergmann @ 2024-05-15 21:51 UTC (permalink / raw)
  To: Ard Biesheuvel, Russell King
  Cc: Florian Fainelli, Linus Walleij, Geert Uytterhoeven,
	Stefan Wahren, Kees Cook, linux-arm-kernel, Catalin Marinas

On Wed, May 15, 2024, at 16:36, Ard Biesheuvel wrote:
> On Wed, 15 May 2024 at 18:18, Russell King (Oracle) <linux@armlinux.org.uk> wrote:
>> > and so R0 is preserved, and the issue does not happen.
>> >
>> > Not sure how to reduce this to a reproducer that can be used to report
>> > the issue to the GCC folks, but it is most definitely a compiler
>> > problem, as far as I can tell.
>>
>> Well this has come up before...
>>
>> commit 851140ab0d083c78e5723a8b1cbd258f567a7aff
>> Author: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Date:   Wed Oct 2 11:28:02 2019 +0100
>>
>>     ARM: 8908/1: add __always_inline to functions called from __get_user_check()
>>
>> I assume it's a wontfix on the GCC side.
>>
>
> Yes, that is the exact same issue.
>
> Not sure whether it has been reported - the GCC side might not even be aware.
>
> I managed to reproduce this in godbolt - it happens even with -O2 not
> just with -Os
>
> https://godbolt.org/z/do56voKsa
>
> As far as I can tell, functions that use asm("r#") cannot safely call
> other functions at all unless those are __always_inline. The fact that
> it triggers with -Os first is just because it inlines much less
> aggressively than other optimization levels.

I remember that one now, and I also think this was a wontfix,
since the compiler has no way of doing the right thing here when
a variable is forced into an argument register and then used
after a function call. Spilling the contents on the stack or another
register would break the 'asm("%r0") annotation.

It would be nice to get a compiler warning for code like this, but
I don't know what legitimate use cases that would warn for, e.g. it
may be indistinguishable from the global 'register unsigned long
current_stack_pointer asm("r1");' construct we have on a couple
of architectures.

     Arnd

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

end of thread, other threads:[~2024-05-15 21:53 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-12 12:52 [PATCH v3 0/4] PAN for ARM32 using LPAE Linus Walleij
2024-03-12 12:52 ` [PATCH v3 1/4] ARM: Add TTBCR_* definitions to pgtable-3level-hwdef.h Linus Walleij
2024-03-12 12:52 ` [PATCH v3 2/4] ARM: Move asm statements accessing TTBCR into C functions Linus Walleij
2024-03-12 12:52 ` [PATCH v3 3/4] ARM: Reduce the number of #ifdef CONFIG_CPU_SW_DOMAIN_PAN Linus Walleij
2024-03-12 12:52 ` [PATCH v3 4/4] ARM: Implement PAN for LPAE by TTBR0 page table walks disablement Linus Walleij
2024-05-07 13:10   ` Geert Uytterhoeven
2024-05-13 19:23     ` Linus Walleij
2024-05-13 19:58       ` Geert Uytterhoeven
2024-05-13 20:29         ` Linus Walleij
2024-05-14  3:56           ` Florian Fainelli
2024-05-14  8:14             ` Russell King (Oracle)
2024-05-14 11:22               ` Geert Uytterhoeven
2024-05-14 11:33                 ` Russell King (Oracle)
2024-05-14 12:32                   ` Geert Uytterhoeven
2024-05-14 12:38                     ` Russell King (Oracle)
2024-05-14 15:03                       ` Catalin Marinas
2024-05-14  6:41           ` Geert Uytterhoeven
2024-05-14  7:46             ` Linus Walleij
2024-05-14  7:59               ` Ard Biesheuvel
2024-05-14  8:04                 ` Geert Uytterhoeven
2024-05-14  8:25                   ` Ard Biesheuvel
2024-05-14  9:22                     ` Russell King (Oracle)
2024-05-14 11:40                       ` Linus Walleij
2024-05-14 11:28                     ` Geert Uytterhoeven
2024-05-14 16:06                       ` Geert Uytterhoeven
2024-05-14 16:54                         ` Florian Fainelli
2024-05-14 17:03                           ` Russell King (Oracle)
2024-05-14 18:26                             ` Florian Fainelli
2024-05-14 20:33                               ` Linus Walleij
2024-05-14 20:34                                 ` Florian Fainelli
2024-05-15  8:36                                   ` Ard Biesheuvel
2024-05-15  8:45                                     ` Geert Uytterhoeven
2024-05-15  8:49                                       ` Ard Biesheuvel
2024-05-15  9:21                                         ` Geert Uytterhoeven
2024-05-15  9:39                                           ` Ard Biesheuvel
2024-05-15 11:58                                           ` Linus Walleij
2024-05-15 14:05                                             ` Geert Uytterhoeven
2024-05-15  8:48                                     ` Russell King (Oracle)
2024-05-15  8:53                                       ` Ard Biesheuvel
2024-05-15 12:27                                         ` Russell King (Oracle)
2024-05-15 15:41                                           ` Ard Biesheuvel
2024-05-15 16:18                                             ` Russell King (Oracle)
2024-05-15 16:36                                               ` Ard Biesheuvel
2024-05-15 21:51                                                 ` Arnd Bergmann
2024-05-15  8:10                               ` Geert Uytterhoeven
2024-05-14  7:37           ` Linus Walleij
2024-05-14 14:39             ` Catalin Marinas
2024-03-12 17:45 ` [PATCH v3 0/4] PAN for ARM32 using LPAE Florian Fainelli
2024-03-13  8:13   ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).