<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 17, 2020 at 11:08 AM Gedare Bloom <<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Is this code working? how do you test it?<br>
<br>
There are several points buried below that you might need to bring out<br>
to separate threads for discussion, while you work on the code.<br>
<br>
On Thu, Jul 16, 2020 at 5:34 AM Utkarsh Rai <<a href="mailto:utkarsh.rai60@gmail.com" target="_blank">utkarsh.rai60@gmail.com</a>> wrote:<br>
><br>
> - This is the complete set of changes for strict isolation of thread stacks.<br>
> - There needs to be a confiuration operation,(#if defined(USE_THREAD_STACK_PROTECTION) for simple configuration can be used)<br>
> - The stack attributes are allocated through malloc, this needs to be done through score unlimited objects.<br>
><br>
> Mechanism for thread-stack isolation<br>
> - Whenever a stack is allocated we assign READ_WRITE memory attributes to the memory region, the stack attribute structure is appended to a chain for tracking<br>
> - On each context switch, the executing stack is marked as 'not-current' and we unset its memory attributes. The heir stack is marked as 'current'<br>
> - On context restore we set the memory attributes of the heir stack and iterate thorugh the chain to check for any other 'current' stack and unset its memory<br>
> attribute (This requires some refinement, so that we don't have to iterate over the chain).<br>
<br>
Commit formatting:<br>
<a href="https://docs.rtems.org/branches/master/eng/vc-users.html#creating-a-patch" rel="noreferrer" target="_blank">https://docs.rtems.org/branches/master/eng/vc-users.html#creating-a-patch</a><br>
points to: <a href="https://devel.rtems.org/wiki/Developer/Git#GitCommits" rel="noreferrer" target="_blank">https://devel.rtems.org/wiki/Developer/Git#GitCommits</a><br>
-- that should probably get merged to the docs.<br>
<br>
It may not say in our docs, but wrap your commit messages to 80 chars.<br>
The first line looks best at about 60 characters when it gets used as<br>
a 'summary'. The way you've written a set of bullets is more like<br>
paragraphs. You might just aim to write paragraphs. Bullets are nice<br>
when you want to format a list of related items. In that case, you<br>
should provide a list heading for some structure/organization.<br>
<br>
<br>
> ---<br>
> .../include/bsp/arm-cp15-set-ttb-entries.h | 0<br>
> bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c | 79 ++++++++<br>
> bsps/shared/start/stackalloc.c | 16 ++<br>
> c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am | 5 +-<br>
> cpukit/Makefile.am | 1 +<br>
> cpukit/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a> | 2 +<br>
> cpukit/include/rtems/score/memorymanagement.h | 89 +++++++++<br>
> cpukit/include/rtems/score/stackprotection.h | 149 ++++++++++++++<br>
> cpukit/score/cpu/arm/cpu.c | 4 +<br>
> cpukit/score/cpu/arm/cpu_asm.S | 43 +++-<br>
> .../score/cpu/arm/include/rtems/score/cpu.h | 18 ++<br>
> cpukit/score/src/stackprotection.c | 185 ++++++++++++++++++<br>
> 12 files changed, 589 insertions(+), 2 deletions(-)<br>
> create mode 100644 bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h<br>
> create mode 100644 bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c<br>
> create mode 100644 cpukit/include/rtems/score/memorymanagement.h<br>
> create mode 100644 cpukit/include/rtems/score/stackprotection.h<br>
> create mode 100644 cpukit/score/src/stackprotection.c<br>
><br>
> diff --git a/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h b/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h<br>
> new file mode 100644<br>
> index 0000000000..e69de29bb2<br>
> diff --git a/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c b/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c<br>
> new file mode 100644<br>
> index 0000000000..0c82f113a9<br>
> --- /dev/null<br>
> +++ b/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c<br>
> @@ -0,0 +1,79 @@<br>
> +#include <bsp/arm-cp15-start.h><br>
> +#include <rtems/score/memorymanagement.h><br>
> +#include <libcpu/arm-cp15.h><br>
> +#include <rtems.h><br>
> +<br>
<br>
This could use a comment why it is necessary. And also a note about<br>
converting to a configuration option<br>
> +#ifdef USE_THREAD_STACK_PROTECTION<br>
> + #define ARM_MMU_USE_SMALL_PAGES<br>
> +#endif<br>
> +<br>
<br>
Although it is not strictly required to have doxygen in the BSPs, I<br>
would like to see some function documentation of this new interface.<br>
> +void Memorymanagement_Set_entries(uintptr_t begin, size_t size, Memorymanagement_flags flags)<br>
<br>
I don't like the namespace choice. Did you discuss this with anyone?<br>
<br>
The naming rules are in<br>
<a href="https://docs.rtems.org/branches/master/eng/coding-naming.html" rel="noreferrer" target="_blank">https://docs.rtems.org/branches/master/eng/coding-naming.html</a><br>
<br>
I guess we haven't codified the BSP interface rules. You might inquire<br>
in a separate thread for some discussion. From what I recall, we have<br>
bsp_* for some generic kind of functions that should be implemented by<br>
bsp. So if this is meant to be a BSP implemented interface, you might<br>
like "bsp_memory_management" or some such. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Memory management might be an overloaded term that could encompass<br>
allocation/deallocation, sharing, protection, translation/virtual<br>
memory, maybe more. If allocation is not in scope, then some other<br>
name might be better. I guess you may be inspired by MMU. If the<br>
point of this interface is to manipulate the mmu/mpu hardware then the<br>
name should reflect that. I know we've had these discussions in the<br>
past, every time a student works on this topic--did you look for/find<br>
any prior discussion or interface descriptions?<br>
<br></blockquote><div><br></div><div>As you suggested earlier, maybe this can be modeled along the lines of the cache manager?</div><div>Maybe 'rtems_memory_protection_xx_xx' is a good naming pattern (This will be moved to cpukit/include/rtems)?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Do we really need a typedef for the flags? Are they ever more than a<br>
bit-mask? I'm just not convinced it can't be simply: uint32_t flags;<br>
<br>
> +{<br>
> +<br>
no blank line needed after {<br>
<br>
> + uintptr_t end;<br>
> + rtems_interrupt_level irq_level;<br>
> + uint32_t access_flags;<br>
> +<br>
> + end = begin + size;<br>
> + access_flags = Memorymanagement_Translate_flags(flags);<br>
<br>
Maybe it is better for users to get the flags in the right format ahead of time?<br></blockquote><div><br></div><div>Agreed, this way we won't have 'Memorymanagement_Translate_flags' in the public interface.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> + /**<br>
> + * The ARM reference manual instructs to disable all the interrupts before<br>
> + * setting up page table entries.<br>
> + */<br>
<br>
I still have questions/concerns about whether the PTEs are shared<br>
across multiple cores. If so, then interrupt disable is insufficient<br>
if this function can be called in parallel by multiple threads.<br>
<br>
Also, (why) doesn't arm_cp15_set_translation_table_entries() provide<br>
protection itself, internally, before modifying the PTEs? That may be<br>
worth a separate discussion.<br>
<br></blockquote><div> </div><div>Ok, I will enquire about this separately.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + rtems_interrupt_disable(irq_level);<br>
> + arm_cp15_set_translation_table_entries(begin, end, access_flags);<br>
> + rtems_interrupt_enable(irq_level);<br>
> +}<br>
> +<br>
> +void Memorymanagement_Unset_entries(uintptr_t begin, size_t size)<br>
> +{<br>
> + uint32_t access_flags;<br>
> + uintptr_t end;<br>
> + rtems_interrupt_level irq_level;<br>
> +<br>
> + end = begin + size;<br>
> + access_flags = Memorymanagement_Translate_flags(NO_ACCESS);<br>
white space inside function parameters, like: ...Translate_flags( NO_ACCES );<br>
<br>
> +<br>
> + /**<br>
> + * The ARM reference manual instructs to disable all the interrupts before<br>
> + * setting up page table entries.<br>
> + */<br>
> + rtems_interrupt_disable(irq_level);<br>
> + arm_cp15_set_translation_table_entries(begin, end, access_flags);<br>
> + rtems_interrupt_enable(irq_level);<br>
> +}<br>
> +<br>
> +<br>
Never need to put 2 blank lines in a row.<br>
<br>
> +uint32_t Memorymanagement_Translate_flags(Memorymanagement_flags attr_flags)<br>
<br>
Is this function meant to be callable from outside of the above two<br>
functions? If not, it can be made 'private' (static) in this file.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +{<br>
> + uint32_t flags;<br>
> + switch (attr_flags)<br>
> + {<br>
Put { on same line as the conditional expression (switch statement).<br>
<a href="https://docs.rtems.org/branches/master/eng/coding-conventions.html#formatting" rel="noreferrer" target="_blank">https://docs.rtems.org/branches/master/eng/coding-conventions.html#formatting</a><br>
"Put braces on the same line as and one space after the conditional<br>
expression ends."<br>
<br>
> + case READ_WRITE:<br>
Indent case statements within the scoped block. (For some reason, vim<br>
doesn't like to indent them correctly for you by default.) So here<br>
there should be four blank spaces since the case statement is at the<br>
second level of scope.<br>
<br>
> + flags = ARMV7_MMU_READ_WRITE;<br>
> + break;<br>
indent/align the break statement with the rest of the code in the case<br>
<br>
> +<br>
> + case READ_WRITE_CACHED:<br>
> + flags = ARMV7_MMU_DATA_READ_WRITE_CACHED;<br>
> + break;<br>
> +<br>
> + case READ_ONLY:<br>
> + flags = ARMV7_MMU_READ_ONLY;<br>
> + break;<br>
> +<br>
> + case READ_ONLY_CACHED:<br>
> + flags = ARMV7_MMU_READ_ONLY_CACHED;<br>
> + break;<br>
> +<br>
> + case NO_ACCESS:<br>
> + flags = 0;<br>
> + break;<br>
> +<br>
> + default:<br>
> + return 0;<br>
> + break;<br>
> + }<br>
> +<br>
> + return flags;<br>
> +}<br>
> \ No newline at end of file<br>
> diff --git a/bsps/shared/start/stackalloc.c b/bsps/shared/start/stackalloc.c<br>
> index f7cf7be0f1..ea8028c8db 100644<br>
> --- a/bsps/shared/start/stackalloc.c<br>
> +++ b/bsps/shared/start/stackalloc.c<br>
> @@ -25,6 +25,7 @@<br>
> #include <rtems.h><br>
> #include <rtems/score/heapimpl.h><br>
> #include <rtems/score/wkspace.h><br>
> +#include <rtems/score/stackprotection.h><br>
><br>
> #include <bsp/linker-symbols.h><br>
><br>
> @@ -43,6 +44,7 @@ void bsp_stack_allocate_init(size_t stack_space_size)<br>
> void *bsp_stack_allocate(size_t size)<br>
> {<br>
> void *stack = NULL;<br>
> + uintptr_t page_table_base;<br>
><br>
> if (bsp_stack_heap.area_begin != 0) {<br>
> stack = _Heap_Allocate(&bsp_stack_heap, size);<br>
> @@ -52,6 +54,20 @@ void *bsp_stack_allocate(size_t size)<br>
> stack = _Workspace_Allocate(size);<br>
> }<br>
><br>
> +#ifdef USE_THREAD_STACK_PROTECTION<br>
> + /**<br>
> + * Although we are not performing page table switching, still we assign a value<br>
> + * to avoid compiler warniing.<br>
> + */<br>
> + page_table_base = (uintptr_t)0x1000;<br>
There's (still) stray whitespace at the end of the line.<br>
<br>
You can suppress unused variable warnings by using this code line:<br>
(void) page_table_base;<br>
<br>
> +<br>
> + /**<br>
> + * The current way to get protected stack is to assign memory attributes<br>
> + * to the allocated memory.<br>
> + */<br>
I don't really understand this comment.<br>
<br>
> + _Stackprotection_Allocate_attr( (uintptr_t)stack, size, page_table_base );<br>
> +<br>
> +#endif<br>
> return stack;<br>
> }<br>
><br>
> diff --git a/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am b/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am<br>
> index cfd59475c2..490f99792e 100644<br>
> --- a/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am<br>
> +++ b/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am<br>
> @@ -74,6 +74,9 @@ librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/i2c/cadence-i2c.<br>
> # Cache<br>
> librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/shared/cache/cache-l2c-310.c<br>
><br>
> +#MMU<br>
> +librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c<br>
> +<br>
> # Start hooks<br>
> librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/start/bspstarthooks.c<br>
> librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/start/bspstartmmu.c<br>
> @@ -85,4 +88,4 @@ librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/start/bspstartmm<br>
><br>
> include $(srcdir)/../../../../../../bsps/shared/<a href="http://irq-sources.am" rel="noreferrer" target="_blank">irq-sources.am</a><br>
> include $(srcdir)/../../../../../../bsps/shared/<a href="http://shared-sources.am" rel="noreferrer" target="_blank">shared-sources.am</a><br>
> -include $(srcdir)/../../../../../../bsps/arm/xilinx-zynq/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a><br>
> +include $(srcdir)/../../../../../../bsps/arm/xilinx-zynq/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a><br>
> \ No newline at end of file<br>
> diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am<br>
> index 51f38c84c7..83f9bfb3ef 100644<br>
> --- a/cpukit/Makefile.am<br>
> +++ b/cpukit/Makefile.am<br>
> @@ -929,6 +929,7 @@ librtemscpu_a_SOURCES += score/src/schedulercbsgetserverid.c<br>
> librtemscpu_a_SOURCES += score/src/schedulercbssetparameters.c<br>
> librtemscpu_a_SOURCES += score/src/schedulercbsreleasejob.c<br>
> librtemscpu_a_SOURCES += score/src/schedulercbsunblock.c<br>
> +librtemscpu_a_SOURCES += score/src/stackprotection.c<br>
> librtemscpu_a_SOURCES += score/src/stackallocator.c<br>
> librtemscpu_a_SOURCES += score/src/pheapallocate.c<br>
> librtemscpu_a_SOURCES += score/src/pheapextend.c<br>
> diff --git a/cpukit/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a> b/cpukit/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a><br>
> index fcf679f09d..2f16c71d9c 100644<br>
> --- a/cpukit/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a><br>
> +++ b/cpukit/<a href="http://headers.am" rel="noreferrer" target="_blank">headers.am</a><br>
> @@ -352,6 +352,7 @@ include_rtems_score_HEADERS += include/rtems/score/isr.h<br>
> include_rtems_score_HEADERS += include/rtems/score/isrlevel.h<br>
> include_rtems_score_HEADERS += include/rtems/score/isrlock.h<br>
> include_rtems_score_HEADERS += include/rtems/score/memory.h<br>
> +include_rtems_score_HEADERS += include/rtems/score/memorymanagement.h<br>
> include_rtems_score_HEADERS += include/rtems/score/mpci.h<br>
> include_rtems_score_HEADERS += include/rtems/score/mpciimpl.h<br>
> include_rtems_score_HEADERS += include/rtems/score/mppkt.h<br>
> @@ -405,6 +406,7 @@ include_rtems_score_HEADERS += include/rtems/score/smplockstats.h<br>
> include_rtems_score_HEADERS += include/rtems/score/smplockticket.h<br>
> include_rtems_score_HEADERS += include/rtems/score/stack.h<br>
> include_rtems_score_HEADERS += include/rtems/score/stackimpl.h<br>
> +include_rtems_score_HEADERS += include/rtems/score/stackprotection.h<br>
> include_rtems_score_HEADERS += include/rtems/score/states.h<br>
> include_rtems_score_HEADERS += include/rtems/score/statesimpl.h<br>
> include_rtems_score_HEADERS += include/rtems/score/status.h<br>
> diff --git a/cpukit/include/rtems/score/memorymanagement.h b/cpukit/include/rtems/score/memorymanagement.h<br>
> new file mode 100644<br>
> index 0000000000..2a5490c680<br>
> --- /dev/null<br>
> +++ b/cpukit/include/rtems/score/memorymanagement.h<br>
> @@ -0,0 +1,89 @@<br>
> +/* SPDX-License-Identifier: BSD-2-Clause */<br>
> +<br>
> +/**<br>
> + * @file<br>
> + *<br>
> + * @ingroup RTEMSScoreMemorymanagement<br>
> + *<br>
> + * @brief This file provodes APIs for high-level memory management<br>
typo: provides<br>
<br>
> + *<br>
> + */<br>
> +<br>
> +/*<br>
> + * Copyright (C) 2020 Utkarsh Rai<br>
> + *<br>
> + * Redistribution and use in source and binary forms, with or without<br>
> + * modification, are permitted provided that the following conditions<br>
> + * are met:<br>
> + * 1. Redistributions of source code must retain the above copyright<br>
> + * notice, this list of conditions and the following disclaimer.<br>
> + * 2. Redistributions in binary form must reproduce the above copyright<br>
> + * notice, this list of conditions and the following disclaimer in the<br>
> + * documentation and/or other materials provided with the distribution.<br>
> + *<br>
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"<br>
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE<br>
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE<br>
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE<br>
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR<br>
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF<br>
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS<br>
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN<br>
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)<br>
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE<br>
> + * POSSIBILITY OF SUCH DAMAGE.<br>
> + */<br>
> +<br>
> +#ifndef _RTEMS_SCORE_MEMORYMANAGEMENT_H<br>
> +#define _RTEMS_SCORE_MEMORYMANAGEMENT_H<br>
> +<br>
> +#include <rtems/score/basedefs.h><br>
> +<br>
> +#ifdef __cplusplus<br>
> +extern "C" {<br>
> +#endif<br>
> +<br>
> +/** This defines the various high-level memory access flags.*/<br>
> +typedef enum<br>
> +{<br>
> + READ_WRITE,<br>
> + READ_WRITE_CACHED,<br>
> + READ_ONLY,<br>
> + READ_ONLY_CACHED,<br>
> + NO_ACCESS<br>
> +} Memorymanagement_flags;<br>
> +<br>
> +/**<br>
> + * @brief Define the memory access permission for the specified memory region<br>
> + *<br>
> + * @param begin_addr Begining of the memory region<br>
> + * @param size Size of the memory region<br>
> + * @param flag Memory access flag<br>
> + *<br>
> + */<br>
> +void Memorymanagement_Set_entries(uintptr_t begin_addr, size_t size, Memorymanagement_flags flag);<br>
<br>
I definitely don't like this name in the score. It needs to be<br>
reconsidered. Note that (although it is reserved) we use a leading<br>
underscore character for functions in the supercore.<br>
<br>
I might suggest _Memory_protection_Xxx_xxx.<br>
<br>
Again, these interfaces have been discussed in the past, and some<br>
prior GSoC proposals had some sketches as well.<br>
<br>
> +<br>
> +/**<br>
> + * @brief Unset the memory access permission for the specified memory region<br>
> + * This operation implcitly sets the specified memory region with 'NO_ACCESS'<br>
typo: implicitly<br>
<br>
> + * flag.<br>
> + *<br>
> + * @param begin_addr Begining of the memory region<br>
typo 'Beginning'<br>
<br>
> + * @param size Size of the memory region<br>
> + */<br>
> +void Memorymanagement_Unset_entries(uintptr_t begin_addr, size_t size);<br>
> +<br>
> +/**<br>
> + * @brief Translate the high-level flags to BSP-specifc MMU flags.<br>
typo: specific<br>
<br>
Please edit your code a bit more carefully for typos.<br>
<br>
> + *<br>
> + * @param attr_flags High-level memory access flags.<br>
> + *<br>
> + * @retval flag BSP-specifc MMU flag<br>
> + */<br>
> +uint32_t Memorymanagement_Translate_flags(Memorymanagement_flags attr_flags);<br>
> +<br>
> +#ifdef __cplusplus<br>
> +}<br>
> +#endif<br>
> +<br>
> +#endif<br>
> \ No newline at end of file<br>
> diff --git a/cpukit/include/rtems/score/stackprotection.h b/cpukit/include/rtems/score/stackprotection.h<br>
> new file mode 100644<br>
> index 0000000000..91e3f6daba<br>
> --- /dev/null<br>
> +++ b/cpukit/include/rtems/score/stackprotection.h<br>
> @@ -0,0 +1,149 @@<br>
> +/**<br>
> + * @file<br>
> + *<br>
> + * @ingroup RTEMSScoreStackprotection<br>
> + *<br>
> + * @brief Stackprotection API<br>
space<br>
<br>
> + *<br>
> + * This include file provides the API for the management of protected thread-<br>
> + * stacks<br>
> + */<br>
<br>
I'm not convinced this interface is necessary or the right way to do<br>
this. Why not just integrate directly with threads/thread stacks?<br>
<br>
Or is there a reasonable way to accomplish this by using the custom<br>
Stack Allocator, maybe in combination with a context switch hook<br>
call-out? I know we discussed this, but I can't remember the<br>
resolution. and as I look at this, it just looks a little bit overly<br>
complicated for what it is trying to get done. At least for this<br>
specific purpose of stack protection.<br>
<br></blockquote><div><br></div><div>We discussed that the way to handle the dynamic allocation of stack attributes was through score objects. I have had this in my to-do notes</div><div>and I will integrate the protected stack structure with the thread control block. Nevertheless, we need methods to manage these protected</div><div>stacks (Context switching/ restoration/ initialization, stack sharing ) this interface provides some of these methods as well as a structure to handle</div><div> the attributes of the protected/shared stacks. There are some simplifications that I have figured out in the current implementation.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +/*<br>
> + * COPYRIGHT (c) 2020 Utkarsh Rai.<br>
> + *<br>
> + * Redistribution and use in source and binary forms, with or without<br>
> + * modification, are permitted provided that the following conditions<br>
> + * are met:<br>
> + * 1. Redistributions of source code must retain the above copyright<br>
> + * notice, this list of conditions and the following disclaimer.<br>
> + * 2. Redistributions in binary form must reproduce the above copyright<br>
> + * notice, this list of conditions and the following disclaimer in the<br>
> + * documentation and/or other materials provided with the distribution.<br>
> + *<br>
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"<br>
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE<br>
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE<br>
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE<br>
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR<br>
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF<br>
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS<br>
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN<br>
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)<br>
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE<br>
> + * POSSIBILITY OF SUCH DAMAGE.<br>
> + *<br>
> + */<br>
> +<br>
> +#ifndef _RTEMS_SCORE_STACKPROTECTION_H<br>
> +#define _RTEMS_SCORE_STACKPROTECTION_H<br>
> +<br>
> +#if defined ( ASM )<br>
> +#include <rtems/asm.h><br>
> +#else<br>
> + #include <rtems/score/basedefs.h><br>
> + #include <rtems/score/memorymanagement.h><br>
> + #include <rtems/score/chainimpl.h><br>
> +#endif<br>
> +<br>
> +#ifdef __cplusplus<br>
> +extern "C" {<br>
> +#endif<br>
> +<br>
> +#if !defined ( ASM )<br>
> +<br>
> +/**<br>
> + * The following defines the attributes of a protected stack<br>
> + */<br>
> +typedef struct<br>
> +{<br>
> + /** This is the node to the chain for tracking each allocated/shared stack */<br>
> + Chain_Node node;<br>
> + /** This is the stack address */<br>
> + uintptr_t stack_address;<br>
> + /** This is the stack size */<br>
> + size_t size;<br>
> + /** This is the pointer to the page table base */<br>
> + uintptr_t page_table_base;<br>
> + /**Memory flag for the alllocated/shared stack */<br>
> + Memorymanagement_flags access_flags;<br>
> +} Stackprotection_Attr;<br>
> +<br>
> +/**<br>
> + * The following defines the control block of a shared stack<br>
> + */<br>
> +typedef struct<br>
> +{<br>
> + /** This is the attribute of a shared stack*/<br>
> + Stackprotection_Attr Base;<br>
> +} Stackprotection_Shared_stack;<br>
> +<br>
> +/**<br>
> + * The following defines the control block of an allocated stack<br>
> + */<br>
> +typedef struct<br>
> +{<br>
> + /** This is the attribute of an allocated stack*/<br>
> + Stackprotection_Attr Base;<br>
> + /** This is the pointer to the attributes of a stack shared with the stack<br>
> + * in question<br>
> + */<br>
> + Stackprotection_Shared_stack *shared_stacks;<br>
> + /**This marks if the stack in question belongs to an executing thread*/<br>
> + bool current_stack;<br>
> +} Stackprotection_The_stack;<br>
<br>
Stack_protection_Stack would be better. "_The_stack" is a bit odd.<br>
<br>
> +<br>
> +/**<br>
> + * @brief Allocate the attributes of the stack in question.<br>
> + *<br>
> + * @param freechain The stack address.<br>
freechain?<br>
<br>
> + * @param size Size of the stack.<br>
> + * @param page_table_base Pointer to the start of a page table<br>
> + */<br>
> +void _Stackprotection_Allocate_attr(uintptr_t stack_address, size_t size, uintptr_t page_table_base);<br>
> +<br>
> +/**<br>
> + * @brief Share a stack with another stack.<br>
> + *<br>
> + * @param shared_stack The stack to be shared<br>
> + * @param target_stack The stack with which to share<br>
> + */<br>
> +void _Stackprotection_Share_stack(Stackprotection_The_stack *shared_stack, Stackprotection_The_stack* target_stack);<br>
> +<br>
> +/**<br>
> + * @brief Initialize the context of a thread with the control block of a<br>
> + * protected stack<br>
> + *<br>
> + * @retval the_stack The protected stack<br>
> + */<br>
> +Stackprotection_The_stack *_Stackprotection_Context_initialize(void);<br>
> +<br>
> +/**<br>
> + * @brief Swap out the executing protected stack from the page table during<br>
> + * context switch<br>
> + *<br>
> + * The current method of switching the protected stack is to mark the switched<br>
> + * out stack as 'NO ACCESS'<br>
> + *<br>
> + * @param excuting_stack Control block of the executing stack<br>
> + */<br>
> +void _Stackprotection_Context_switch(Stackprotection_The_stack *executing_stack, Stackprotection_The_stack *heir_stack);<br>
> +<br>
> +/**<br>
> + * @brief Swap the restored protected stack in the page table during context<br>
> + * restoration<br>
> + *<br>
> + * We set the entries of the restored stack and mark all the remainig stacks as<br>
> + * 'NO-ACCESS'.<br>
> + *<br>
> + * @param Control block of the restored stack<br>
> + */<br>
> +void _Stackprotection_Context_restore(Stackprotection_The_stack *restored_stack);<br>
> +<br>
> +#endif /* !defined ( ASM ) */<br>
> +<br>
> +#ifdef __cplusplus<br>
> +}<br>
> +#endif<br>
> +<br>
> +#endif<br>
> diff --git a/cpukit/score/cpu/arm/cpu.c b/cpukit/score/cpu/arm/cpu.c<br>
> index 07b9588afd..cbb62d45e5 100644<br>
> --- a/cpukit/score/cpu/arm/cpu.c<br>
> +++ b/cpukit/score/cpu/arm/cpu.c<br>
> @@ -28,6 +28,7 @@<br>
><br>
> #include <rtems/score/assert.h><br>
> #include <rtems/score/cpu.h><br>
> +#include <rtems/score/stackprotection.h><br>
> #include <rtems/score/thread.h><br>
> #include <rtems/score/tls.h><br>
><br>
> @@ -97,6 +98,9 @@ void _CPU_Context_Initialize(<br>
> {<br>
> (void) new_level;<br>
><br>
> + #if defined (USE_THREAD_STACK_PROTECTION)<br>
> + the_context->the_stack = _Stackprotection_Context_initialize();<br>
> + #endif<br>
<br>
align #endif with the #if. And we discussed recently about whether or<br>
not to indent them. I can't remember if there was a resolution, but<br>
generally they are not indented at all.<br>
<br>
> the_context->register_sp = (uint32_t) stack_area_begin + stack_area_size;<br>
> the_context->register_lr = (uint32_t) entry_point;<br>
> the_context->isr_dispatch_disable = 0;<br>
> diff --git a/cpukit/score/cpu/arm/cpu_asm.S b/cpukit/score/cpu/arm/cpu_asm.S<br>
> index 66f8ba6032..d4d98af5b2 100644<br>
> --- a/cpukit/score/cpu/arm/cpu_asm.S<br>
> +++ b/cpukit/score/cpu/arm/cpu_asm.S<br>
> @@ -66,6 +66,27 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)<br>
><br>
> str r3, [r0, #ARM_CONTEXT_CONTROL_ISR_DISPATCH_DISABLE]<br>
><br>
> +#if defined ( USE_THREAD_STACK_PROTECTION )<br>
> + /*<br>
> + * Save the registers modifed during function call<br>
> + */<br>
> + mov r8, r0<br>
> + mov r9, r1<br>
> + mov r10, r2<br>
I already commented about some of this code before, but the space<br>
alignment here is inconsistent<br>
<br>
> + /*<br>
> + * Load the parameters to be passed<br>
> + */<br>
> + ldr r0, [r0, #ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET]<br>
> + ldr r1, [r1, #ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET]<br>
> + bl _Stackprotection_Context_switch<br>
<br>
And I'm not sure it is great to add a function call here. Especially<br>
considering that, currently, you're just switching out one active<br>
stack (executing) for another (heir), this could be done directly,<br>
albeit with more ASM written but less runtime overhead.<br>
<br>
> + /*<br>
> + * Restore the saved registers<br>
> + */<br>
> + mov r0, r8<br>
> + mov r1, r9<br>
> + mov r2, r10<br>
> +#endif<br>
> +<br>
> #ifdef RTEMS_SMP<br>
> /*<br>
> * The executing thread no longer executes on this processor. Switch<br>
> @@ -132,7 +153,27 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)<br>
> *<br>
> */<br>
> DEFINE_FUNCTION_ARM(_CPU_Context_restore)<br>
> - mov r1, r0<br>
> +<br>
> + mov r1, r0<br>
avoid spurious formatting changes to surrounding code<br>
<br>
> +#if defined( USE_THREAD_STACK_PROTECTION )<br>
> + /*<br>
> + * Save the registers modifed during function call<br>
> + */<br>
> + mov r8, r0<br>
> + mov r9, r1<br>
> + mov r10, r2<br>
> + /*<br>
> + * Load the parameters to be passed<br>
> + */<br>
> + ldr r0, [r0, #ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET]<br>
> + bl _Stackprotection_Context_switch<br>
<br>
I'm not sure why you call the same function in save as restore. Also,<br>
r1 is not set to anything?<br>
<br>
> + /*<br>
> + * Restore the saved registers<br>
> + */<br>
> + mov r0, r8<br>
> + mov r1, r9<br>
> + mov r2, r10<br>
> +#endif<br>
> GET_SELF_CPU_CONTROL r2<br>
> b .L_restore<br>
><br>
> diff --git a/cpukit/score/cpu/arm/include/rtems/score/cpu.h b/cpukit/score/cpu/arm/include/rtems/score/cpu.h<br>
> index b7b48a3ac3..df13fed258 100644<br>
> --- a/cpukit/score/cpu/arm/include/rtems/score/cpu.h<br>
> +++ b/cpukit/score/cpu/arm/include/rtems/score/cpu.h<br>
> @@ -34,6 +34,7 @@<br>
> #include <rtems/score/paravirt.h><br>
> #endif<br>
> #include <rtems/score/arm.h><br>
> +#include <rtems/score/stackprotection.h><br>
><br>
> /**<br>
> * @addtogroup RTEMSScoreCPUARM<br>
> @@ -157,6 +158,21 @@<br>
><br>
> #ifdef ARM_MULTILIB_HAS_THREAD_ID_REGISTER<br>
> #define ARM_CONTEXT_CONTROL_THREAD_ID_OFFSET 44<br>
> + #ifdef ARM_MULITLIB_VFP<br>
> + #define ARM_STACK_PROT_ATTR_OFFSET 112<br>
> + #else<br>
> + #define ARM_STACK_PROT_ATTR_OFFSET 48<br>
> + #endif<br>
> +#endif<br>
> +<br>
> +#ifdef USE_THREAD_STACK_PROTECTION<br>
> + #if defined ARM_MULITLIB_VFP<br>
> + #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 112<br>
> + #elif ARM_MULTILIB_HAS_THREAD_ID_REGISTER<br>
> + #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 48<br>
> + #else<br>
> + #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 44<br>
> + #endif<br>
> #endif<br>
><br>
> #ifdef ARM_MULTILIB_VFP<br>
> @@ -185,6 +201,7 @@<br>
><br>
> #define ARM_VFP_CONTEXT_SIZE 264<br>
><br>
> +<br>
remove stray changes<br>
<br>
> #ifndef ASM<br>
><br>
> #ifdef __cplusplus<br>
> @@ -235,6 +252,7 @@ typedef struct {<br>
> #ifdef RTEMS_SMP<br>
> volatile bool is_executing;<br>
> #endif<br>
> +Stackprotection_The_stack *the_stack;<br>
<br>
I don't see that this needs to be in the context structure. It is more<br>
logically part of the task control block. (I suppose it makes it<br>
easier for you to get the pointer, but that is just a minor<br>
convenience. You can also pass the pointer to the TCB, or offset from<br>
it.)<br>
<br>
> } Context_Control;<br>
><br>
> static inline void _ARM_Data_memory_barrier( void )<br>
> diff --git a/cpukit/score/src/stackprotection.c b/cpukit/score/src/stackprotection.c<br>
> new file mode 100644<br>
> index 0000000000..7f7c3fd6e5<br>
> --- /dev/null<br>
> +++ b/cpukit/score/src/stackprotection.c<br>
> @@ -0,0 +1,185 @@<br>
> +/* SPDX-License-Identifier: BSD-2-Clause */<br>
> +<br>
> +/**<br>
> + * @file<br>
> + *<br>
> + * @ingroup RTEMSScoreStackprotection<br>
> + *<br>
> + */<br>
> +<br>
> +/*<br>
> + * Copyright (C) 2020 Utkarsh Rai<br>
> + *<br>
> + * Redistribution and use in source and binary forms, with or without<br>
> + * modification, are permitted provided that the following conditions<br>
> + * are met:<br>
> + * 1. Redistributions of source code must retain the above copyright<br>
> + * notice, this list of conditions and the following disclaimer.<br>
> + * 2. Redistributions in binary form must reproduce the above copyright<br>
> + * notice, this list of conditions and the following disclaimer in the<br>
> + * documentation and/or other materials provided with the distribution.<br>
> + *<br>
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"<br>
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE<br>
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE<br>
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE<br>
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR<br>
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF<br>
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS<br>
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN<br>
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)<br>
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE<br>
> + * POSSIBILITY OF SUCH DAMAGE.<br>
> + */<br>
> +<br>
> +<br>
2 lines<br>
<br>
> +#ifdef HAVE_CONFIG_H<br>
> +#include "config.h"<br>
> +#endif<br>
> +<br>
> +#include <rtems/score/stackprotection.h><br>
> +<br>
> +Chain_Control _Stackprotection_node_control = CHAIN_INITIALIZER_EMPTY(_Stackprotection_node_control);<br>
> +<br>
> +static void _Stackprotection_Remove_prev_entry(void)<br>
> +{<br>
> +<br>
> + Chain_Node *node;<br>
> + Stackprotection_The_stack *prot_stack;<br>
> +<br>
> + if( _Chain_Is_empty(&_Stackprotection_node_control) == true ) {<br>
<br>
add a space after if.<br>
if ( _Chain...<br>
<br>
> + _Chain_Initialize_empty(&_Stackprotection_node_control);<br>
> + }<br>
> + node = _Chain_First( &_Stackprotection_node_control );<br>
> +<br>
> + while(_Chain_Is_tail(&_Stackprotection_node_control, node) == false) {<br>
ditto for while<br>
<br>
> +<br>
> + prot_stack = RTEMS_CONTAINER_OF(node,Stackprotection_The_stack, Base.node);<br>
> +<br>
> + if( prot_stack->current_stack == false ) {<br>
> + Memorymanagement_Unset_entries(prot_stack->Base.stack_address, prot_stack->Base.size);<br>
> + }<br>
> + node = _Chain_Immutable_next( node<br>
> + }<br>
> +<br>
<br>
I think I already complained about this design. Please discuss and<br>
justify the need for a dynamic data structure instead of a single<br>
pointer, or even an array of pointers. </blockquote><div><br></div><div>Every time a new thread is allocated we need to allocate its corresponding stack-protection control block, just like we allocate</div><div>TCB for each thread. I realize this will make more sense if we integrate the structure with the TCB.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +}<br>
> +<br>
> +/*<br>
> +Iterate to the end of the chain and mark all the 'currnet' stack as false<br>
> +Append the current stack attribute to the end of the chain<br>
> +*/<br>
> +static void _Stackprotection_Append_chain (Chain_Control *control, Stackprotection_The_stack *stack_append_attr)<br>
> +{<br>
> + Chain_Node *node;<br>
> + Stackprotection_The_stack *present_stacks_attr;<br>
> +<br>
> + if(_Chain_Is_empty(&_Stackprotection_node_control) == true ) {<br>
> +<br>
the blank line here does not help readability. Use blank lines to<br>
separate semi- or un-related chunks of code.<br>
<br>
> + _Chain_Initialize_one(&_Stackprotection_node_control, &stack_append_attr->Base.node);<br>
> + } else {<br>
> + _Chain_Append_unprotected(&_Stackprotection_node_control, &stack_append_attr->Base.node);<br>
> + }<br>
> +}<br>
> +<br>
> +void _Stackprotection_Allocate_attr(uintptr_t stack_address, size_t size, uintptr_t page_table_base)<br>
> +{<br>
> + Stackprotection_The_stack *prot_stack;<br>
> +<br>
> +/*This field will be refactored and score objects will be used for dynamic allocation*/<br>
> + prot_stack = malloc(sizeof(Stackprotection_The_stack));<br>
<br>
Or, this stuff can be included in the TCB or stack structures that<br>
already exist and are already managed by the Object Allocation. You<br>
could add the extra fields contingent on the configuration option for<br>
thread stack protection.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> + if(prot_stack != NULL) {<br>
> + prot_stack->Base.stack_address = stack_address;<br>
> + prot_stack->Base.size = size;<br>
> + prot_stack->Base.page_table_base = page_table_base;<br>
> + prot_stack->Base.access_flags = READ_WRITE_CACHED;<br>
> + prot_stack->current_stack = true;<br>
wrong indent level<br>
<br>
> + }<br>
> +<br>
> + /*<br>
> + Add the attribute field to the end of the chain, remove the memory entries of<br>
> + previously allocated stack and set the memory entry of the currnet stack.<br>
> + */<br>
> + _Stackprotection_Append_chain(&_Stackprotection_node_control, prot_stack );<br>
> + Memorymanagement_Set_entries(stack_address, size, READ_WRITE_CACHED);<br>
> +<br>
> +}<br>
> +<br>
> +Stackprotection_The_stack *_Stackprotection_Context_initialize(void)<br>
> +{<br>
> + Chain_Node *node;<br>
> + Stackprotection_The_stack *prot_stack;<br>
> +<br>
> + if( _Chain_Is_empty(&_Stackprotection_node_control) == false ) {<br>
> + node = _Chain_First( &_Stackprotection_node_control );<br>
> +<br>
> + while( _Chain_Is_tail(&_Stackprotection_node_control, node ) == false) {<br>
> + prot_stack = RTEMS_CONTAINER_OF( node, Stackprotection_The_stack, Base.node);<br>
> +<br>
> + if(prot_stack->current_stack == true) {<br>
> + return prot_stack;<br>
> + } else {<br>
> + node = _Chain_Immutable_next( node );<br>
> + }<br>
> + }<br>
> + }<br>
> +<br>
> + return prot_stack;<br>
> +}<br>
> +<br>
> +void _Stackprotection_Context_switch(Stackprotection_The_stack *executing_stack, Stackprotection_The_stack *heir_stack)<br>
> +{<br>
> + void *stack_address;<br>
> + size_t size;<br>
> + Chain_Node *node;<br>
> + Chain_Control *shared_node_control;<br>
> +<br>
> + /*<br>
> + Remove the stacks shared with the current stack by iterating the chain<br>
> + */<br>
> + if( executing_stack != NULL) {<br>
> +<br>
> + stack_address = executing_stack->Base.stack_address;<br>
> + size = executing_stack->Base.size;<br>
> +<br>
> + if(executing_stack->current_stack == true) {<br>
> + executing_stack->current_stack = false;<br>
> + Memorymanagement_Unset_entries(stack_address, size);<br>
> + }<br>
> + }<br>
> +<br>
> + _Stackprotection_Context_restore(heir_stack);<br>
> +<br>
> +}<br>
> +<br>
> +void _Stackprotection_Context_restore(Stackprotection_The_stack *heir_stack)<br>
> +{<br>
> + void *stack_address;<br>
> + size_t size;<br>
> + Chain_Node *node;<br>
> + Memorymanagement_flags flags;<br>
> + Chain_Control *shared_node_control;<br>
> + Stackprotection_The_stack *present_stacks_attr;<br>
> +<br>
> + if(heir_stack != NULL) {<br>
> + heir_stack->current_stack = true;<br>
too much indent<br>
<br>
> + stack_address = heir_stack->Base.stack_address;<br>
> + size = heir_stack->Base.size;<br>
> + flags = heir_stack->Base.access_flags;<br>
> + Memorymanagement_Set_entries(stack_address, size, flags);<br>
> + }<br>
> +<br>
> + node = _Chain_First(&_Stackprotection_node_control);<br>
> + /** Iterate through the chain and unset memory entries of all the<br>
> + * previous thread-stacks<br>
> + */<br>
> + while(_Chain_Is_tail(&_Stackprotection_node_control,node) == false) {<br>
whitespace:<br>
while ( _Chain_Is_tail( &_...,node ) == false ) {<br>
<br>
> +<br>
> + present_stacks_attr = RTEMS_CONTAINER_OF(node, Stackprotection_The_stack, Base.node);<br>
> +<br>
> + if(present_stacks_attr->current_stack == true && present_stacks_attr != heir_stack)<br>
> + present_stacks_attr->current_stack = false;<br>
> + Memorymanagement_Unset_entries(present_stacks_attr->Base.stack_address, present_stacks_attr->Base.size);<br>
> + node = _Chain_Immutable_next( node );<br>
> + }<br>
> +}<br>
> --<br>
> 2.17.1<br>
><br>
</blockquote></div></div>