<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jan 23, 2021 at 9:29 PM Joel Sherrill <<a href="mailto:joel@rtems.org" target="_blank">joel@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"><div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jan 23, 2021, 12:28 AM Utkarsh Rai <<a href="mailto:utkarsh.rai60@gmail.com" target="_blank">utkarsh.rai60@gmail.com</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"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 7, 2020 at 8:00 AM Utkarsh Rai <<a href="mailto:utkarsh.rai60@gmail.com" rel="noreferrer" target="_blank">utkarsh.rai60@gmail.com</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"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 3, 2020 at 10:22 PM Gedare Bloom <<a href="mailto:gedare@rtems.org" rel="noreferrer" 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"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 2, 2020 at 5:53 PM Utkarsh Rai <<a href="mailto:utkarsh.rai60@gmail.com" rel="noreferrer" target="_blank">utkarsh.rai60@gmail.com</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"><div dir="ltr">Hello,<br><div>As discussed in <a href="https://lists.rtems.org/pipermail/devel/2020-November/063341.html" rel="noreferrer" target="_blank">this</a> thread, I have compiled a list of the tests that deal with inter stack communication and fail with the thread stack protection option. Most of these tests pass when, as Sebastian suggested and had provided a wonderful example, I disable memory protection at places where contents of different thread stacks are accessed by the current thread. There are a few tests that still fail due to inter-stack access in the application code itself.</div><div><br></div><div>The changes I have made are - </div><div><br></div><div>diff --git a/bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c b/bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c<br>index c176d4b8c5..a45b175395 100644<br>--- a/bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c<br>+++ b/bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c<br>@@ -1,15 +1,49 @@<br> #include <bsp/arm-cp15-start.h><br> #include <rtems/score/memoryprotection.h><br>+#include <rtems/score/threadimpl.h><br> #include <libcpu/arm-cp15.h><br> <br>+bool set_memory_flags(Thread_Control* thread, void* arg)<br>+{<br>+ uintptr_t begin;<br>+ uintptr_t end;<br>+ uint32_t flags;<br>+ rtems_interrupt_level irq_level;<br>+ Thread_Control *executing;<br>+<br>+ executing = _Thread_Executing;<br>+<br>+ if(thread != executing) {<br></div></div></blockquote><div>This is not concurrency-safe. By time the check happens, or following code happens, the thread could become executing.</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"><div dir="ltr"><div>+ <br>+ flags = *( uint32_t *)( arg );<br>+ begin = thread->Start.Initial_stack.area;<br>+ end = begin + thread->Start.Initial_stack.size; <br>+<br>+ rtems_interrupt_disable(irq_level);<br>+ arm_cp15_set_translation_table_entries(begin, end, flags);<br>+ rtems_interrupt_enable(irq_level);<br>+ }<br>+ <br>+ return false;<br></div></div></blockquote><div>why -- what does the return value mean?</div></div></div></blockquote><div><br></div><div>While iterating over threads, if the visitor returns true the iteration stops.</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"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>+}<br>+<br>+rtems_status_code _Memory_protection_Enable( void )<br>+{<br>+ uint32_t access_flags;<br>+<br>+ access_flags = translate_flags( RTEMS_NO_ACCESS );<br>+<br>+ _Thread_Iterate( set_memory_flags, &access_flags );<br>+<br>+ return RTEMS_SUCCESSFUL; // check the return values for iterating function and current method.<br>+}<br>+<br>+rtems_status_code _Memory_protection_Disable( void )<br>+{<br>+ uint32_t access_flags;<br>+<br>+ access_flags = translate_flags( RTEMS_READ_WRITE );<br>+<br>+ _Thread_Iterate( set_memory_flags, &access_flags );<br>+<br>+ return RTEMS_SUCCESSFUL;<br> }<br>\ No newline at end of file<br>diff --git a/cpukit/include/rtems/score/coremsgimpl.h b/cpukit/include/rtems/score/coremsgimpl.h<br>index e598dce96a..3719a3d3c8 100644<br>--- a/cpukit/include/rtems/score/coremsgimpl.h<br>+++ b/cpukit/include/rtems/score/coremsgimpl.h<br>@@ -27,6 +27,10 @@<br> #include <rtems/score/threaddispatch.h><br> #include <rtems/score/threadqimpl.h><br> <br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ #include <rtems/score/memoryprotection.h><br>+#endif<br>+<br> #include <limits.h><br> #include <string.h><br> <br>@@ -586,7 +590,9 @@ RTEMS_INLINE_ROUTINE Thread_Control *_CORE_message_queue_Dequeue_receiver(<br> if ( the_thread == NULL ) {<br> return NULL;<br> }<br>-<br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ _Memory_protection_Disable();<br></div></div></blockquote><div>I wonder if it is necessary to disable all protection, or can you just disable the protection applied to 'the_thread' (or maybe to 'executing')?</div></div></div></blockquote><div><br></div><div>No, it is not necessary. I will make the changes.</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"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>+#endif<br> *(size_t *) the_thread->Wait.return_argument = size;<br> the_thread->Wait.count = (uint32_t) submit_type;<br> <br>@@ -595,6 +601,9 @@ RTEMS_INLINE_ROUTINE Thread_Control *_CORE_message_queue_Dequeue_receiver(<br> the_thread->Wait.return_argument_second.mutable_object,<br> size<br> );<br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ _Memory_protection_Enable();<br>+#endif<br> <br> _Thread_queue_Extract_critical(<br> &the_message_queue->Wait_queue.Queue,<br><br>diff --git a/cpukit/posix/src/psignalunblockthread.c b/cpukit/posix/src/psignalunblockthread.c<br>index 80a0f33a09..e0f8468de6 100644<br>--- a/cpukit/posix/src/psignalunblockthread.c<br>+++ b/cpukit/posix/src/psignalunblockthread.c<br>@@ -24,6 +24,9 @@<br> #include <signal.h><br> <br> #include <rtems/score/isr.h><br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+#include <rtems/score/memoryprotection.h><br>+#endif<br> #include <rtems/score/threadimpl.h><br> #include <rtems/score/threadqimpl.h><br> #include <rtems/score/watchdogimpl.h><br>@@ -205,6 +208,10 @@ bool _POSIX_signals_Unblock_thread(<br> <br> the_info = (siginfo_t *) the_thread->Wait.return_argument;<br> <br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+_Memory_protection_Disable();<br>+#endif<br>+<br> if ( !info ) {<br> the_info->si_signo = signo;<br> the_info->si_code = SI_USER;<br>@@ -212,6 +219,9 @@ bool _POSIX_signals_Unblock_thread(<br> } else {<br> *the_info = *info;<br> }<br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+_Memory_protection_Enable();<br>+#endif<br> <br> _Thread_queue_Extract_with_proxy( the_thread );<br> return _POSIX_signals_Unblock_thread_done( the_thread, api, true );<br>diff --git a/cpukit/rtems/src/eventsurrender.c b/cpukit/rtems/src/eventsurrender.c<br>index 49f77d2663..5de62ec292 100644<br>--- a/cpukit/rtems/src/eventsurrender.c<br>+++ b/cpukit/rtems/src/eventsurrender.c<br>@@ -23,6 +23,10 @@<br> #include <rtems/score/threadimpl.h><br> #include <rtems/score/watchdogimpl.h><br> <br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ #include <rtems/score/memoryprotection.h><br>+#endif<br>+<br> static void _Event_Satisfy(<br> Thread_Control *the_thread,<br> Event_Control *event,<br>@@ -31,7 +35,13 @@ static void _Event_Satisfy(<br> )<br> {<br> event->pending_events = _Event_sets_Clear( pending_events, seized_events );<br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ _Memory_protection_Disable();<br>+#endif<br> *(rtems_event_set *) the_thread->Wait.return_argument = seized_events;<br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ _Memory_protection_Enable();<br>+#endif<br> }<br> <br> static bool _Event_Is_blocking_on_event(<br>diff --git a/cpukit/rtems/src/regionprocessqueue.c b/cpukit/rtems/src/regionprocessqueue.c<br>index 4adaf66674..29b078a38c 100644<br>--- a/cpukit/rtems/src/regionprocessqueue.c<br>+++ b/cpukit/rtems/src/regionprocessqueue.c<br>@@ -22,6 +22,10 @@<br> #include <rtems/score/status.h><br> #include <rtems/score/threadqimpl.h><br> <br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ #include <rtems/score/memoryprotection.h><br>+#endif<br>+<br> void _Region_Process_queue(<br> Region_Control *the_region<br> )<br>@@ -63,8 +67,13 @@ void _Region_Process_queue(<br> <br> if ( the_segment == NULL )<br> break;<br>-<br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ _Memory_protection_Disable();<br>+#endif <br> *(void **)the_thread->Wait.return_argument = the_segment;<br>+#if defined RTEMS_THREAD_STACK_PROTECTION<br>+ _Memory_protection_Enable();<br>+#endif<br> _Thread_queue_Extract( the_thread );<br> the_thread->Wait.return_code = STATUS_SUCCESSFUL;<br> }<br></div><div><br></div></div>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" rel="noreferrer" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a></blockquote></div></div></blockquote></div></div></blockquote><div><br></div><div><br></div><div>Hello,</div><div>Sorry for the long break but my university examinations had put me off track. A quick review of work done till now - </div><div>The tests where inter-stack access takes place were identified (the test report is attached in this thread) and the concerning mechanisms were made thread-stack protection compatible. The current method of disabling memory protection for these</div><div>regions uses _Thread_Iterate() which is bad in terms of performance, I have modified it by disabling the memory protection solely for 'the_thread'. The issue that remains is of User extension iterators, in particular of nested iterators. My idea is to disable memory protection in places during iteration where inter-stack access takes place. The problem is determining the region for which memory protection needs to be disabled. One way to get around this would be by disabling memory protection for all the stacks, in a blanket-based manner. The other possibility is to access the last blocked thread and disable memory protection just for the stack of this thread (as inter-stack access of the previously blocked thread takes place during iteration). How to get access to the queue of blocked threads? And is this method feasible?</div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Is it possible to identify the reasons that threads fail this check? I'm suspicious there are a few places where disabling checks entirely for the scope of an RTEMS method is needed. Running the stack usage report would seem to be a case for that.</div></div></blockquote><div><br></div><div> Ok, this is an area that I have not looked into.</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"><div dir="auto"><div dir="auto"><br></div><div dir="auto">But other cases might be writing method results like a message buffer on a stack would need to get access to a specific task just long enough to write the result.</div><div dir="auto"><br></div></div></blockquote><div><br></div><div>I was able to identify these regions, Sebastian had provided me with a<a href="https://lists.rtems.org/pipermail/devel/2020-November/063354.html"> nice patch </a>that fixed cases like these.</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"><div dir="auto"><div dir="auto"></div><div dir="auto">Random cases in applications can't be generally solved but RTEMS itself is a finite set of code. Identifying the reasons RTEMS needs access and handling those might eliminate most of these. Fix one case and multiple tests may pass.</div><div dir="auto"><br></div><div dir="auto">Also important to know when these cases occur in RTEMS.</div><div dir="auto"><br></div><div dir="auto">Just a thought.</div></div></blockquote><div><br></div><div>I believe the test report documents most of the important cases where inter-stack access place 'inside' the RTEMS kernel code. The case that remains to be addressed is of User extension iterators.</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"><div dir="auto"><div dir="auto"><br></div><div dir="auto">--joel</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" rel="noreferrer" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a></blockquote></div></div></div>
</blockquote></div></div>