Sun Apr 14 12:51:17 2024 UTC (36d)
kern/58149: aarch64: Cannot return from a signal handler if SP was misaligned when the signal arrived

Apply the kernel diff from the PR

1. sendsig_siginfo() previously assumed that user SP was always aligned to
   16 bytes and could call signal handlers with SP misaligned. This is a
   wrong assumption because aarch64 demands that SP is aligned *only while*
   it's being used to access memory. Now it properly aligns it before
   pusing anything on the stack.

2. cpu_mcontext_validate() used to check if _REG_SP was aligned and
   considered the ucontext invalid otherwise. This meant if a signal was
   sent to a process whose SP was misaligned, the signal handler would fail
   to return because the ucontext passed from the kernel was an invalid
   one. Now setcontext(2) doesn't complain about misaligned SP.


(skrll)
diff -r1.14 -r1.15 src/sys/arch/aarch64/aarch64/cpu_machdep.c
diff -r1.8 -r1.9 src/sys/arch/aarch64/aarch64/sig_machdep.c

cvs diff -r1.14 -r1.15 src/sys/arch/aarch64/aarch64/cpu_machdep.c (expand / switch to context diff)
--- src/sys/arch/aarch64/aarch64/cpu_machdep.c 2023/02/25 00:40:22 1.14
+++ src/sys/arch/aarch64/aarch64/cpu_machdep.c 2024/04/14 12:51:16 1.15
@@ -1,4 +1,4 @@
-/* $NetBSD: cpu_machdep.c,v 1.14 2023/02/25 00:40:22 riastradh Exp $ */
+/* $NetBSD: cpu_machdep.c,v 1.15 2024/04/14 12:51:16 skrll Exp $ */
 
 /*-
  * Copyright (c) 2014, 2019 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
 
 #include <sys/cdefs.h>
 
-__KERNEL_RCSID(1, "$NetBSD: cpu_machdep.c,v 1.14 2023/02/25 00:40:22 riastradh Exp $");
+__KERNEL_RCSID(1, "$NetBSD: cpu_machdep.c,v 1.15 2024/04/14 12:51:16 skrll Exp $");
 
 #include "opt_multiprocessor.h"
 
@@ -158,9 +158,13 @@
 int
 cpu_mcontext_validate(struct lwp *l, const mcontext_t *mcp)
 {
+	/*
+	 * We intentionally don't verify that _REG_SP is aligned to
+	 * 16-bytes boundaries because it can be legally misaligned as long
+	 * as it's not used for accessing memory.
+	 */
 	if ((mcp->__gregs[_REG_SPSR] & ~SPSR_NZCV)
-	    || (mcp->__gregs[_REG_PC] & 3)
-	    || (mcp->__gregs[_REG_SP] & 15))
+	    || (mcp->__gregs[_REG_PC] & 3))
 		return EINVAL;
 
 	return 0;

cvs diff -r1.8 -r1.9 src/sys/arch/aarch64/aarch64/sig_machdep.c (expand / switch to context diff)
--- src/sys/arch/aarch64/aarch64/sig_machdep.c 2021/11/01 05:07:15 1.8
+++ src/sys/arch/aarch64/aarch64/sig_machdep.c 2024/04/14 12:51:16 1.9
@@ -1,4 +1,4 @@
-/* $NetBSD: sig_machdep.c,v 1.8 2021/11/01 05:07:15 thorpej Exp $ */
+/* $NetBSD: sig_machdep.c,v 1.9 2024/04/14 12:51:16 skrll Exp $ */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
 
 #include <sys/cdefs.h>
 
-__KERNEL_RCSID(1, "$NetBSD: sig_machdep.c,v 1.8 2021/11/01 05:07:15 thorpej Exp $");
+__KERNEL_RCSID(1, "$NetBSD: sig_machdep.c,v 1.9 2024/04/14 12:51:16 skrll Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -58,8 +58,14 @@
 
 	vaddr_t sp;
 
-	sp = onstack_p ? ((vaddr_t)ss->ss_sp + ss->ss_size) & -16 : tf->tf_sp;
+	/*
+	 * The user stack isn't guaranteed to be aligned to 16 bytes. Align
+	 * it before pushing anything onto it.
+	 */
+	sp = onstack_p ? ((vaddr_t)ss->ss_sp + ss->ss_size) : tf->tf_sp;
+	sp &= -16;
 
+	__CTASSERT(sizeof(ucontext_t) % 16 == 0);
 	sp -= sizeof(ucontext_t);
 	const vaddr_t ucp = sp;