You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2023/01/26 12:41:49 UTC
[nuttx] branch master updated: arch/ARCH_KERNEL_STACK: Fix signal handling with kernel stack
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 686b990a85 arch/ARCH_KERNEL_STACK: Fix signal handling with kernel stack
686b990a85 is described below
commit 686b990a8529d3a85854590d285e5da67a2e6547
Author: Ville Juven <vi...@unikie.com>
AuthorDate: Tue Jan 24 14:20:55 2023 +0200
arch/ARCH_KERNEL_STACK: Fix signal handling with kernel stack
There were two issues with signal handling:
- With a kernel stack the "info" parameter was passed from kernel memory.
This is fixed by making a stack frame to the user stack and copying it
there.
- If the signal handler uses a system call, the kernel stack was completely
and unconditionally destroyed, resulting in a crash in the user application
There is also no need to check ustkptr, it is always NULL. Why ? Because
signal delivery is deferred when a system call is being executed.
---
arch/arm/src/armv7-a/arm_syscall.c | 38 +++++++++++++++++++++-----
arch/arm/src/armv7-r/arm_syscall.c | 40 +++++++++++++++++++++++-----
arch/arm64/src/common/arm64_syscall.c | 50 ++++++++++++++++++++++++++---------
arch/risc-v/src/common/riscv_swint.c | 41 ++++++++++++++++++++++------
4 files changed, 136 insertions(+), 33 deletions(-)
diff --git a/arch/arm/src/armv7-a/arm_syscall.c b/arch/arm/src/armv7-a/arm_syscall.c
index 4ca5b9441f..55d54d9c96 100644
--- a/arch/arm/src/armv7-a/arm_syscall.c
+++ b/arch/arm/src/armv7-a/arm_syscall.c
@@ -438,11 +438,31 @@ uint32_t *arm_syscall(uint32_t *regs)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr == NULL &&
- rtcb->xcp.ustkptr != NULL);
+ uint32_t usp;
+
+ DEBUGASSERT(rtcb->xcp.kstkptr == NULL);
+
+ /* Copy "info" into user stack */
+
+ if (rtcb->xcp.sigdeliver)
+ {
+ usp = rtcb->xcp.saved_regs[REG_SP];
+ }
+ else
+ {
+ usp = rtcb->xcp.regs[REG_SP];
+ }
+
+ /* Create a frame for info and copy the kernel info */
+
+ usp = usp - sizeof(siginfo_t);
+ memcpy((void *)usp, (void *)regs[REG_R2], sizeof(siginfo_t));
+
+ /* Now set the updated SP and user copy of "info" to R2 */
rtcb->xcp.kstkptr = (uint32_t *)regs[REG_SP];
- regs[REG_SP] = (uint32_t)rtcb->xcp.ustkptr;
+ regs[REG_SP] = usp;
+ regs[REG_R2] = usp;
}
#endif
}
@@ -480,8 +500,7 @@ uint32_t *arm_syscall(uint32_t *regs)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr != NULL &&
- (uint32_t)rtcb->xcp.ustkptr == regs[REG_SP]);
+ DEBUGASSERT(rtcb->xcp.kstkptr != NULL);
regs[REG_SP] = (uint32_t)rtcb->xcp.kstkptr;
rtcb->xcp.kstkptr = NULL;
@@ -540,8 +559,15 @@ uint32_t *arm_syscall(uint32_t *regs)
if (index == 0 && rtcb->xcp.kstack != NULL)
{
rtcb->xcp.ustkptr = (uint32_t *)regs[REG_SP];
- regs[REG_SP] = (uint32_t)rtcb->xcp.kstack +
+ if (rtcb->xcp.kstkptr != NULL)
+ {
+ regs[REG_SP] = (uint32_t)rtcb->xcp.kstkptr;
+ }
+ else
+ {
+ regs[REG_SP] = (uint32_t)rtcb->xcp.kstack +
ARCH_KERNEL_STACKSIZE;
+ }
}
#endif
diff --git a/arch/arm/src/armv7-r/arm_syscall.c b/arch/arm/src/armv7-r/arm_syscall.c
index fce27f5373..20bd7803cb 100644
--- a/arch/arm/src/armv7-r/arm_syscall.c
+++ b/arch/arm/src/armv7-r/arm_syscall.c
@@ -434,11 +434,31 @@ uint32_t *arm_syscall(uint32_t *regs)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr == NULL &&
- rtcb->xcp.ustkptr != NULL);
+ uint32_t usp;
+
+ DEBUGASSERT(rtcb->xcp.kstkptr == NULL);
+
+ /* Copy "info" into user stack */
+
+ if (rtcb->xcp.sigdeliver)
+ {
+ usp = rtcb->xcp.saved_regs[REG_SP];
+ }
+ else
+ {
+ usp = rtcb->xcp.regs[REG_SP];
+ }
+
+ /* Create a frame for info and copy the kernel info */
+
+ usp = usp - sizeof(siginfo_t);
+ memcpy((void *)usp, (void *)regs[REG_R2], sizeof(siginfo_t));
+
+ /* Now set the updated SP and user copy of "info" to R2 */
rtcb->xcp.kstkptr = (uint32_t *)regs[REG_SP];
- regs[REG_SP] = (uint32_t)rtcb->xcp.ustkptr;
+ regs[REG_SP] = usp;
+ regs[REG_R2] = usp;
}
#endif
}
@@ -476,8 +496,7 @@ uint32_t *arm_syscall(uint32_t *regs)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr != NULL &&
- (uint32_t)rtcb->xcp.ustkptr == regs[REG_SP]);
+ DEBUGASSERT(rtcb->xcp.kstkptr != NULL);
regs[REG_SP] = (uint32_t)rtcb->xcp.kstkptr;
rtcb->xcp.kstkptr = NULL;
@@ -536,8 +555,15 @@ uint32_t *arm_syscall(uint32_t *regs)
if (index == 0 && rtcb->xcp.kstack != NULL)
{
rtcb->xcp.ustkptr = (uint32_t *)regs[REG_SP];
- regs[REG_SP] = (uint32_t)rtcb->xcp.kstack +
- ARCH_KERNEL_STACKSIZE;
+ if (rtcb->xcp.kstkptr != NULL)
+ {
+ regs[REG_SP] = (uint32_t)rtcb->xcp.kstkptr;
+ }
+ else
+ {
+ regs[REG_SP] = (uint32_t)rtcb->xcp.kstack +
+ ARCH_KERNEL_STACKSIZE;
+ }
}
#endif
diff --git a/arch/arm64/src/common/arm64_syscall.c b/arch/arm64/src/common/arm64_syscall.c
index 4f97c2cf1f..eb8b552ad9 100644
--- a/arch/arm64/src/common/arm64_syscall.c
+++ b/arch/arm64/src/common/arm64_syscall.c
@@ -297,7 +297,7 @@ int arm64_syscall(uint64_t *regs)
if (index == 0 && rtcb->xcp.ustkptr != NULL)
{
- regs[REG_SP] = (uint32_t)rtcb->xcp.ustkptr;
+ regs[REG_SP] = (uint64_t)rtcb->xcp.ustkptr;
rtcb->xcp.ustkptr = NULL;
}
#endif
@@ -413,7 +413,7 @@ int arm64_syscall(uint64_t *regs)
* unprivileged mode.
*/
- regs[REG_PC] = (uint32_t)ARCH_DATA_RESERVE->ar_sigtramp;
+ regs[REG_PC] = (uint64_t)ARCH_DATA_RESERVE->ar_sigtramp;
cpsr = regs[REG_CPSR] & ~PSR_MODE_MASK;
regs[REG_CPSR] = cpsr | PSR_MODE_USR;
@@ -436,11 +436,31 @@ int arm64_syscall(uint64_t *regs)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr == NULL &&
- rtcb->xcp.ustkptr != NULL);
+ uint64_t usp;
- rtcb->xcp.kstkptr = (uint32_t *)regs[REG_SP];
- regs[REG_SP] = (uint32_t)rtcb->xcp.ustkptr;
+ DEBUGASSERT(rtcb->xcp.kstkptr == NULL);
+
+ /* Copy "info" into user stack */
+
+ if (rtcb->xcp.sigdeliver)
+ {
+ usp = rtcb->xcp.saved_regs[REG_SP];
+ }
+ else
+ {
+ usp = rtcb->xcp.regs[REG_SP];
+ }
+
+ /* Create a frame for info and copy the kernel info */
+
+ usp = usp - sizeof(siginfo_t);
+ memcpy((void *)usp, (void *)regs[REG_R2], sizeof(siginfo_t));
+
+ /* Now set the updated SP and user copy of "info" to R2 */
+
+ rtcb->xcp.kstkptr = (uint64_t *)regs[REG_SP];
+ regs[REG_SP] = usp;
+ regs[REG_R2] = usp;
}
#endif
}
@@ -478,10 +498,9 @@ int arm64_syscall(uint64_t *regs)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr != NULL &&
- (uint32_t)rtcb->xcp.ustkptr == regs[REG_SP]);
+ DEBUGASSERT(rtcb->xcp.kstkptr != NULL);
- regs[REG_SP] = (uint32_t)rtcb->xcp.kstkptr;
+ regs[REG_SP] = (uint64_t)rtcb->xcp.kstkptr;
rtcb->xcp.kstkptr = NULL;
}
#endif
@@ -517,7 +536,7 @@ int arm64_syscall(uint64_t *regs)
rtcb->xcp.syscall[index].cpsr = regs[REG_CPSR];
#endif
- regs[REG_PC] = (uint32_t)dispatch_syscall;
+ regs[REG_PC] = (uint64_t)dispatch_syscall;
#ifdef CONFIG_BUILD_KERNEL
cpsr = regs[REG_CPSR] & ~PSR_MODE_MASK;
regs[REG_CPSR] = cpsr | PSR_MODE_SVC;
@@ -537,9 +556,16 @@ int arm64_syscall(uint64_t *regs)
if (index == 0 && rtcb->xcp.kstack != NULL)
{
- rtcb->xcp.ustkptr = (uint32_t *)regs[REG_SP];
- regs[REG_SP] = (uint32_t)rtcb->xcp.kstack +
+ rtcb->xcp.ustkptr = (uint64_t *)regs[REG_SP];
+ if (rtcb->xcp.kstkptr != NULL)
+ {
+ regs[REG_SP] = (uint64_t)rtcb->xcp.kstkptr;
+ }
+ else
+ {
+ regs[REG_SP] = (uint64_t)rtcb->xcp.kstack +
ARCH_KERNEL_STACKSIZE;
+ }
}
#endif
diff --git a/arch/risc-v/src/common/riscv_swint.c b/arch/risc-v/src/common/riscv_swint.c
index b20c25341c..9104a1a1a0 100644
--- a/arch/risc-v/src/common/riscv_swint.c
+++ b/arch/risc-v/src/common/riscv_swint.c
@@ -366,8 +366,7 @@ int riscv_swint(int irq, void *context, void *arg)
#if defined (CONFIG_BUILD_PROTECTED)
regs[REG_EPC] = (uintptr_t)USERSPACE->signal_handler;
#else
- regs[REG_EPC] =
- (uintptr_t)ARCH_DATA_RESERVE->ar_sigtramp;
+ regs[REG_EPC] = (uintptr_t)ARCH_DATA_RESERVE->ar_sigtramp;
#endif
regs[REG_INT_CTX] &= ~STATUS_PPP; /* User mode */
@@ -390,11 +389,31 @@ int riscv_swint(int irq, void *context, void *arg)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr == NULL &&
- rtcb->xcp.ustkptr != NULL);
+ uintptr_t usp;
+
+ DEBUGASSERT(rtcb->xcp.kstkptr == NULL);
+
+ /* Copy "info" into user stack */
+
+ if (rtcb->xcp.sigdeliver)
+ {
+ usp = rtcb->xcp.saved_regs[REG_SP];
+ }
+ else
+ {
+ usp = rtcb->xcp.regs[REG_SP];
+ }
+
+ /* Create a frame for info and copy the kernel info */
+
+ usp = usp - sizeof(siginfo_t);
+ memcpy((void *)usp, (void *)regs[REG_A2], sizeof(siginfo_t));
+
+ /* Now set the updated SP and user copy of "info" to A2 */
rtcb->xcp.kstkptr = (uintptr_t *)regs[REG_SP];
- regs[REG_SP] = (uintptr_t)rtcb->xcp.ustkptr;
+ regs[REG_SP] = usp;
+ regs[REG_A2] = usp;
}
#endif
}
@@ -431,8 +450,7 @@ int riscv_swint(int irq, void *context, void *arg)
if (rtcb->xcp.kstack != NULL)
{
- DEBUGASSERT(rtcb->xcp.kstkptr != NULL &&
- (uintptr_t)rtcb->xcp.ustkptr == regs[REG_SP]);
+ DEBUGASSERT(rtcb->xcp.kstkptr != NULL);
regs[REG_SP] = (uintptr_t)rtcb->xcp.kstkptr;
rtcb->xcp.kstkptr = NULL;
@@ -497,8 +515,15 @@ int riscv_swint(int irq, void *context, void *arg)
if (index == 0 && rtcb->xcp.kstack != NULL)
{
rtcb->xcp.ustkptr = (uintptr_t *)regs[REG_SP];
- regs[REG_SP] = (uintptr_t)rtcb->xcp.kstack +
+ if (rtcb->xcp.kstkptr != NULL)
+ {
+ regs[REG_SP] = (uintptr_t)rtcb->xcp.kstkptr;
+ }
+ else
+ {
+ regs[REG_SP] = (uintptr_t)rtcb->xcp.kstack +
ARCH_KERNEL_STACKSIZE;
+ }
}
#endif
}