Report on failing tests with thread stack protection and their resolution.
Utkarsh Rai
utkarsh.rai60 at gmail.com
Mon Dec 7 02:30:00 UTC 2020
On Thu, Dec 3, 2020 at 10:22 PM Gedare Bloom <gedare at rtems.org> wrote:
>
>
> On Wed, Dec 2, 2020 at 5:53 PM Utkarsh Rai <utkarsh.rai60 at 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 at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20201207/eef56e51/attachment-0001.html>
More information about the devel
mailing list