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

[Gedare Bloom]

Sorry I should also clarify, PIP stands for "Priority Inheritance Protocol"

On Thu, Jan 2, 2020 at 4:33 PM Gedare Bloom <gedare at rtems.org> wrote:

> 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/2a96e5ba/attachment.html>