[PATCH] cpukit/score: avoid NULL and races in priority mutex

[Gedare Bloom]

Hi Mathew,

Yes. I reworked the 4.10 priority inheritance mechanism in late 2018, post
4.10.2. The changes should go into 4.10.3 if that gets cut. With the 4.10
head using the reworked priority inheritance, there is a NULL pointer
access in case of using priority discipline but non-priority-inheritance
mutexes.

The 4.11 and 5.x branches reworked PIP in different ways.

Gedare

On Thu, Jan 2, 2020 at 4:27 PM Mathew Benson <mbenson at windhoverlabs.com>
wrote:

> This piqued my interest due to an issue we ran into several months ago.
> Just so I can better understand this, what is "PIP"?  Are you referring to
> maybe Priority Inversion Protection?
>
>
> On Thu, Jan 2, 2020 at 5:25 PM Gedare Bloom <gedare at rtems.org> wrote:
>
>> The PIP modifications from #3359 introduced new data structures
>> to track priority inheritance. Prioritized mutexes without PIP
>> share some of the code paths, and may result in NULL pointer
>> accesses. This patch checks for NULL, and also adds ISR critical
>> sections to an uncovered corner case during thread restarts.
>>
>> Closes #3829.
>> ---
>>  cpukit/score/src/threadqextractpriority.c | 4 +++-
>>  cpukit/score/src/threadreset.c            | 5 +++++
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/cpukit/score/src/threadqextractpriority.c
>> b/cpukit/score/src/threadqextractpriority.c
>> index 5c8188d661..9288d17980 100644
>> --- a/cpukit/score/src/threadqextractpriority.c
>> +++ b/cpukit/score/src/threadqextractpriority.c
>> @@ -109,7 +109,9 @@ bool _Thread_queue_Extract_priority_helper(
>>    }
>>
>>    mutex = _Thread_Dequeue_priority_node( &the_thread->Priority_node );
>> -  _Thread_Evaluate_priority( mutex->holder );
>> +  if ( mutex != NULL ) {
>> +    _Thread_Evaluate_priority( mutex->holder );
>> +  }
>>
>>    if ( !_Watchdog_Is_active( &the_thread->Timer ) ) {
>>      _ISR_Enable( level );
>> diff --git a/cpukit/score/src/threadreset.c
>> b/cpukit/score/src/threadreset.c
>> index 464a611391..dfc85c93aa 100644
>> --- a/cpukit/score/src/threadreset.c
>> +++ b/cpukit/score/src/threadreset.c
>> @@ -48,6 +48,7 @@ void _Thread_Reset(
>>  )
>>  {
>>    CORE_mutex_Control *mutex;
>> +  ISR_Level              level;
>>
>>    the_thread->resource_count   = 0;
>>    #if defined(RTEMS_ITRON_API)
>> @@ -66,17 +67,21 @@ void _Thread_Reset(
>>        (void) _Watchdog_Remove( &the_thread->Timer );
>>    }
>>
>> +  _ISR_Disable( level );
>>    if ( the_thread->Priority_node.waiting_to_hold != NULL ) {
>>      mutex = _Thread_Dequeue_priority_node( &the_thread->Priority_node );
>>      _Thread_Evaluate_priority( mutex->holder );
>>    }
>> +  _ISR_Enable( level );
>>
>>    while ( !_Chain_Is_empty(
>> &the_thread->Priority_node.Inherited_priorities ) ) {
>> +    _ISR_Disable( level );
>>      _Thread_Dequeue_priority_node(
>>        ((Thread_Priority_node*)_Chain_First(
>>          &the_thread->Priority_node.Inherited_priorities
>>        ))
>>      );
>> +    _ISR_Enable( level );
>>    }
>>
>>    if ( the_thread->Priority_node.current_priority !=
>> the_thread->Start.initial_priority ) {
>> --
>> 2.17.1
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>>
>
>
> --
> *Mathew Benson*
> CEO | Chief Engineer
> Windhover Labs, LLC
> 832-640-4018
>
>
> www.windhoverlabs.com
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200102/319bcead/attachment-0001.html>