diff --git a/TODO b/TODO index 8c1133404d9..c397e69e0fe 100644 --- a/TODO +++ b/TODO @@ -1,4 +1,4 @@ -NuttX TODO List (Last updated January 21, 2019) +NuttX TODO List (Last updated February 4, 2019) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This file summarizes known NuttX bugs, limitations, inconsistencies with @@ -16,7 +16,7 @@ nuttx/: (3) Signals (sched/signal, arch/) (2) pthreads (sched/pthread) (0) Message Queues (sched/mqueue) - (9) Kernel/Protected Build + (10) Kernel/Protected Build (3) C++ Support (5) Binary loaders (binfmt/) (18) Network (net/, drivers/net) @@ -1060,6 +1060,28 @@ o Kernel/Protected Build performance implications if the the errno variable is accessed via a system call at high rates. + Title: SIGNAL DELIVERY VULNERABILITY + Description: When a signal is delivered, the user stack is used. Unlike + Linux, applications do not have separate user and supervisor + stacks; everything is done on the user stack. + + In the implementation of up_sigdeliver, a copy of the + register contents that will be restored is present on the + stack and could be modified by the user application. Thus, + if the user mucks with the return stack, problems could + occur when the user task returns to supervisor mode from + the the signal handler. + + A recent commit (4 Feb 2019) does protect the status register + and return address so that a malicious task cannot change the + return address and switch to supervisor mode. Other register + are still modifiable and there is other possible mayhem that + could be done. + Status: Open + Priority: Medium-ish if are attempting to make a secure environment that + may host malicious code. Very low for the typical FLAT build, + however. + o C++ Support ^^^^^^^^^^^ diff --git a/arch/arm/src/arm/up_sigdeliver.c b/arch/arm/src/arm/up_sigdeliver.c index 5eae180317b..6406536c30e 100644 --- a/arch/arm/src/arm/up_sigdeliver.c +++ b/arch/arm/src/arm/up_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/arm/src/arm/up_sigdeliver.c * - * Copyright (C) 2007-2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2007-2010, 2015, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -54,18 +55,6 @@ #ifndef CONFIG_DISABLE_SIGNALS -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -99,11 +88,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copyfullstate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; - regs[REG_CPSR] = rtcb->xcp.saved_cpsr; /* Get a local copy of the sigdeliver function pointer. we do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -134,6 +121,19 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; + regs[REG_CPSR] = rtcb->xcp.saved_cpsr; + /* Then restore the correct state for this thread of execution. */ board_autoled_off(LED_SIGNAL); diff --git a/arch/arm/src/armv6-m/up_sigdeliver.c b/arch/arm/src/armv6-m/up_sigdeliver.c index c4781b4dce1..1af19185c20 100644 --- a/arch/arm/src/armv6-m/up_sigdeliver.c +++ b/arch/arm/src/armv6-m/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/src/armv6-m/up_sigdeliver.c * - * Copyright (C) 2013-2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2013-2015, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -92,15 +92,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copyfullstate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; - regs[REG_PRIMASK] = rtcb->xcp.saved_primask; - regs[REG_XPSR] = rtcb->xcp.saved_xpsr; -#ifdef CONFIG_BUILD_PROTECTED - regs[REG_LR] = rtcb->xcp.saved_lr; -#endif /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -131,6 +125,23 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; + regs[REG_PRIMASK] = rtcb->xcp.saved_primask; + regs[REG_XPSR] = rtcb->xcp.saved_xpsr; +#ifdef CONFIG_BUILD_PROTECTED + regs[REG_LR] = rtcb->xcp.saved_lr; +#endif + /* Then restore the correct state for this thread of * execution. */ diff --git a/arch/arm/src/armv7-a/arm_sigdeliver.c b/arch/arm/src/armv7-a/arm_sigdeliver.c index d47041dafe7..5f07737110b 100644 --- a/arch/arm/src/armv7-a/arm_sigdeliver.c +++ b/arch/arm/src/armv7-a/arm_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/arm/src/armv7-a/arm_sigdeliver.c * - * Copyright (C) 2013, 2015-2016, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2013, 2015-2016, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -96,11 +97,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copyfullstate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; - regs[REG_CPSR] = rtcb->xcp.saved_cpsr; /* Get a local copy of the sigdeliver function pointer. we do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -161,7 +160,20 @@ void up_sigdeliver(void) /* Restore the saved errno value */ - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; + regs[REG_CPSR] = rtcb->xcp.saved_cpsr; #ifdef CONFIG_SMP /* Restore the saved 'irqcount' and recover the critical section diff --git a/arch/arm/src/armv7-m/up_sigdeliver.c b/arch/arm/src/armv7-m/up_sigdeliver.c index 3b917eb3cb5..b6b3c18240c 100644 --- a/arch/arm/src/armv7-m/up_sigdeliver.c +++ b/arch/arm/src/armv7-m/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/src/armv7-m/up_sigdeliver.c * - * Copyright (C) 2009-2010, 2013-2016, 2018 Gregory Nutt. All rights + * Copyright (C) 2009-2010, 2013-2016, 2018-2019 Gregory Nutt. All rights * reserved. * Author: Gregory Nutt * @@ -97,19 +97,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copyfullstate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; -#ifdef CONFIG_ARMV7M_USEBASEPRI - regs[REG_BASEPRI] = rtcb->xcp.saved_basepri; -#else - regs[REG_PRIMASK] = rtcb->xcp.saved_primask; -#endif - regs[REG_XPSR] = rtcb->xcp.saved_xpsr; -#ifdef CONFIG_BUILD_PROTECTED - regs[REG_LR] = rtcb->xcp.saved_lr; -#endif /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -174,7 +164,28 @@ void up_sigdeliver(void) /* Restore the saved errno value */ - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; +#ifdef CONFIG_ARMV7M_USEBASEPRI + regs[REG_BASEPRI] = rtcb->xcp.saved_basepri; +#else + regs[REG_PRIMASK] = rtcb->xcp.saved_primask; +#endif + regs[REG_XPSR] = rtcb->xcp.saved_xpsr; +#ifdef CONFIG_BUILD_PROTECTED + regs[REG_LR] = rtcb->xcp.saved_lr; +#endif #ifdef CONFIG_SMP /* Restore the saved 'irqcount' and recover the critical section diff --git a/arch/arm/src/armv7-r/arm_sigdeliver.c b/arch/arm/src/armv7-r/arm_sigdeliver.c index d5260acdca7..8e3e7a3940d 100644 --- a/arch/arm/src/armv7-r/arm_sigdeliver.c +++ b/arch/arm/src/armv7-r/arm_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/arm/src/armv7-r/arm_sigdeliver.c * - * Copyright (C) 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2015, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -87,11 +87,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copyfullstate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; - regs[REG_CPSR] = rtcb->xcp.saved_cpsr; /* Get a local copy of the sigdeliver function pointer. we do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -122,6 +120,19 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; + regs[REG_CPSR] = rtcb->xcp.saved_cpsr; + /* Then restore the correct state for this thread of execution. */ board_autoled_off(LED_SIGNAL); diff --git a/arch/avr/src/avr/up_sigdeliver.c b/arch/avr/src/avr/up_sigdeliver.c index 4a548597a1b..30bfa8b8e62 100644 --- a/arch/avr/src/avr/up_sigdeliver.c +++ b/arch/avr/src/avr/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/avr/src/avr/up_sigdeliver.c * - * Copyright (C) 2011, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2011, 2015, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -86,15 +86,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_PC0] = rtcb->xcp.saved_pc0; - regs[REG_PC1] = rtcb->xcp.saved_pc1; -#if defined(REG_PC2) - regs[REG_PC2] = rtcb->xcp.saved_pc2; -#endif - regs[REG_SREG] = rtcb->xcp.saved_sreg; /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -125,6 +119,23 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC0] = rtcb->xcp.saved_pc0; + regs[REG_PC1] = rtcb->xcp.saved_pc1; +#if defined(REG_PC2) + regs[REG_PC2] = rtcb->xcp.saved_pc2; +#endif + regs[REG_SREG] = rtcb->xcp.saved_sreg; + /* Then restore the correct state for this thread of execution. This is an * unusual case that must be handled by up_fullcontextresore. This case is * unusal in two ways: diff --git a/arch/avr/src/avr32/up_sigdeliver.c b/arch/avr/src/avr32/up_sigdeliver.c index c3ffe6f8789..2b175c77d08 100644 --- a/arch/avr/src/avr32/up_sigdeliver.c +++ b/arch/avr/src/avr32/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/avr/src/avr32/up_sigdeliver.c * - * Copyright (C) 2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2010, 2015, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -90,11 +90,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; - regs[REG_SR] = rtcb->xcp.saved_sr; /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -125,6 +123,19 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; + regs[REG_SR] = rtcb->xcp.saved_sr; + /* Then restore the correct state for this thread of execution. This is an * unusual case that must be handled by up_fullcontextresore. This case is * unusal in two ways: diff --git a/arch/mips/src/mips32/up_sigdeliver.c b/arch/mips/src/mips32/up_sigdeliver.c index b56cde6487e..3f0f3c9be15 100644 --- a/arch/mips/src/mips32/up_sigdeliver.c +++ b/arch/mips/src/mips32/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/mips/src/mips32/up_sigdeliver.c * - * Copyright (C) 2011, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2011, 2015, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -55,18 +55,6 @@ #ifndef CONFIG_DISABLE_SIGNALS -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -100,11 +88,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_EPC] = rtcb->xcp.saved_epc; - regs[REG_STATUS] = rtcb->xcp.saved_status; /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -134,7 +120,20 @@ void up_sigdeliver(void) sinfo("Resuming EPC: %08x STATUS: %08x\n", regs[REG_EPC], regs[REG_STATUS]); (void)up_irq_save(); - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_EPC] = rtcb->xcp.saved_epc; + regs[REG_STATUS] = rtcb->xcp.saved_status; /* Then restore the correct state for this thread of * execution. diff --git a/arch/misoc/src/lm32/lm32_sigdeliver.c b/arch/misoc/src/lm32/lm32_sigdeliver.c index 8b1b18ec8f1..276c46741f6 100644 --- a/arch/misoc/src/lm32/lm32_sigdeliver.c +++ b/arch/misoc/src/lm32/lm32_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/misoc/src/lm32/lm32_sigdeliver.c * - * Copyright (C) 2016, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2016, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -87,11 +87,9 @@ void lm32_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_EPC] = rtcb->xcp.saved_epc; - regs[REG_INT_CTX] = rtcb->xcp.saved_int_ctx; /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -121,7 +119,20 @@ void lm32_sigdeliver(void) sinfo("Resuming EPC: %08x INT_CTX: %08x\n", regs[REG_EPC], regs[REG_INT_CTX]); (void)up_irq_save(); - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_EPC] = rtcb->xcp.saved_epc; + regs[REG_INT_CTX] = rtcb->xcp.saved_int_ctx; /* Then restore the correct state for this thread of * execution. diff --git a/arch/renesas/src/m16c/m16c_sigdeliver.c b/arch/renesas/src/m16c/m16c_sigdeliver.c index 46e511a0bd0..390d6dec9b7 100644 --- a/arch/renesas/src/m16c/m16c_sigdeliver.c +++ b/arch/renesas/src/m16c/m16c_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/renesas/src/m16c/m16c_sigdeliver.c * - * Copyright (C) 2009-2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2009-2010, 2015, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -53,18 +54,6 @@ #ifndef CONFIG_DISABLE_SIGNALS -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -99,12 +88,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc[0]; - regs[REG_PC+1] = rtcb->xcp.saved_pc[1]; - regs[REG_FLG] = rtcb->xcp.saved_flg; /* Get a local copy of the sigdeliver function pointer. We do this so * that we can nullify the sigdeliver function pointer in the TCB and @@ -134,7 +120,21 @@ void up_sigdeliver(void) sinfo("Resuming\n"); (void)up_irq_save(); - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc[0]; + regs[REG_PC+1] = rtcb->xcp.saved_pc[1]; + regs[REG_FLG] = rtcb->xcp.saved_flg; /* Then restore the correct state for this thread of * execution. diff --git a/arch/renesas/src/sh1/sh1_sigdeliver.c b/arch/renesas/src/sh1/sh1_sigdeliver.c index 50b15087ea7..25b6e5bcd12 100644 --- a/arch/renesas/src/sh1/sh1_sigdeliver.c +++ b/arch/renesas/src/sh1/sh1_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/renesas/src/common/up_sigdeliver.c * - * Copyright (C) 2008-2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2010, 2015, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -53,18 +54,6 @@ #ifndef CONFIG_DISABLE_SIGNALS -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -99,11 +88,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; - regs[REG_SR] = rtcb->xcp.saved_sr; /* Get a local copy of the sigdeliver function pointer. We do this so * that we can nullify the sigdeliver function pointer in the TCB and @@ -135,6 +122,19 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; + regs[REG_SR] = rtcb->xcp.saved_sr; + /* Then restore the correct state for this thread of execution. */ board_autoled_off(LED_SIGNAL); diff --git a/arch/risc-v/src/rv32im/up_sigdeliver.c b/arch/risc-v/src/rv32im/up_sigdeliver.c index 81ea6bf7961..7819e06e004 100644 --- a/arch/risc-v/src/rv32im/up_sigdeliver.c +++ b/arch/risc-v/src/rv32im/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/risc-v/src/rv32im/up_sigdeliver.c * - * Copyright (C) 2011, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2011, 2015, 2018-2018 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Modified for RISC-V: @@ -93,11 +93,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_EPC] = rtcb->xcp.saved_epc; - regs[REG_INT_CTX] = rtcb->xcp.saved_int_ctx; /* Get a local copy of the sigdeliver function pointer. We do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -127,7 +125,20 @@ void up_sigdeliver(void) sinfo("Resuming EPC: %08x INT_CTX: %08x\n", regs[REG_EPC], regs[REG_INT_CTX]); (void)up_irq_save(); - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_EPC] = rtcb->xcp.saved_epc; + regs[REG_INT_CTX] = rtcb->xcp.saved_int_ctx; /* Then restore the correct state for this thread of * execution. diff --git a/arch/x86/src/i486/up_sigdeliver.c b/arch/x86/src/i486/up_sigdeliver.c index c738be94bed..06099965151 100644 --- a/arch/x86/src/i486/up_sigdeliver.c +++ b/arch/x86/src/i486/up_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/x86/src/i486/up_sigdeliver.c * - * Copyright (C) 2011, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2011, 2015, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -87,11 +87,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs[REG_EIP] = rtcb->xcp.saved_eip; - regs[REG_EFLAGS] = rtcb->xcp.saved_eflags; /* Get a local copy of the sigdeliver function pointer. we do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -120,7 +118,20 @@ void up_sigdeliver(void) sinfo("Resuming\n"); (void)up_irq_save(); - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_EIP] = rtcb->xcp.saved_eip; + regs[REG_EFLAGS] = rtcb->xcp.saved_eflags; /* Then restore the correct state for this thread of execution. */ diff --git a/arch/xtensa/src/common/xtensa_sigdeliver.c b/arch/xtensa/src/common/xtensa_sigdeliver.c index ee8d024afdd..a61a68a7ed0 100644 --- a/arch/xtensa/src/common/xtensa_sigdeliver.c +++ b/arch/xtensa/src/common/xtensa_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/xtensa/src/common/arm_sigdeliver.c * - * Copyright (C) 2016, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2016, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -95,11 +95,9 @@ void xtensa_sig_deliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ xtensa_copystate(regs, rtcb->xcp.regs); - regs[REG_PC] = rtcb->xcp.saved_pc; - regs[REG_PS] = rtcb->xcp.saved_ps; /* Get a local copy of the sigdeliver function pointer. we do this so that * we can nullify the sigdeliver function pointer in the TCB and accept @@ -153,6 +151,19 @@ void xtensa_sig_deliver(void) rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[REG_PC] = rtcb->xcp.saved_pc; + regs[REG_PS] = rtcb->xcp.saved_ps; + #ifdef CONFIG_SMP /* Restore the saved 'irqcount' and recover the critical section * spinlocks. diff --git a/arch/z16/src/common/up_sigdeliver.c b/arch/z16/src/common/up_sigdeliver.c index 54f3723ce0d..44fb6f7a2bb 100644 --- a/arch/z16/src/common/up_sigdeliver.c +++ b/arch/z16/src/common/up_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/z16/src/common/up_sigdeliver.c * - * Copyright (C) 2008-2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2010, 2015, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -88,11 +89,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ up_copystate(regs, rtcb->xcp.regs); - regs32[REG_PC/2] = rtcb->xcp.saved_pc; - regs[REG_FLAGS] = rtcb->xcp.saved_i; /* Get a local copy of the sigdeliver function pointer. We do this so * that we can nullify the sigdeliver function point in the TCB and @@ -122,7 +121,20 @@ void up_sigdeliver(void) sinfo("Resuming\n"); (void)up_irq_save(); - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs32[REG_PC / 2] = rtcb->xcp.saved_pc; + regs[REG_FLAGS] = rtcb->xcp.saved_i; /* Then restore the correct state for this thread of execution. */ diff --git a/arch/z80/src/ez80/ez80_sigdeliver.c b/arch/z80/src/ez80/ez80_sigdeliver.c index 43e7de155d8..29e91d7cd86 100644 --- a/arch/z80/src/ez80/ez80_sigdeliver.c +++ b/arch/z80/src/ez80/ez80_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/z80/src/ez80/ez80_sigdeliver.c * - * Copyright (C) 2008-2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2010, 2015, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -53,18 +54,6 @@ #ifndef CONFIG_DISABLE_SIGNALS -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -99,11 +88,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ ez80_copystate(regs, rtcb->xcp.regs); - regs[XCPT_PC] = rtcb->xcp.saved_pc; - regs[XCPT_I] = rtcb->xcp.saved_i; /* Get a local copy of the sigdeliver function pointer. We do this so * that we can nullify the sigdeliver function pointer in the TCB and @@ -135,6 +122,19 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + regs[XCPT_PC] = rtcb->xcp.saved_pc; + regs[XCPT_I] = rtcb->xcp.saved_i; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + /* Then restore the correct state for this thread of * execution. */ diff --git a/arch/z80/src/z180/z180_sigdeliver.c b/arch/z80/src/z180/z180_sigdeliver.c index d1d166af4f7..f532a626880 100644 --- a/arch/z80/src/z180/z180_sigdeliver.c +++ b/arch/z80/src/z180/z180_sigdeliver.c @@ -1,7 +1,7 @@ /**************************************************************************** * arch/z80/src/z180/z180_sigdeliver.c * - * Copyright (C) 2012, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2012, 2015, 2018-2019 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -52,18 +52,6 @@ #ifndef CONFIG_DISABLE_SIGNALS -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -98,11 +86,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ z180_copystate(regs, rtcb->xcp.regs); - regs[XCPT_PC] = rtcb->xcp.saved_pc; - regs[XCPT_I] = rtcb->xcp.saved_i; /* Get a local copy of the sigdeliver function pointer. We do this so * that we can nullify the sigdeliver function pointer in the TCB and @@ -134,6 +120,19 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[XCPT_PC] = rtcb->xcp.saved_pc; + regs[XCPT_I] = rtcb->xcp.saved_i; + /* Then restore the correct state for this thread of execution. */ board_autoled_off(LED_SIGNAL); diff --git a/arch/z80/src/z8/z8_sigdeliver.c b/arch/z80/src/z8/z8_sigdeliver.c index 7ac065f48e8..d55c874d63e 100644 --- a/arch/z80/src/z8/z8_sigdeliver.c +++ b/arch/z80/src/z8/z8_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/z80/src/z8/z8_sigdeliver.c * - * Copyright (C) 2008-2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2008-2010, 2015, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -105,11 +106,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ z8_copystate(regs, rtcb->xcp.regs); - regs[XCPT_PC] = rtcb->xcp.saved_pc; - regs[XCPT_IRQCTL] = rtcb->xcp.saved_irqctl; /* Get a local copy of the sigdeliver function pointer. We do this so * that we can nullify the sigdeliver function pointer in the TCB and @@ -139,7 +138,20 @@ void up_sigdeliver(void) sinfo("Resuming\n"); (void)up_irq_save(); - rtcb->pterrno = saved_errno; + rtcb->pterrno = saved_errno; + + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[XCPT_PC] = rtcb->xcp.saved_pc; + regs[XCPT_IRQCTL] = rtcb->xcp.saved_irqctl; /* Then restore the correct state for this thread of execution. */ diff --git a/arch/z80/src/z80/z80_sigdeliver.c b/arch/z80/src/z80/z80_sigdeliver.c index 8750cecbb24..815851634dc 100644 --- a/arch/z80/src/z80/z80_sigdeliver.c +++ b/arch/z80/src/z80/z80_sigdeliver.c @@ -1,7 +1,8 @@ /**************************************************************************** * arch/z80/src/z80/z80_sigdeliver.c * - * Copyright (C) 2007-2010, 2015, 2018 Gregory Nutt. All rights reserved. + * Copyright (C) 2007-2010, 2015, 2018-2019 Gregory Nutt. All rights + * reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -52,18 +53,6 @@ #ifndef CONFIG_DISABLE_SIGNALS -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -98,11 +87,9 @@ void up_sigdeliver(void) rtcb, rtcb->xcp.sigdeliver, rtcb->sigpendactionq.head); DEBUGASSERT(rtcb->xcp.sigdeliver != NULL); - /* Save the real return state on the stack. */ + /* Save the return state on the stack. */ z80_copystate(regs, rtcb->xcp.regs); - regs[XCPT_PC] = rtcb->xcp.saved_pc; - regs[XCPT_I] = rtcb->xcp.saved_i; /* Get a local copy of the sigdeliver function pointer. We do this so * that we can nullify the sigdeliver function pointer in the TCB and @@ -134,6 +121,19 @@ void up_sigdeliver(void) (void)up_irq_save(); rtcb->pterrno = saved_errno; + /* Modify the saved return state with the actual saved values in the + * TCB. This depends on the fact that nested signal handling is + * not supported. Therefore, these values will persist throughout the + * signal handling action. + * + * Keeping this data in the TCB resolves a security problem in protected + * and kernel mode: The regs[] array is visible on the user stack and + * could be modified by a hostile program. + */ + + regs[XCPT_PC] = rtcb->xcp.saved_pc; + regs[XCPT_I] = rtcb->xcp.saved_i; + /* Then restore the correct state for this thread of execution. */ board_autoled_off(LED_SIGNAL); diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h index a91a016aae9..cac8cfa4dfc 100644 --- a/include/nuttx/sched.h +++ b/include/nuttx/sched.h @@ -150,8 +150,9 @@ # define TCB_FLAG_SCHED_SPORADIC (2 << TCB_FLAG_POLICY_SHIFT) /* Sporadic scheding policy */ # define TCB_FLAG_SCHED_OTHER (3 << TCB_FLAG_POLICY_SHIFT) /* Other scheding policy */ #define TCB_FLAG_CPU_LOCKED (1 << 7) /* Bit 7: Locked to this CPU */ -#define TCB_FLAG_EXIT_PROCESSING (1 << 8) /* Bit 8: Exitting */ - /* Bits 9-15: Available */ +#define TCB_FLAG_SIGNAL_ACTION (1 << 8) /* Bit 8: In a signal handler */ +#define TCB_FLAG_EXIT_PROCESSING (1 << 9) /* Bit 9: Exitting */ + /* Bits 10-15: Available */ /* Values for struct task_group tg_flags */ diff --git a/sched/signal/sig_deliver.c b/sched/signal/sig_deliver.c index be96de27529..1faccabe808 100644 --- a/sched/signal/sig_deliver.c +++ b/sched/signal/sig_deliver.c @@ -79,11 +79,20 @@ void nxsig_deliver(FAR struct tcb_s *stcb) for (; ; ) { - /* Remove the signal structure from the head of the sigpendactionq. */ + /* Test if this task is already handling a signal */ flags = enter_critical_section(); - sigq = (FAR sigq_t *)sq_remfirst(&stcb->sigpendactionq); + if ((stcb->flags & TCB_FLAG_SIGNAL_ACTION) != 0) + { + /* Yes.. then we must wait for the signal handler to return */ + leave_critical_section(flags); + break; + } + + /* Remove the signal structure from the head of the sigpendactionq. */ + + sigq = (FAR sigq_t *)sq_remfirst(&stcb->sigpendactionq); if (sigq == NULL) { /* All queued signal actions have been dispatched */ @@ -92,6 +101,10 @@ void nxsig_deliver(FAR struct tcb_s *stcb) break; } + /* Indicate that a signal is being delivered */ + + stcb->flags |= TCB_FLAG_SIGNAL_ACTION; + sinfo("Deliver signal %d to PID %d\n", sigq->info.si_signo, stcb->pid); @@ -173,11 +186,13 @@ void nxsig_deliver(FAR struct tcb_s *stcb) NULL); } - /* Restore the critical section, the original errno value, and - * the original sigprocmask. - */ + /* Indicate that a signal has been delivered */ flags = enter_critical_section(); + stcb->flags &= ~TCB_FLAG_SIGNAL_ACTION; + + /* Restore the original errno value and sigprocmask. */ + stcb->pterrno = saved_errno; /* What if the signal handler changed the sigprocmask? Try to retain