xtensa: make arch_user_string_nlen actually work

arch_user_string_nlen() did not exactly work correctly as any
invalid pointers being passed are de-referenced naively, which
results in DTLB misses (MMU) or access errors (MPU). However,
arch_user_string_nlen() should always return to the caller
with appropriate error code set, and should never result in
thread termination. Since we are usually going through syscalls
when arch_user_string_nlen() is called, for MMU, the DTLB miss
goes through double exception. Since the pointer is invalid,
there is a high chance there is not even a L2 page table
associated with that bad address. So the DTLB miss cannot be
handled and it just keeps looping in double exception until
there is another exception type where we get to the C handler.
However, the stack frame is no longer the frame associated
with the call to arch_user_string_nlen(), and the call return
address would be incorrect. Forcing this incorrect address as
the next PC would result in some other exceptions, e.g.
illegal instruction, which would go to the C handler again.
This time it will go to the end of handler and would result
in thread termination. For MPU systems, access errors would
simply result in thread terminal in the C handler. Because of
these reasons, change the arch_user_string_nlen() to check if
the memory region can be accessed under kernel mode first
before feeding it to strnlen().

Also remove the exception fixup arrays as there is nothing
there anymore.

Signed-off-by: Daniel Leung <[email protected]>

Author: dcpleung

arch/xtensa/core/syscall_helper.c

@@ -4,8 +4,13 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#include <string.h>
+
 #include <zephyr/arch/xtensa/syscall.h>
 
+#include <zephyr/internal/syscall_handler.h>
+#include <xtensa_internal.h>
+
 #ifdef CONFIG_XTENSA_SYSCALL_USE_HELPER
 uintptr_t xtensa_syscall_helper(uintptr_t arg1, uintptr_t arg2,
 				uintptr_t arg3, uintptr_t arg4,
@@ -47,3 +52,41 @@ bool xtensa_is_user_context(void)
 	return ret != 0;
 }
 #endif /* XCHAL_HAVE_THREADPTR */
+
+size_t arch_user_string_nlen(const char *s, size_t maxsize, int *err_arg)
+{
+	/* Check if we can actually read the whole length.
+	 *
+	 * arch_user_string_nlen() is supposed to naively go through
+	 * the string passed from user thread, and relies on page faults
+	 * to catch inaccessible strings, such that user thread can pass
+	 * a string that is shorter than the max length this function
+	 * caller expects. So at least we want to make sure kernel has
+	 * access to the whole length, aka. memory being mapped.
+	 * Note that arch_user_string_nlen() should never result in
+	 * thread termination due to page faults, and must always
+	 * return to the caller with err_arg set or cleared.
+	 * For MMU systems, unmapped memory will result in a DTLB miss
+	 * and that might trigger an infinite DTLB miss storm if
+	 * the corresponding L2 page table never exists in the first
+	 * place (which would result in DTLB misses through L1 page
+	 * table), until some other exceptions occur to break
+	 * the cycle.
+	 * For MPU systems, this would simply results in access errors
+	 * and the exception handler will terminate the thread.
+	 */
+	if (!xtensa_mem_kernel_has_access((void *)s, maxsize, 0)) {
+		/*
+		 * API says we need to set err_arg to -1 if there are
+		 * any errors.
+		 */
+		*err_arg = -1;
+
+		return 0;
+	}
+
+	/* No error and we can proceed to getting the string length. */
+	*err_arg = 0;
+
+	return strnlen(s, maxsize);
+}

arch/xtensa/core/userspace.S

@@ -352,45 +352,3 @@ xtensa_userspace_enter:
 	movi a0, 0
 
 	rfi 2
-
-/*
- * size_t arch_user_string_nlen(const char *s, size_t maxsize, int *err_arg)
- */
-.global arch_user_string_nlen
-.type arch_user_string_nlen, @function
-.align 4
-arch_user_string_nlen:
-	entry a1, 32
-
-	/* error value, set to -1. */
-	movi a5, -1
-	s32i a5, a4, 0
-
-	/* length count */
-	xor a5, a5, a5
-
-	/* This code might page fault */
-strlen_loop:
-.global xtensa_user_string_nlen_fault_start
-xtensa_user_string_nlen_fault_start:
-	l8ui a6, a2, 0 /* Current char */
-
-.global xtensa_user_string_nlen_fault_end
-xtensa_user_string_nlen_fault_end:
-	beqz a6, strlen_done
-	addi a5, a5, 1
-	addi a2, a2, 1
-	beq a5, a3, strlen_done
-	j strlen_loop
-
-strlen_done:
-	/* Set return value */
-	mov a2, a5
-
-	/* Set error value to 0 since we succeeded */
-	movi a5, 0x0
-	s32i a5, a4, 0
-
-.global xtensa_user_string_nlen_fixup
-xtensa_user_string_nlen_fixup:
-	retw

arch/xtensa/core/vector_handlers.c

@@ -29,14 +29,6 @@ LOG_MODULE_DECLARE(os, CONFIG_KERNEL_LOG_LEVEL);
 extern char xtensa_arch_except_epc[];
 extern char xtensa_arch_kernel_oops_epc[];
 
-#ifdef CONFIG_USERSPACE
-Z_EXC_DECLARE(xtensa_user_string_nlen);
-
-static const struct z_exc_handle exceptions[] = {
-	Z_EXC_HANDLE(xtensa_user_string_nlen)
-};
-#endif /* CONFIG_USERSPACE */
-
 void xtensa_dump_stack(const void *stack)
 {
 	_xtensa_irq_stack_frame_raw_t *frame = (void *)stack;
@@ -265,21 +257,6 @@ void *xtensa_excint1_c(void *esf)
 		ps = bsa->ps;
 		pc = (void *)bsa->pc;
 
-#ifdef CONFIG_USERSPACE
-		/* If the faulting address is from one of the known
-		 * exceptions that should not be fatal, return to
-		 * the fixup address.
-		 */
-		for (int i = 0; i < ARRAY_SIZE(exceptions); i++) {
-			if ((pc >= exceptions[i].start) &&
-			    (pc < exceptions[i].end)) {
-				bsa->pc = (uintptr_t)exceptions[i].fixup;
-
-				goto fixup_out;
-			}
-		}
-#endif /* CONFIG_USERSPACE */
-
 		/* Default for exception */
 		int reason = K_ERR_CPU_EXCEPTION;
 		is_fatal_error = true;
@@ -371,9 +348,6 @@ void *xtensa_excint1_c(void *esf)
 		_current_cpu->nested = 1;
 	}
 
-#ifdef CONFIG_USERSPACE
-fixup_out:
-#endif
 #if defined(CONFIG_XTENSA_MMU)
 	if (is_dblexc) {
 		XTENSA_WSR(ZSR_DEPC_SAVE_STR, 0);