[PATCH v2 1/3] score: Enforce stack and TLS alignment

[Sebastian Huber]

On 02/03/2021 23:35, Kinsey Moore wrote:

> -----Original Message-----
> From: Sebastian Huber <sebastian.huber at embedded-brains.de>
> Sent: Tuesday, March 2, 2021 15:09
> To: Kinsey Moore <kinsey.moore at oarcorp.com>; devel at rtems.org
> Subject: Re: [PATCH v2 1/3] score: Enforce stack and TLS alignment
>
>
>> On 02/03/2021 20:47, Kinsey Moore wrote:
>>> Enforce alignment of the stack begin and end by allocating enough stack
>>> area that the desired size can be aligned to CPU_STACK_ALIGNMENT. This
>>> also ensures that the space reserved for TLS data falls on stack
>>> alignment boundaries which satisfies TLS alignment requirements.
>>> ---
>>>    cpukit/rtems/src/taskcreate.c       |  5 ++++-
>>>    cpukit/score/src/threadinitialize.c |  8 +++++---
>>>    cpukit/score/src/tlsallocsize.c     | 22 ++++++++++++++--------
>>>    3 files changed, 23 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/cpukit/rtems/src/taskcreate.c b/cpukit/rtems/src/taskcreate.c
>>> index c065a159c6..bad13cb532 100644
>>> --- a/cpukit/rtems/src/taskcreate.c
>>> +++ b/cpukit/rtems/src/taskcreate.c
>>> @@ -53,8 +53,11 @@ static rtems_status_code _RTEMS_tasks_Allocate_and_prepare_stack(
>>>      thread_config->stack_free = _Stack_Free;
>>>      size = _Stack_Ensure_minimum( config->storage_size );
>>>      size = _Stack_Extend_size( size, thread_config->is_fp );
>>> +  size = RTEMS_ALIGN_UP( size, CPU_STACK_ALIGNMENT );
>> This can overflow. It needs to move into _Stack_Extend_size() which
>> performs overflow handling. I think we need the following invariants
>> which should be checked by static assertions (if not already done):
>>
>> CPU_STACK_ALIGNMENT is a power of two
>>
>> CPU_HEAP_ALIGNMENT is a power of two
>>
>> CPU_STACK_ALIGNMENT >= CPU_HEAP_ALIGNMENT
>>
>> The _Stack_Extend_size() should add CPU_STACK_ALIGNMENT - 1 to the size.
> I don't see that _Stack_Extend_size() does any overflow handling. Should it? Mathematical overflow is a possibility, but I'd expect to see problems attempting the allocation long before that since it's just pulling from the workspace area of the heap.

Yes, I added this recently:

https://git.rtems.org/rtems/commit/?id=08cbd4ba201317d0f529cbdb48db9f4775804963

The problem is that if there is an overflow, then you may allocate a 
small  number.

Please have a look at this patch set:

https://lists.rtems.org/pipermail/devel/2021-March/065012.html

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/