On Thu, Dec 3, 2020 at 10:22 PM Gedare Bloom <ged...@rtems.org> wrote:
> > > On Wed, Dec 2, 2020 at 5:53 PM Utkarsh Rai <utkarsh.ra...@gmail.com> > wrote: > >> Hello, >> As discussed in this >> <https://lists.rtems.org/pipermail/devel/2020-November/063341.html> 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. >> >> The changes I have made are - >> >> 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 >> index c176d4b8c5..a45b175395 100644 >> --- a/bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c >> +++ b/bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c >> @@ -1,15 +1,49 @@ >> #include <bsp/arm-cp15-start.h> >> #include <rtems/score/memoryprotection.h> >> +#include <rtems/score/threadimpl.h> >> #include <libcpu/arm-cp15.h> >> >> +bool set_memory_flags(Thread_Control* thread, void* arg) >> +{ >> + uintptr_t begin; >> + uintptr_t end; >> + uint32_t flags; >> + rtems_interrupt_level irq_level; >> + Thread_Control *executing; >> + >> + executing = _Thread_Executing; >> + >> + if(thread != executing) { >> > This is not concurrency-safe. By time the check happens, or following code > happens, the thread could become executing. > > >> + >> + flags = *( uint32_t *)( arg ); >> + begin = thread->Start.Initial_stack.area; >> + end = begin + thread->Start.Initial_stack.size; >> + >> + rtems_interrupt_disable(irq_level); >> + arm_cp15_set_translation_table_entries(begin, end, flags); >> + rtems_interrupt_enable(irq_level); >> + } >> + >> + return false; >> > why -- what does the return value mean? > While iterating over threads, if the visitor returns true the iteration stops. > > >> +} >> + >> +rtems_status_code _Memory_protection_Enable( void ) >> +{ >> + uint32_t access_flags; >> + >> + access_flags = translate_flags( RTEMS_NO_ACCESS ); >> + >> + _Thread_Iterate( set_memory_flags, &access_flags ); >> + >> + return RTEMS_SUCCESSFUL; // check the return values for iterating >> function and current method. >> +} >> + >> +rtems_status_code _Memory_protection_Disable( void ) >> +{ >> + uint32_t access_flags; >> + >> + access_flags = translate_flags( RTEMS_READ_WRITE ); >> + >> + _Thread_Iterate( set_memory_flags, &access_flags ); >> + >> + return RTEMS_SUCCESSFUL; >> } >> \ No newline at end of file >> diff --git a/cpukit/include/rtems/score/coremsgimpl.h >> b/cpukit/include/rtems/score/coremsgimpl.h >> index e598dce96a..3719a3d3c8 100644 >> --- a/cpukit/include/rtems/score/coremsgimpl.h >> +++ b/cpukit/include/rtems/score/coremsgimpl.h >> @@ -27,6 +27,10 @@ >> #include <rtems/score/threaddispatch.h> >> #include <rtems/score/threadqimpl.h> >> >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + #include <rtems/score/memoryprotection.h> >> +#endif >> + >> #include <limits.h> >> #include <string.h> >> >> @@ -586,7 +590,9 @@ RTEMS_INLINE_ROUTINE Thread_Control >> *_CORE_message_queue_Dequeue_receiver( >> if ( the_thread == NULL ) { >> return NULL; >> } >> - >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + _Memory_protection_Disable(); >> > 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')? > No, it is not necessary. I will make the changes. > > >> +#endif >> *(size_t *) the_thread->Wait.return_argument = size; >> the_thread->Wait.count = (uint32_t) submit_type; >> >> @@ -595,6 +601,9 @@ RTEMS_INLINE_ROUTINE Thread_Control >> *_CORE_message_queue_Dequeue_receiver( >> the_thread->Wait.return_argument_second.mutable_object, >> size >> ); >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + _Memory_protection_Enable(); >> +#endif >> >> _Thread_queue_Extract_critical( >> &the_message_queue->Wait_queue.Queue, >> >> diff --git a/cpukit/posix/src/psignalunblockthread.c >> b/cpukit/posix/src/psignalunblockthread.c >> index 80a0f33a09..e0f8468de6 100644 >> --- a/cpukit/posix/src/psignalunblockthread.c >> +++ b/cpukit/posix/src/psignalunblockthread.c >> @@ -24,6 +24,9 @@ >> #include <signal.h> >> >> #include <rtems/score/isr.h> >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> +#include <rtems/score/memoryprotection.h> >> +#endif >> #include <rtems/score/threadimpl.h> >> #include <rtems/score/threadqimpl.h> >> #include <rtems/score/watchdogimpl.h> >> @@ -205,6 +208,10 @@ bool _POSIX_signals_Unblock_thread( >> >> the_info = (siginfo_t *) the_thread->Wait.return_argument; >> >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> +_Memory_protection_Disable(); >> +#endif >> + >> if ( !info ) { >> the_info->si_signo = signo; >> the_info->si_code = SI_USER; >> @@ -212,6 +219,9 @@ bool _POSIX_signals_Unblock_thread( >> } else { >> *the_info = *info; >> } >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> +_Memory_protection_Enable(); >> +#endif >> >> _Thread_queue_Extract_with_proxy( the_thread ); >> return _POSIX_signals_Unblock_thread_done( the_thread, api, true ); >> diff --git a/cpukit/rtems/src/eventsurrender.c >> b/cpukit/rtems/src/eventsurrender.c >> index 49f77d2663..5de62ec292 100644 >> --- a/cpukit/rtems/src/eventsurrender.c >> +++ b/cpukit/rtems/src/eventsurrender.c >> @@ -23,6 +23,10 @@ >> #include <rtems/score/threadimpl.h> >> #include <rtems/score/watchdogimpl.h> >> >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + #include <rtems/score/memoryprotection.h> >> +#endif >> + >> static void _Event_Satisfy( >> Thread_Control *the_thread, >> Event_Control *event, >> @@ -31,7 +35,13 @@ static void _Event_Satisfy( >> ) >> { >> event->pending_events = _Event_sets_Clear( pending_events, >> seized_events ); >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + _Memory_protection_Disable(); >> +#endif >> *(rtems_event_set *) the_thread->Wait.return_argument = seized_events; >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + _Memory_protection_Enable(); >> +#endif >> } >> >> static bool _Event_Is_blocking_on_event( >> diff --git a/cpukit/rtems/src/regionprocessqueue.c >> b/cpukit/rtems/src/regionprocessqueue.c >> index 4adaf66674..29b078a38c 100644 >> --- a/cpukit/rtems/src/regionprocessqueue.c >> +++ b/cpukit/rtems/src/regionprocessqueue.c >> @@ -22,6 +22,10 @@ >> #include <rtems/score/status.h> >> #include <rtems/score/threadqimpl.h> >> >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + #include <rtems/score/memoryprotection.h> >> +#endif >> + >> void _Region_Process_queue( >> Region_Control *the_region >> ) >> @@ -63,8 +67,13 @@ void _Region_Process_queue( >> >> if ( the_segment == NULL ) >> break; >> - >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + _Memory_protection_Disable(); >> +#endif >> *(void **)the_thread->Wait.return_argument = the_segment; >> +#if defined RTEMS_THREAD_STACK_PROTECTION >> + _Memory_protection_Enable(); >> +#endif >> _Thread_queue_Extract( the_thread ); >> the_thread->Wait.return_code = STATUS_SUCCESSFUL; >> } >> >> _______________________________________________ >> devel mailing list >> devel@rtems.org >> http://lists.rtems.org/mailman/listinfo/devel > >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
