[PATCH 1/2] taskmode.c: Ensure all error checking is done before modifying executing

[Joel Sherrill]

On Wed, Dec 6, 2017 at 12:25 AM, Sebastian Huber <
sebastian.huber at embedded-brains.de> wrote:

> On 05/12/17 17:13, Joel Sherrill wrote:
>
>> Also use single conditional expressions to simplify error checking.
>> Combined, this all resulted in a block of SMP enabled error checking.
>>
>> Updates #3000.
>> ---
>>   cpukit/rtems/src/taskmode.c | 33 +++++++++++++++++++++++++++++++--
>>   1 file changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/cpukit/rtems/src/taskmode.c b/cpukit/rtems/src/taskmode.c
>> index a345409..49648ac 100644
>> --- a/cpukit/rtems/src/taskmode.c
>> +++ b/cpukit/rtems/src/taskmode.c
>> @@ -43,6 +43,33 @@ rtems_status_code rtems_task_mode(
>>     if ( !previous_mode_set )
>>       return RTEMS_INVALID_ADDRESS;
>>   +#if defined( RTEMS_SMP )
>> +  /*
>> +   * When in SMP, you cannot disable preemption for a thread or
>> +   * alter its interrupt level. It must be fully preemptible with
>> +   * all interrupts enabled.
>> +   */
>> +  if ( rtems_configuration_is_smp_enabled() ) {
>> +    if ( mask & RTEMS_PREEMPT_MASK ) {
>> +      if ( !_Modes_Is_preempt( mode_set ) ) {
>> +        return RTEMS_NOT_IMPLEMENTED;
>> +      }
>> +    }
>> +
>> +    if ( mask & RTEMS_INTERRUPT_MASK ) {
>> +      if (_Modes_Get_interrupt_level( mode_set ) != 0 ) {
>>
>
> if ( _Modes...
>
> +        return RTEMS_NOT_IMPLEMENTED;
>> +      }
>>
>
> There should be a test case for the new else path
> _Modes_Get_interrupt_level( mode_set ) == 0.


Looking at this again, I think adding a test case is OK but we should also
given
an error anytime RTEMS_INTERRUPT_MASK is specified. A user thinking they
are changing the level to 0 (all enabled) is just as wrong as setting it to
non-zero.
The error we are catching is use of RTEMS_INTERRUPT_MASK in SMP mode.

Thanks for catching this.

--joel


>
>
> +    }
>> +  }
>> +#endif
>> +
>> +  /*
>> +   * Complete all error checking before doing any operations which
>> +   * impact the executing thread. There should be no errors returned
>> +   * past this point.
>> +   */
>> +
>>     executing     = _Thread_Get_executing();
>>     api = executing->API_Extensions[ THREAD_API_RTEMS ];
>>     asr = &api->Signal;
>> @@ -63,18 +90,18 @@ rtems_status_code rtems_task_mode(
>>      *  These are generic thread scheduling characteristics.
>>      */
>>     preempt_enabled = false;
>> +#if !defined( RTEMS_SMP )
>>     if ( mask & RTEMS_PREEMPT_MASK ) {
>> -#if defined( RTEMS_SMP )
>>       if ( rtems_configuration_is_smp_enabled() &&
>>            !_Modes_Is_preempt( mode_set ) ) {
>>         return RTEMS_NOT_IMPLEMENTED;
>>       }
>> -#endif
>>       bool is_preempt_enabled = _Modes_Is_preempt( mode_set );
>>         preempt_enabled = !executing->is_preemptible &&
>> is_preempt_enabled;
>>       executing->is_preemptible = is_preempt_enabled;
>>     }
>> +#endif
>>       if ( mask & RTEMS_TIMESLICE_MASK ) {
>>       if ( _Modes_Is_timeslice(mode_set) ) {
>> @@ -88,8 +115,10 @@ rtems_status_code rtems_task_mode(
>>     /*
>>      *  Set the new interrupt level
>>      */
>> +#if !defined( RTEMS_SMP )
>>     if ( mask & RTEMS_INTERRUPT_MASK )
>>       _Modes_Set_interrupt_level( mode_set );
>> +#endif
>>       /*
>>      *  This is specific to the RTEMS API
>>
>
> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax     : +49 89 189 47 41-09
> E-Mail  : sebastian.huber at embedded-brains.de
> PGP     : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20171206/a1ff71ee/attachment-0002.html>