On Fri, Aug 14, 2020 at 9:25 AM Utkarsh Rai <utkarsh.ra...@gmail.com> wrote:
>
> Sorry for the late reply, I somehow missed the mail notification!
>
> On Thu, Aug 13, 2020 at 9:35 PM Gedare Bloom <ged...@rtems.org> wrote:
>>
>> On Thu, Aug 13, 2020 at 9:06 AM Utkarsh Rai <utkarsh.ra...@gmail.com> wrote:
>> >
>> > -This patch provides thread-stack isolation and thread-stack sharing
>> > mechanism for a POSIX thread.
>> >
>> Monster patch! think to yourself whether or not it makes sense to
>> split these up. The main constraint is that every patch in the series
>> should compile/run as they are applied.
>
>
> Maybe I can break this up into 3 separate patches, stack-isolation,
> stack-sharing, and the configuration option?
>
>>
>> > -The patches are based on sebastian's build-2 branch of sebastian's repo
>> > https://git.rtems.org/sebh
>> >
>> > -Configurations options are provided by using the new-build system. Refer
>> > to https://ftp.rtems.org/pub/rtems/people/sebh/user.pdf,
>> > chapter 7,for basic setup and
>> > https://ftp.rtems.org/pub/rtems/people/sebh/eng.pdf(chapter 9) for the
>> > internals of the build system.
>> > ---
>> > .../realview-pbx-a9/mmu/bsp-set-mmu-attr.c | 75 ++++++++++
>> > bsps/shared/start/stackalloc.c | 12 +-
>> > .../libbsp/arm/realview-pbx-a9/Makefile.am | 3 +
>> > cpukit/Makefile.am | 1 +
>> > cpukit/headers.am | 2 +
>> > cpukit/include/rtems/score/memoryprotection.h | 91 ++++++++++++
>> > cpukit/include/rtems/score/stack.h | 49 +++++++
>> > cpukit/include/rtems/score/stackprotection.h | 80 +++++++++++
>> > cpukit/include/rtems/score/thread.h | 1 +
>> > cpukit/posix/src/mmap.c | 41 +++++-
>> > cpukit/posix/src/shmwkspace.c | 46 +++++-
>> > cpukit/score/cpu/arm/cpu_asm.S | 94 ++++++++++++
>> > cpukit/score/src/stackprotection.c | 103 +++++++++++++
>> > cpukit/score/src/threadhandler.c | 6 +
>> > cpukit/score/src/threadinitialize.c | 9 ++
>> > .../arm/realview-pbx-a9/bsprealviewpbxa9.yml | 1 +
>> > spec/build/cpukit/cpuopts.yml | 2 +
>> > spec/build/cpukit/librtemscpu.yml | 3 +
>> > .../build/cpukit/optthreadstackprotection.yml | 16 +++
>> > spec/build/testsuites/samples/grp.yml | 4 +
>> > .../samples/threadstackprotection.yml | 19 +++
>> > .../testsuites/samples/threadstacksharing.yml | 19 +++
>> > testsuites/samples/Makefile.am | 16 +++
>> > testsuites/samples/configure.ac | 2 +
>> > .../samples/thread_stack_protection/init.c | 112 +++++++++++++++
>> > .../thread_stack_protection.doc | 2 +
>> > .../thread_stack_protection.scn | 20 +++
>> > .../samples/thread_stack_sharing/init.c | 136 ++++++++++++++++++
>> > 28 files changed, 959 insertions(+), 6 deletions(-)
>> > create mode 100644 bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c
>> > create mode 100644 cpukit/include/rtems/score/memoryprotection.h
>> > create mode 100644 cpukit/include/rtems/score/stackprotection.h
>> > create mode 100644 cpukit/score/src/stackprotection.c
>> > create mode 100644 spec/build/cpukit/optthreadstackprotection.yml
>> > create mode 100644 spec/build/testsuites/samples/threadstackprotection.yml
>> > create mode 100644 spec/build/testsuites/samples/threadstacksharing.yml
>> > create mode 100644 testsuites/samples/thread_stack_protection/init.c
>> > create mode 100644
>> > testsuites/samples/thread_stack_protection/thread_stack_protection.doc
>> > create mode 100644
>> > testsuites/samples/thread_stack_protection/thread_stack_protection.scn
>> > create mode 100644 testsuites/samples/thread_stack_sharing/init.c
>> >
>> > 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
>> > new file mode 100644
>> > index 0000000000..c3b469dd2a
>> > --- /dev/null
>> > +++ b/bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c
>> > @@ -0,0 +1,75 @@
>> file header block?
>>
>> > +#include <bsp/arm-cp15-start.h>
>> > +#include <rtems/score/memoryprotection.h>
>> > +#include <libcpu/arm-cp15.h>
>> > +#include <rtems.h>
>> > +
>> > +static uint32_t translate_flags(uint32_t attr_flags)
>> > +{
>> > + uint32_t flags;
>> > + uint32_t memory_attribute;
>> > +
>> > + switch (attr_flags)
>> > + {
>> > + case RTEMS_READ_WRITE:
>> > + flags = ARMV7_MMU_READ_WRITE;
>> > + break;
>> > +
>> > + case RTEMS_READ_ONLY:
>> > + flags = ARMV7_MMU_READ_ONLY;
>> > + break;
>> > +
>> > + case RTEMS_NO_ACCESS:
>> > + default:
>> > + flags = 0;
>> > + break;
>> > + }
>>
>> Does this switch statement work when the attr_flags have other fields
>> defined such as RTEMS_MEMORY_CACHED?
>>
>
> Yes, I missed that.
>
>>
>> When bit flags are not mutually exclusive it makes more sense to write as...
>>
>> if ( (attr_flags & RTEMS_READ_WRITE) == RTEMS_READ_WRITE) ) {
>> ...
>> }
>> if ( ... ) {
>> ...
>> }
>> Maybe it makes more sense to write your helper function this way?
>>
>> > +
>> > + /*
>> > + * Check for memory-cache operation
>> > + */
>> > + if( attr_flags & RTEMS_MEMORY_CACHED ) {
>> > + flags |= ARM_MMU_SECT_TEX_0 | ARM_MMU_SECT_C | ARM_MMU_SECT_B;
>> > + }
>> > +
>> > + return flags;
>> > +}
>> > +
>> > +void _Memory_protection_Set_entries(uintptr_t begin, size_t size,
>> > uint32_t flags)
>> > +{
>> > + uintptr_t end;
>> > + rtems_interrupt_level irq_level;
>> > + uint32_t access_flags;
>> > +
>> > + if(begin != NULL ) {
>> whitespace: "if ( begin ..."
>>
>> If begin is invalid, the set_entries fails. Maybe you want to return
>> an error code?
>>
>> You might want to refactor a helper to validate the memory range. That
>> will make this easier to maintain and tighten the checks.
>
>
> We can only change the memory attributes of the workspace and heap region, so
> we need to check the memory range for only these regions right?
>
For data protection that is probably true.
>>
>>
>> > + end = begin + size;
>> > + access_flags = translate_flags(flags);
>> > +
>> > + /*
>> > + * The ARM reference manual instructs to disable all the interrupts
>> > before
>> > + * setting up page table entries.
>> > + */
>> > + rtems_interrupt_disable(irq_level);
>> > + arm_cp15_set_translation_table_entries(begin, end, access_flags);
>> > + rtems_interrupt_enable(irq_level);
>> > + }
>> > +}
>> > +
>> > +void _Memory_protection_Unset_entries(uintptr_t begin, size_t size)
>> > +{
>> > + uint32_t access_flags;
>> > + uintptr_t end;
>> > + rtems_interrupt_level irq_level;
>> > +
>> > + if( begin != NULL ) {
>> > + end = begin + size;
>> > + access_flags = translate_flags( RTEMS_NO_ACCESS );
>> > +
>> > + /*
>> > + * The ARM reference manual instructs to disable all the interrupts
>> > before
>> > + * setting up page table entries.
>> > + */
>> > + rtems_interrupt_disable(irq_level);
>> > + arm_cp15_set_translation_table_entries(begin, end, access_flags);
>> > + rtems_interrupt_enable(irq_level);
>> > + }
>> > +}
>> > \ No newline at end of file
>> > diff --git a/bsps/shared/start/stackalloc.c
>> > b/bsps/shared/start/stackalloc.c
>> > index f7cf7be0f1..6ebef4d6e5 100644
>> > --- a/bsps/shared/start/stackalloc.c
>> > +++ b/bsps/shared/start/stackalloc.c
>> > @@ -44,6 +44,15 @@ void *bsp_stack_allocate(size_t size)
>> > {
>> > void *stack = NULL;
>> >
>> > +#if defined (RTEMS_THREAD_STACK_PROTECTION)
>> > +/*
>> > + * This is a temporary hack, we need to use _Heap_Allocate_aligned() but
>> > the heap
>> > + * intialization for bsp_stack_heap fails as bsp_section_stack_size is 0.
>> > See
>> typo: initialization
>>
>> > + * bsp_stack_allocate_init().
>>
>> This is an interesting problem. You may want to explore how to solve
>> this better.
>>
>> > + */
>> > + posix_memalign(&stack, 4096 , size);
>> > + _Memory_protection_Set_entries( stack, size, ( RTEMS_READ_ONLY |
>> > RTEMS_MEMORY_CACHED ) );
>> > +#else
>> > if (bsp_stack_heap.area_begin != 0) {
>> > stack = _Heap_Allocate(&bsp_stack_heap, size);
>> > }
>> > @@ -51,6 +60,7 @@ void *bsp_stack_allocate(size_t size)
>> > if (stack == NULL) {
>> > stack = _Workspace_Allocate(size);
>> > }
>> > +#endif
>> >
>> > return stack;
>> > }
>> > @@ -62,4 +72,4 @@ void bsp_stack_free(void *stack)
>> > if (!ok) {
>> > _Workspace_Free(stack);
>> > }
>> > -}
>> > +}
>> > \ No newline at end of file
>> spurious change. check the settings on your text editor. It seems to
>> be truncating the end of your files.
>>
>> > diff --git a/c/src/lib/libbsp/arm/realview-pbx-a9/Makefile.am
>> > b/c/src/lib/libbsp/arm/realview-pbx-a9/Makefile.am
>> > index d5549275be..24847c7b91 100644
>> > --- a/c/src/lib/libbsp/arm/realview-pbx-a9/Makefile.am
>> > +++ b/c/src/lib/libbsp/arm/realview-pbx-a9/Makefile.am
>> > @@ -74,6 +74,9 @@ librtemsbsp_a_SOURCES +=
>> > ../../../../../../bsps/arm/shared/clock/clock-a9mpcore.
>> > librtemsbsp_a_SOURCES +=
>> > ../../../../../../bsps/arm/shared/cache/cache-cp15.c
>> > librtemsbsp_a_SOURCES +=
>> > ../../../../../../bsps/arm/shared/cache/cache-v7ar-disable-data.S
>> >
>> > +#MMU
>> > +librtemsbsp_a_SOURCES +=
>> > ../../../../../../bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c
>> > +
>> > # Start hooks
>> > librtemsbsp_a_SOURCES +=
>> > ../../../../../../bsps/arm/realview-pbx-a9/start/bspstarthooks.c
>> >
>> > diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
>> > index 75e119ea3c..2d50d0fdce 100644
>> > --- a/cpukit/Makefile.am
>> > +++ b/cpukit/Makefile.am
>> > @@ -929,6 +929,7 @@ librtemscpu_a_SOURCES +=
>> > score/src/schedulercbsgetserverid.c
>> > librtemscpu_a_SOURCES += score/src/schedulercbssetparameters.c
>> > librtemscpu_a_SOURCES += score/src/schedulercbsreleasejob.c
>> > librtemscpu_a_SOURCES += score/src/schedulercbsunblock.c
>> > +librtemscpu_a_SOURCES += score/src/stackprotection.c
>> > librtemscpu_a_SOURCES += score/src/stackallocator.c
>> > librtemscpu_a_SOURCES += score/src/pheapallocate.c
>> > librtemscpu_a_SOURCES += score/src/pheapextend.c
>> > diff --git a/cpukit/headers.am b/cpukit/headers.am
>> > index 0bba9674cd..211e17ddf8 100644
>> > --- a/cpukit/headers.am
>> > +++ b/cpukit/headers.am
>> > @@ -352,6 +352,7 @@ include_rtems_score_HEADERS +=
>> > include/rtems/score/isr.h
>> > include_rtems_score_HEADERS += include/rtems/score/isrlevel.h
>> > include_rtems_score_HEADERS += include/rtems/score/isrlock.h
>> > include_rtems_score_HEADERS += include/rtems/score/memory.h
>> > +include_rtems_score_HEADERS += include/rtems/score/memoryprotection.h
>> > include_rtems_score_HEADERS += include/rtems/score/mpci.h
>> > include_rtems_score_HEADERS += include/rtems/score/mpciimpl.h
>> > include_rtems_score_HEADERS += include/rtems/score/mppkt.h
>> > @@ -405,6 +406,7 @@ include_rtems_score_HEADERS +=
>> > include/rtems/score/smplockstats.h
>> > include_rtems_score_HEADERS += include/rtems/score/smplockticket.h
>> > include_rtems_score_HEADERS += include/rtems/score/stack.h
>> > include_rtems_score_HEADERS += include/rtems/score/stackimpl.h
>> > +include_rtems_score_HEADERS += include/rtems/score/stackprotection.h
>> > include_rtems_score_HEADERS += include/rtems/score/states.h
>> > include_rtems_score_HEADERS += include/rtems/score/statesimpl.h
>> > include_rtems_score_HEADERS += include/rtems/score/status.h
>> > diff --git a/cpukit/include/rtems/score/memoryprotection.h
>> > b/cpukit/include/rtems/score/memoryprotection.h
>> > new file mode 100644
>> > index 0000000000..245e50313c
>> > --- /dev/null
>> > +++ b/cpukit/include/rtems/score/memoryprotection.h
>> > @@ -0,0 +1,91 @@
>> > +/* SPDX-License-Identifier: BSD-2-Clause */
>> > +
>> > +/**
>> > + * @file
>> > + *
>> > + * @ingroup RTEMSScoreMemoryprotection
>> > + *
>> > + * @brief This file provodes APIs for high-level memory protection
>> typo
>>
>> > + *
>> remove extra blank line
>>
>> > + */
>> > +
>> > +/*
>> > + * Copyright (C) 2020 Utkarsh Rai
>> > + *
>> > + * Redistribution and use in source and binary forms, with or without
>> > + * modification, are permitted provided that the following conditions
>> > + * are met:
>> > + * 1. Redistributions of source code must retain the above copyright
>> > + * notice, this list of conditions and the following disclaimer.
>> > + * 2. Redistributions in binary form must reproduce the above copyright
>> > + * notice, this list of conditions and the following disclaimer in the
>> > + * documentation and/or other materials provided with the distribution.
>> > + *
>> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> > "AS IS"
>> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>> > THE
>> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>> > PURPOSE
>> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
>> > BE
>> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>> > BUSINESS
>> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>> > THE
>> > + * POSSIBILITY OF SUCH DAMAGE.
>> > + */
>> > +
>> > +#ifndef _RTEMS_SCORE_MEMORYPROTECTION_H
>> > +#define _RTEMS_SCORE_MEMORYPROTECTION_H
>> > +
>> > +#if defined ( ASM )
>> > + #include <rtems/asm.h>
>> > +#else
>> > + #include <rtems/score/basedefs.h>
>> > +#endif
>> > +
>> > +#ifdef __cplusplus
>> > +extern "C" {
>> > +#endif
>> > +
>> > +#if !defined ( ASM )
>> > +
>> > +#define RTEMS_NO_ACCESS 0x00
>> > +#define RTEMS_READ_ONLY 0x01
>> > +#define RTEMS_WRITE_ONLY 0x02
>> > +#define RTEMS_READ_WRITE ( RTEMS_READ_ONLY | RTEMS_WRITE_ONLY )
>> > +#define RTEMS_MEMORY_CACHED 0x04
>> > +
>> > +/**
>> > + * @brief Define the memory access permission for the specified memory
>> > region
>> > + *
>> > + * @param begin_addr Beginning of the memory region
>> > + * @param size Size of the memory region
>> > + * @param flag Memory access flag
>> > + *
>> delete extra blank
>>
>> > + */
>> > +void _Memory_protection_Set_entries(
>> > + uintptr_t begin_addr,
>> > + size_t size,
>> > + uint32_t flag
>> > +);
>> > +
>> > +/**
>> > + * @brief Unset the memory access permission for the specified memory
>> > region
>> > + * This operation implicitly sets the specified memory region with
>> > 'NO_ACCESS'
>> > + * flag.
>> > + *
>> > + * @param begin_addr Begining of the memory region
>> typo: Beginning
>>
>> > + * @param size Size of the memory region
>> > + */
>> > +void _Memory_protection_Unset_entries(
>> > + uintptr_t begin_addr,
>> > + size_t size
>> > +);
>> > +
>> > +#endif /* !defined ( ASM ) */
>> > +
>> does this get included from ASM? if so, it is not actually doing anything...
>>
>> > +#ifdef __cplusplus
>> > +}
>> > +#endif
>> > +
>> > +#endif
>> > \ No newline at end of file
>> > diff --git a/cpukit/include/rtems/score/stack.h
>> > b/cpukit/include/rtems/score/stack.h
>> > index df1df74867..d561bf61c1 100644
>> > --- a/cpukit/include/rtems/score/stack.h
>> > +++ b/cpukit/include/rtems/score/stack.h
>> > @@ -47,6 +47,47 @@ extern "C" {
>> > */
>> > #define STACK_MINIMUM_SIZE CPU_STACK_MINIMUM_SIZE
>> >
>> > +/**
>> > + * The number of stacks that can be shared with a thread.
>> > + */
>> > +#define SHARED_STACK_NUMBER 8
>> > +
>> Magic number 8?
>
>
> Will it be ok to have the shared stack number equal to
> CONFIGURE_MAXIMUM_POSIX_THREADS -1?
>
If it is a limit of the hardware, then it needs to come through from
the CPU macros (score/cpu) or as a BSP Option.
>>
>>
>> > +#if defined ( RTEMS_THREAD_STACK_PROTECTION )
>> > +/**
>> > + * The following defines the attributes of a protected stack
>> > + */
>> > +typedef struct
>> > +{
>> > + /** The pointer to the page table base */
>> > + uintptr_t page_table_base;
>> PT is an attribute of a thread, not a stack. This is a little bit misplaced.
>>
>> > + /**Memory flag for the alllocated/shared stack */
>> typo: allocated
>>
>> > + uint32_t access_flags;
>> > +} Stack_Protection_attr;
>> > +
>> > +
>> > +/**
>> > + * The following defines the control block of a shared stack. Each stack
>> > can have
>> > + * different sharing attributes.
>> > + */
>> > +typedef struct
>> > +{
>> > + /** This is the attribute of a shared stack*/
>> > + Stack_Protection_attr Base;
>> > + /** This is the stack address of the sharing thread*/
>> > + void* shared_stack_area;
>> > + /** Stack size of the sharing thread*/
>> > + size_t shared_stack_size;
>> > + /** This is the stack address of the target stack. Maybe this area is
>> > not
>> > + * needed but this helps in selecting the target thread during stack
>> > sharing.
>> > + */
>> > + void* target_stack_area;
>> > + /** Error checking for valid target stack address. This is also used to
>> > + * distinguish between a normal mmap operation and a stack sharing
>> > operation.
>> > + */
>> > + bool stack_shared;
>> > +} Stack_Shared_attr;
>> > +#endif
>> > +
>> I thought you were planning to integrate this better with the existing
>> Stack_Control stuff?
>>
>> Do we really need a separate structure to track 'shared' stacks (as
>> opposed to 'non-shared' ones?)
>>
>
> This is because of the design of the stack sharing mechanism. I have
> elaborated more on this below.
>
>>
>> > /**
>> > * The following defines the control block used to manage each stack.
>> > */
>> > @@ -55,6 +96,14 @@ typedef struct {
>> > size_t size;
>> > /** This is the low memory address of stack. */
>> > void *area;
>> > +#if defined (RTEMS_THREAD_STACK_PROTECTION)
>> > + /** The attribute of a protected stack */
>> > + Stack_Protection_attr Base;
>>
>> This 'Base' stuff should only be used when the structure sits at the
>> base, to implement some kind of inheritance/polymorphism.
>>
>> > +
>> > + Stack_Shared_attr *shared_stacks[SHARED_STACK_NUMBER];
>> I'm not sure about this design choice.
>>
>
> Yes, I now realize that we cannot do it this way. But we do have to track the
> shared stacks as discussed here, and for setting memory attributes of any
> shared stack we need three properties, base address, size of the stack, and
> the memory flag for sharing which can be different from its 'intrinsic'
> memory flag. The stack control structure already has the size and the base
> address but a single thread stack can be shared with multiple threads with
> different memory flags. This is an implementation challenge I have to look
> out for.
>
I see. Maybe it can be a pointer to the Stack_Control, plus the flags?
>>
>> > +
>> > + uint32_t shared_stacks_count;
>> > +#endif
>> > } Stack_Control;
>> >
>> > /**
>> > diff --git a/cpukit/include/rtems/score/stackprotection.h
>> > b/cpukit/include/rtems/score/stackprotection.h
>> > new file mode 100644
>> > index 0000000000..58ab5f8673
>> > --- /dev/null
>> > +++ b/cpukit/include/rtems/score/stackprotection.h
>> > @@ -0,0 +1,80 @@
>> > +/**
>> > + * @file
>> > + *
>> > + * @ingroup RTEMSScoreStackprotection
>> > + *
>> > + * @brief Stackprotection API
>> > + *
>> > + * This include file provides the API for the management of protected
>> > thread-
>> > + * stacks
>> > + */
>> > +
>> > +/*
>> > + * COPYRIGHT (c) 2020 Utkarsh Rai.
>> > + *
>> > + * Redistribution and use in source and binary forms, with or without
>> > + * modification, are permitted provided that the following conditions
>> > + * are met:
>> > + * 1. Redistributions of source code must retain the above copyright
>> > + * notice, this list of conditions and the following disclaimer.
>> > + * 2. Redistributions in binary form must reproduce the above copyright
>> > + * notice, this list of conditions and the following disclaimer in the
>> > + * documentation and/or other materials provided with the distribution.
>> > + *
>> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> > "AS IS"
>> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>> > THE
>> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>> > PURPOSE
>> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
>> > BE
>> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>> > BUSINESS
>> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>> > THE
>> > + * POSSIBILITY OF SUCH DAMAGE.
>> > + *
>> > + */
>> > +
>> > +#ifndef _RTEMS_SCORE_STACKPROTECTION_H
>> > +#define _RTEMS_SCORE_STACKPROTECTION_H
>> > +
>> > +#include <rtems/score/basedefs.h>
>> > +#include <rtems/score/stack.h>
>> > +#include <rtems/score/memoryprotection.h>
>> > +
>> > +#ifdef __cplusplus
>> > +extern "C" {
>> > +#endif
>> > +
>> > +/**
>> > + * @brief Share the stack address of a given thread with the target
>> > thread.
>> > + *
>> > + * @param target_stack Stack address of the target thread
>> > + * @param sharing_stack Stack address of the sharin thread
>> > + */
>> > +
>> > +int _Stackprotection_Share_stack(
>> _Stack_protection_
>>
>> > + void *target_stack,
>> > + void *sharing_stack,
>> > + size_t size,
>> > + uint32_t memory_flag
>> > +);
>> > +
>> > +/**
>> > + * @brief Swap the restored shared stacks in the page table during
>> > context
>> > + * restoration
>> > + *
>> > + * We set the entries of the restored stack and mark all the remainig
>> > stacks as
>> typo: remaining
>>
>> > + * 'NO-ACCESS'.
>> > + *
>> > + * @param Control block of the restored stack
>> > + */
>> > +void _Stackprotection_Context_restore(
>> > + Stack_Control *heir_stack
>> > +);
>> > +
>> > +#ifdef __cplusplus
>> > +}
>> > +#endif
>> > +
>> > +#endif
>> > \ No newline at end of file
>> > diff --git a/cpukit/include/rtems/score/thread.h
>> > b/cpukit/include/rtems/score/thread.h
>> > index ee0aee5b79..0ad22feac4 100644
>> > --- a/cpukit/include/rtems/score/thread.h
>> > +++ b/cpukit/include/rtems/score/thread.h
>> > @@ -34,6 +34,7 @@
>> > #include <rtems/score/priority.h>
>> > #include <rtems/score/schedulernode.h>
>> > #include <rtems/score/stack.h>
>> > +#include <rtems/score/stackprotection.h>
>> > #include <rtems/score/states.h>
>> > #include <rtems/score/threadq.h>
>> > #include <rtems/score/timestamp.h>
>> > diff --git a/cpukit/posix/src/mmap.c b/cpukit/posix/src/mmap.c
>> > index 176c6e4fe8..6587819c0f 100644
>> > --- a/cpukit/posix/src/mmap.c
>> > +++ b/cpukit/posix/src/mmap.c
>> > @@ -28,7 +28,27 @@
>> >
>> > #include <rtems/posix/mmanimpl.h>
>> > #include <rtems/posix/shmimpl.h>
>> > +#include <rtems/score/stackprotection.h>
>> > +#include <rtems/score/memoryprotection.h>
>> >
>> > +static uint32_t mmap_flag_translate(int prot)
>> > +{
>> > + int prot_read;
>> > + int prot_write;
>> > + int memory_flag;
>> > +
>> > + prot_read = (prot_read & PROT_READ) == PROT_READ;
>> > + prot_write = (prot_write & PROT_WRITE) == PROT_WRITE;
>> > +
>> > + if(prot_read){
>> whitespace
>>
>> > + memory_flag |= ( RTEMS_READ_ONLY| RTEMS_MEMORY_CACHED );
>> > + }
>> > + if(prot_write) {
>> ws
>>
>> > + memory_flag |= ( RTEMS_READ_WRITE | RTEMS_MEMORY_CACHED );
>> > + }
>> > +
>> > + return memory_flag;
>> > +}
>> >
>> > /**
>> > * mmap chain of mappings.
>> > @@ -50,6 +70,9 @@ void *mmap(
>> > bool map_private;
>> > bool is_shared_shm;
>> > int err;
>> > + uint32_t memory_flags;
>> > + uintptr_t shared_stack_address;
>> > + int status;
>> >
>> > map_fixed = (flags & MAP_FIXED) == MAP_FIXED;
>> > map_anonymous = (flags & MAP_ANON) == MAP_ANON;
>> > @@ -67,7 +90,10 @@ void *mmap(
>> >
>> > /*
>> > * We can provide read, write and execute because the memory in RTEMS
>> > does
>> > - * not normally have protections but we cannot hide access to memory.
>> > + * not normally have protections but we cannot hide access to memory.
>> > For
>> > + * thread-stack protection we can provide no-access option, but stacks
>> > are
>> > + * implicitly isolated and it makes no sense to specify no-access
>> > option for
>> > + * already isolated stacks.
>> > */
>> > if ( prot == PROT_NONE ) {
>> > errno = ENOTSUP;
>> > @@ -292,11 +318,18 @@ void *mmap(
>> > free( mapping );
>> > return MAP_FAILED;
>> > }
>> > + /**
>> > + * We share thread-stacks only when we have a shared memory object and
>> > map
>> > + * shared flag set
>> > + */
>> > + memory_flags = mmap_flag_translate( prot );
>> > + status = _Stackprotection_Share_stack( mapping->addr, addr,
>> > len,memory_flags );
>>
>> What if someone passes an addr that is not in stack?
>>
>> Fundamentally what is the difference between sharing stacks and
>> sharing some arbitrary 'pages'?
>>
>> > + }
>> > + if(status == RTEMS_INVALID_ADDRESS ) {
>> ws
>>
>> what leads to this status?
>
>
>
> The _Stackprotection_Share_stack() takes the address of the target
> thread-stack and sharing thread-stack. Now we check all the thread stack
> addresses against the passed
> thread stack address. If we match against a valid address it returns
> RTEMS_SUCCESSFUL otherwise we return RTEMS_INVALID_ADDRESS. Although now when
> I relook at
> the implementation there is a glaring mistake, simply adding the pointer of
> the Shared_stack structure that has been initialized from a thread stack
> that we may not be able
> to access from a certain thread. I will have to change the implementation a
> bit, again :(.
>
I'm confused why it is only tracking the mapping for an invalid
address. that seems backward. is it to avoid breaking mmap() on
non-stack objects? if so, it should be commented.
>>
>> > + rtems_chain_append_unprotected( &mmap_mappings, &mapping->node );
>> > }
>> > -
>> > - rtems_chain_append_unprotected( &mmap_mappings, &mapping->node );
>> >
>> > mmap_mappings_lock_release( );
>> >
>> > return mapping->addr;
>> > -}
>> > +}
>> > \ No newline at end of file
>> > diff --git a/cpukit/posix/src/shmwkspace.c b/cpukit/posix/src/shmwkspace.c
>> > index 4fa4ec4771..e4e3c594d9 100644
>> > --- a/cpukit/posix/src/shmwkspace.c
>> > +++ b/cpukit/posix/src/shmwkspace.c
>> > @@ -16,6 +16,7 @@
>> >
>> > #include <errno.h>
>> > #include <string.h>
>> > +#include <rtems/posix/pthread.h>
>> > #include <rtems/score/wkspace.h>
>> > #include <rtems/posix/shmimpl.h>
>> >
>> > @@ -24,6 +25,49 @@ int _POSIX_Shm_Object_create_from_workspace(
>> > size_t size
>> > )
>> > {
>> > +#if defined(RTEMS_THREAD_STACK_PROTECTION)
>> > + POSIX_Shm_Control *shm;
>> > + Objects_Id id;
>> > + Objects_Name_or_id_lookup_errors err;
>> > + Thread_Control *Control;
>> > + ISR_lock_Context lock_context;
>> > + char *name;
>> > +
>> > + shm = RTEMS_CONTAINER_OF(shm_obj, POSIX_Shm_Control, shm_object);
>> > + name = shm->Object.name.name_p;
>> > + /** We assign fixed pattern of naming for thread-stacks, and treat them
>> > + * accordingly.
>> > + */
>> > + if( strncmp( name, "/taskfs/", 8) == 0 ) {
>> > + /**
>> > + * Obtain the object id of the thread and then get the thread control
>> > block
>> > + * corresponding to that id.
>> > + */
>> > + err = _Objects_Name_to_id_u32(
>> > + &_POSIX_Threads_Information.Objects,
>> > + _Objects_Build_name( name[8], name[9], name[10], name[11]),
>> > + RTEMS_LOCAL,
>> > + &id
>> > + );
>> > + Control = _Thread_Get( id, &lock_context );
>> > + if( Control != NULL ) {
>> ws
>>
>> > + shm_obj->handle = Control->Start.Initial_stack.area;
>> > + if( size != Control->Start.Initial_stack.size) {
>> ws
>>
>> > + return ENOMEM;
>> > + }
>>
>> I'm not sure this is the right place for this logic. It's not really
>> creating from the workplace in case of the tasksfs name match? should
>> this be handled before calling this function?
>>
>> > + } else {
>> > + return ENOMEM;
>> > + }
>> > + } else {
>> > + shm_obj->handle = _Workspace_Allocate( size );
>> > + if ( shm_obj->handle == NULL ) {
>> > + return ENOMEM;
>> > + }
>> > +
>> > + shm_obj->size = size;
>> > + return 0;
>> > +}
>> > +#else
>> > shm_obj->handle = _Workspace_Allocate( size );
>> > if ( shm_obj->handle == NULL ) {
>> > return ENOMEM;
>> > @@ -32,6 +76,7 @@ int _POSIX_Shm_Object_create_from_workspace(
>> > memset( shm_obj->handle, 0, size );
>> > shm_obj->size = size;
>> > return 0;
>> > +#endif
>> > }
>> >
>> > int _POSIX_Shm_Object_delete_from_workspace( POSIX_Shm_Object *shm_obj )
>> > @@ -97,4 +142,3 @@ void * _POSIX_Shm_Object_mmap_from_workspace(
>> >
>> > return (char*)shm_obj->handle + off;
>> > }
>> > -
>> stray change
>>
>> > diff --git a/cpukit/score/cpu/arm/cpu_asm.S
>> > b/cpukit/score/cpu/arm/cpu_asm.S
>> > index 66f8ba6032..8e9a4a9f88 100644
>> > --- a/cpukit/score/cpu/arm/cpu_asm.S
>> > +++ b/cpukit/score/cpu/arm/cpu_asm.S
>> > @@ -66,6 +66,73 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)
>> >
>> > str r3, [r0, #ARM_CONTEXT_CONTROL_ISR_DISPATCH_DISABLE]
>> >
>> > +#if defined ( RTEMS_THREAD_STACK_PROTECTION )
>> > +
>> > +/*
>> > + * We make a function call to set the memory attributes of the heir
>> > + * stack and unset that of the executing stack (including shared stacks ).
>> > + * This will be a simple asm code when done by switching the translation
>> > + * table base.
>> > + */
>> > +
>> > +/* Save the registers modified during function call */
>> > + mov r8, r0
>> > + mov r9, r1
>> > + mov r10, r2
>> > +/* Load the parameters for function call */
>> > + mov r0, r1
>> > + sub r0, r0, #76
>> > + ldr r1, [r0]
>> > + ldr r0, [r0, #4]
>> > + mov r2, #3
>> > + bl _Memory_protection_Set_entries
>> > +
>> > +/* Restore the saved registers */
>> > + mov r0, r8
>> > + mov r1, r9
>> > + mov r2, r10
>> > +
>> > +/* We load the stack-pointer of the heir thread pre-maturely, this is
>> > + * done as oherwise the *_unset_entries call will execute from the stack
>> > of the
>> > + * executing thread causing fatal exceptions.
>> > + */
>> > + ldr sp, [r1, #32]
>> > +/* Load the parameters of the function call */
>> > + mov r1, r0
>> > + sub r0, r0, #76
>> > + ldr r1, [r0]
>> > + ldr r0, [r0, #4]
>> > + bl _Memory_protection_Unset_entries
>> > +/*
>> > + * Restore the saved registers
>> > + */
>> > + mov r0, r8
>> > + mov r1, r9
>> > + mov r2, r10
>> > +
>> > +/* Set the memory entries of the shared stack */
>> > + sub r0, r0, #76
>> > + add r6, r0, #16
>> > +
>> > +.L_shared_stack_restore:
>> > +
>> > + ldr r1, [r6, #8]
>> > + ldr r0, [r6, #12]
>> > + bl _Memory_protection_Unset_entries
>> > + add r6, #4
>> > + add r11, #1
>> > +/* We compare with a hard-coded value, this is the number of shared stacks
>> > + * (SHARED_STACK_NUMBER)
>> > + */
>> > + cmp r11, #8
>> > + bne .L_shared_stack_restore
>> > +
>> > +/* Restore the saved registers */
>> > + mov r0, r8
>> > + mov r1, r9
>> > + mov r2, r10
>> > +#endif
>> > +
>> > #ifdef RTEMS_SMP
>> > /*
>> > * The executing thread no longer executes on this processor.
>> > Switch
>> > @@ -95,6 +162,7 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)
>> >
>> > /* Start restoring context */
>> > .L_restore:
>> > +
>> > #if !defined(RTEMS_SMP) && defined(ARM_MULTILIB_HAS_LOAD_STORE_EXCLUSIVE)
>> > clrex
>> > #endif
>> > @@ -133,7 +201,33 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)
>> > */
>> > DEFINE_FUNCTION_ARM(_CPU_Context_restore)
>> > mov r1, r0
>> > +
>> > GET_SELF_CPU_CONTROL r2
>> > +
>> > +#if defined ( RTEMS_THREAD_STACK_PROTECTION )
>> > +
>> > + /* Save the registers modified during function call */
>> > + mov r8, r0
>> > + mov r9, r1
>> > + mov r10, r2
>> > +
>> > + /* Load the parameters for function call */
>> > + mov r1, r0
>> > + sub r0, r0, #76
>> > + ldr r1, [r0]
>> > + ldr r0, [r0, #4]
>> > + mov r2, #3
>> > + bl _Memory_protection_Set_entries
>> > +
>> > + /* Restore the saved registers */
>> > + mov r0, r8
>> > + mov r1, r9
>> > + mov r2, r10
>> > +
>> > + /* Set memory entries of the shared thread stacks */
>> > +
>> > +
>> > +#endif
>> > b .L_restore
>> >
>> > #ifdef RTEMS_SMP
>> > diff --git a/cpukit/score/src/stackprotection.c
>> > b/cpukit/score/src/stackprotection.c
>> > new file mode 100644
>> > index 0000000000..8427046aa0
>> > --- /dev/null
>> > +++ b/cpukit/score/src/stackprotection.c
>> > @@ -0,0 +1,103 @@
>> > +/* SPDX-License-Identifier: BSD-2-Clause */
>> > +
>> > +/**
>> > + * @file
>> > + *
>> > + * @ingroup RTEMSScoreStackprotection
>> > + *
>> > + */
>> > +
>> > +/*
>> > + * Copyright (C) 2020 Utkarsh Rai
>> > + *
>> > + * Redistribution and use in source and binary forms, with or without
>> > + * modification, are permitted provided that the following conditions
>> > + * are met:
>> > + * 1. Redistributions of source code must retain the above copyright
>> > + * notice, this list of conditions and the following disclaimer.
>> > + * 2. Redistributions in binary form must reproduce the above copyright
>> > + * notice, this list of conditions and the following disclaimer in the
>> > + * documentation and/or other materials provided with the distribution.
>> > + *
>> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> > "AS IS"
>> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>> > THE
>> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>> > PURPOSE
>> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
>> > BE
>> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>> > BUSINESS
>> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>> > THE
>> > + * POSSIBILITY OF SUCH DAMAGE.
>> > + */
>> > +
>> > +
>> > +#ifdef HAVE_CONFIG_H
>> > +#include "config.h"
>> > +#endif
>> > +
>> > +#include <rtems/score/stackprotection.h>
>> > +#include <rtems/score/threadimpl.h>
>> > +
>> > +void _Stackprotection_Context_restore(
>> > +Stack_Control *heir_stack
>> > +)
>> > +{
>> > + void* stack_address;
>> > + size_t stack_size;
>> > + uint32_t memory_flags;
>> > + uint32_t index;
>> > +
>> > + for(index = 0;index < heir_stack->shared_stacks_count; index++) {
>> > + stack_address = heir_stack->shared_stacks[index]->shared_stack_area;
>> > + stack_size = heir_stack->shared_stacks[index]->shared_stack_size;
>> > + _Memory_protection_Set_entries( stack_address, stack_size,
>> > memory_flags );
>> > + }
>> > +
>> > +}
>> > +
>> > +static bool get_target_thread( Thread_Control *Control, void *arg)
>> > +{
>> > + Stack_Shared_attr *shared_stack;
>> > + uint32_t count;
>> > + shared_stack = arg;
>> > + /**
>> > + * Check for the target thread by comparing the stack address. Add the
>> > shared stack
>> > + * attribute structure to the array tracking all the shared stacks.
>> > + */
>> > + if( Control->Start.Initial_stack.area ==
>> > shared_stack->target_stack_area) {
>> > + count = Control->Start.Initial_stack.shared_stacks_count + 1;
>> > + if(count >= SHARED_STACK_NUMBER) {
>> > + return false;
>> > + }
>> > + Control->Start.Initial_stack.shared_stacks_count = count;
>> > + Control->Start.Initial_stack.shared_stacks[count] = shared_stack;
>> > + shared_stack->stack_shared = true;
>> > +
>> > + return true;
>> > + }
>> > +}
>> > +
>> > +int _Stackprotection_Share_stack(
>> > + void* target_stack,
>> > + void* sharing_stack,
>> > + size_t size,
>> > + uint32_t memory_flag
>> > +)
>> > +{
>> > + Thread_Control *target_thread;
>> > + Stack_Shared_attr shared_stack;
>> > +
>> > + shared_stack.Base.access_flags= memory_flag;
>> > + shared_stack.shared_stack_area = sharing_stack;
>> > + shared_stack.target_stack_area = target_stack;
>> > + shared_stack.shared_stack_size = size;
>> > +
>> > + _Thread_Iterate( get_target_thread, &shared_stack );
>> > + if( shared_stack.stack_shared == true ) {
>> > + return 0;
>> > + } else {
>> > + return -1;
>> > + }
>> > +}
>> > \ No newline at end of file
>> > diff --git a/cpukit/score/src/threadhandler.c
>> > b/cpukit/score/src/threadhandler.c
>> > index 6742b0b391..c18e33f688 100644
>> > --- a/cpukit/score/src/threadhandler.c
>> > +++ b/cpukit/score/src/threadhandler.c
>> > @@ -22,6 +22,7 @@
>> > #include <rtems/score/assert.h>
>> > #include <rtems/score/interr.h>
>> > #include <rtems/score/isrlevel.h>
>> > +#include <rtems/score/stackprotection.h>
>> > #include <rtems/score/userextimpl.h>
>> >
>> > /*
>> > @@ -94,6 +95,11 @@ void _Thread_Handler( void )
>> > level = executing->Start.isr_level;
>> > _ISR_Set_level( level );
>> >
>> > + /*
>> > + * Switch-out the the shared thread-stacks
>> delete the
>>
>> > + */
>> > + _Stackprotection_Context_restore( &executing->Start.Initial_stack );
>> > +
>> > /*
>> > * Initialize the floating point context because we do not come
>> > * through _Thread_Dispatch on our first invocation. So the normal
>> > diff --git a/cpukit/score/src/threadinitialize.c
>> > b/cpukit/score/src/threadinitialize.c
>> > index 691f56388e..96db4193bc 100644
>> > --- a/cpukit/score/src/threadinitialize.c
>> > +++ b/cpukit/score/src/threadinitialize.c
>> > @@ -114,6 +114,15 @@ bool _Thread_Initialize(
>> > stack_size
>> > );
>> >
>> > +#if defined ( RTEMS_THREAD_STACK_PROTECTION )
>> > + /**
>> > + * Initialize the protected stack attributes. Initially we initialize
>> > the
>> > + * thread with no access attributes, on context switch/restore action
>> > the
>> > + * thread stacks are assigned READ/WRITE attribute.
>> > + */
>> > + the_thread->Start.Initial_stack.Base.access_flags = RTEMS_NO_ACCESS |
>> > RTEMS_MEMORY_CACHED;
>> > +#endif
>> > +
>> > /*
>> > * Get thread queue heads
>> > */
>> > diff --git a/spec/build/bsps/arm/realview-pbx-a9/bsprealviewpbxa9.yml
>> > b/spec/build/bsps/arm/realview-pbx-a9/bsprealviewpbxa9.yml
>> > index 2721152b93..f7d2187c66 100644
>> > --- a/spec/build/bsps/arm/realview-pbx-a9/bsprealviewpbxa9.yml
>> > +++ b/spec/build/bsps/arm/realview-pbx-a9/bsprealviewpbxa9.yml
>> > @@ -57,6 +57,7 @@ links:
>> > source:
>> > - bsps/arm/realview-pbx-a9/console/console-config.c
>> > - bsps/arm/realview-pbx-a9/console/console-polled.c
>> > +- bsps/arm/realview-pbx-a9/mmu/bsp-set-mmu-attr.c
>> > - bsps/arm/realview-pbx-a9/start/bspreset.c
>> > - bsps/arm/realview-pbx-a9/start/bspstart.c
>> > - bsps/arm/realview-pbx-a9/start/bspstarthooks.c
>> > diff --git a/spec/build/cpukit/cpuopts.yml b/spec/build/cpukit/cpuopts.yml
>> > index 1902e543ca..f7641a5e50 100644
>> > --- a/spec/build/cpukit/cpuopts.yml
>> > +++ b/spec/build/cpukit/cpuopts.yml
>> > @@ -63,6 +63,8 @@ links:
>> > uid: optszoff
>> > - role: build-dependency
>> > uid: optsztime
>> > +- role: build-dependency
>> > + uid: optthreadstackprotection
>> > - role: build-dependency
>> > uid: optversion
>> > target: cpukit/include/rtems/score/cpuopts.h
>> > diff --git a/spec/build/cpukit/librtemscpu.yml
>> > b/spec/build/cpukit/librtemscpu.yml
>> > index 63a0b7dca5..9855401665 100644
>> > --- a/spec/build/cpukit/librtemscpu.yml
>> > +++ b/spec/build/cpukit/librtemscpu.yml
>> > @@ -353,6 +353,7 @@ install:
>> > - cpukit/include/rtems/score/isrlevel.h
>> > - cpukit/include/rtems/score/isrlock.h
>> > - cpukit/include/rtems/score/memory.h
>> > + - cpukit/include/rtems/score/memoryprotection.h
>> > - cpukit/include/rtems/score/mpci.h
>> > - cpukit/include/rtems/score/mpciimpl.h
>> > - cpukit/include/rtems/score/mppkt.h
>> > @@ -405,6 +406,7 @@ install:
>> > - cpukit/include/rtems/score/smplockstats.h
>> > - cpukit/include/rtems/score/smplockticket.h
>> > - cpukit/include/rtems/score/stack.h
>> > + - cpukit/include/rtems/score/stackprotection.h
>> > - cpukit/include/rtems/score/stackimpl.h
>> > - cpukit/include/rtems/score/states.h
>> > - cpukit/include/rtems/score/statesimpl.h
>> > @@ -1509,6 +1511,7 @@ source:
>> > - cpukit/score/src/semaphore.c
>> > - cpukit/score/src/smpbarrierwait.c
>> > - cpukit/score/src/stackallocator.c
>> > +- cpukit/score/src/stackprotection.c
>> > - cpukit/score/src/threadallocateunlimited.c
>> > - cpukit/score/src/thread.c
>> > - cpukit/score/src/threadchangepriority.c
>> > diff --git a/spec/build/cpukit/optthreadstackprotection.yml
>> > b/spec/build/cpukit/optthreadstackprotection.yml
>> > new file mode 100644
>> > index 0000000000..4722d9f0cb
>> > --- /dev/null
>> > +++ b/spec/build/cpukit/optthreadstackprotection.yml
>> > @@ -0,0 +1,16 @@
>> > +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
>> > +actions:
>> > +- get-boolean: null
>> > +- env-enable: null
>> > +- define-condition: null
>> > +build-type: option
>> > +copyrights:
>> > +- Copyright (C) 2020 Utkarsh Rai (utkarsh.ra...@gmail.com)
>> > +default: false
>> > +default-by-variant: []
>> > +description: |
>> > + Enable the thread stack protection support
>> > +enabled-by: true
>> > +links: []
>> > +name: RTEMS_THREAD_STACK_PROTECTION
>> > +type: build
>> > diff --git a/spec/build/testsuites/samples/grp.yml
>> > b/spec/build/testsuites/samples/grp.yml
>> > index c7591dc551..f0367b0f9b 100644
>> > --- a/spec/build/testsuites/samples/grp.yml
>> > +++ b/spec/build/testsuites/samples/grp.yml
>> > @@ -40,6 +40,10 @@ links:
>> > uid: pppd
>> > - role: build-dependency
>> > uid: ticker
>> > +- role: build-dependency
>> > + uid: threadstackprotection
>> > +- role: build-dependency
>> > + uid: threadstacksharing
>> > - role: build-dependency
>> > uid: unlimited
>> > type: build
>> > diff --git a/spec/build/testsuites/samples/threadstackprotection.yml
>> > b/spec/build/testsuites/samples/threadstackprotection.yml
>> > new file mode 100644
>> > index 0000000000..a33c53d392
>> > --- /dev/null
>> > +++ b/spec/build/testsuites/samples/threadstackprotection.yml
>> > @@ -0,0 +1,19 @@
>> > +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
>> > +build-type: test-program
>> > +cflags: []
>> > +copyrights:
>> > +- Copyright (C) 2020 Utkarsh Rai (utkarsh.ra...@gmail.com)
>> > +cppflags: []
>> > +cxxflags: []
>> > +enabled-by: true
>> > +features: c cprogram
>> > +includes: []
>> > +ldflags: []
>> > +links: []
>> > +source:
>> > +- testsuites/samples/thread_stack_protection/init.c
>> > +stlib: []
>> > +target: testsuites/samples/thread_stack_protection.exe
>> > +type: build
>> > +use-after: []
>> > +use-before: []
>> > diff --git a/spec/build/testsuites/samples/threadstacksharing.yml
>> > b/spec/build/testsuites/samples/threadstacksharing.yml
>> > new file mode 100644
>> > index 0000000000..bbe99af189
>> > --- /dev/null
>> > +++ b/spec/build/testsuites/samples/threadstacksharing.yml
>> > @@ -0,0 +1,19 @@
>> > +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
>> > +build-type: test-program
>> > +cflags: []
>> > +copyrights:
>> > +- Copyright (C) 2020 Utkarsh Rai (utkarsh.ra...@gmail.com)
>> > +cppflags: []
>> > +cxxflags: []
>> > +enabled-by: true
>> > +features: c cprogram
>> > +includes: []
>> > +ldflags: []
>> > +links: []
>> > +source:
>> > +- testsuites/samples/thread_stack_sharing/init.c
>> > +stlib: []
>> > +target: testsuites/samples/thread_stack_sharing.exe
>> > +type: build
>> > +use-after: []
>> > +use-before: []
>> > diff --git a/testsuites/samples/Makefile.am
>> > b/testsuites/samples/Makefile.am
>> > index 1944d90ccc..4d76bcb167 100644
>> > --- a/testsuites/samples/Makefile.am
>> > +++ b/testsuites/samples/Makefile.am
>> > @@ -146,6 +146,22 @@ ticker_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_FLAGS_ticker)
>> > \
>> > $(support_includes)
>> > endif
>> >
>> > +if TEST_thread_stack_protection
>> > +samples += thread_stack_protection
>> > +sample_screens += thread_stack_protection/thread_stack_protection.scn
>> > +sample_docs += thread_stack_protection/thread_stack_protection.doc
>> > +thread_stack_protection_SOURCES = thread_stack_protection/init.c
>> > +thread_stack_protection_CPPFLAGS = $(AM_CPPFLAGS)
>> > $(TEST_FLAGS_thread_stack_protection) \
>> > + $(support_includes)
>> > +endif
>> > +
>> > +if TEST_thread_stack_sharing
>> > +samples += thread_stack_sharing
>> > +thread_stack_sharing_SOURCES = thread_stack_sharing/init.c
>> > +thread_stack_sharing_CPPFLAGS = $(AM_CPPFLAGS)
>> > $(TEST_FLAGS_thread_stack_sharing) \
>> > + $(support_includes)
>> > +endif
>> > +
>> > if TEST_unlimited
>> > samples += unlimited
>> > sample_screens += unlimited/unlimited.scn
>> > diff --git a/testsuites/samples/configure.ac
>> > b/testsuites/samples/configure.ac
>> > index 9721ea4f26..6460f8f2c9 100644
>> > --- a/testsuites/samples/configure.ac
>> > +++ b/testsuites/samples/configure.ac
>> > @@ -50,6 +50,8 @@ RTEMS_TEST_CHECK([nsecs])
>> > RTEMS_TEST_CHECK([paranoia])
>> > RTEMS_TEST_CHECK([pppd])
>> > RTEMS_TEST_CHECK([ticker])
>> > +RTEMS_TEST_CHECK([thread_stack_protection])
>> > +RTEMS_TEST_CHECK([thread_stack_sharing])
>> > RTEMS_TEST_CHECK([unlimited])
>> >
>> > AC_CONFIG_FILES([Makefile])
>> > diff --git a/testsuites/samples/thread_stack_protection/init.c
>> > b/testsuites/samples/thread_stack_protection/init.c
>> > new file mode 100644
>> > index 0000000000..a9359b459d
>> > --- /dev/null
>> > +++ b/testsuites/samples/thread_stack_protection/init.c
>> > @@ -0,0 +1,112 @@
>> > +#ifdef HAVE_CONFIG_H
>> > +#include "config.h"
>> > +#endif
>> > +
>> > +#include <rtems.h>
>> > +#include <tmacros.h>
>> > +#include <pthread.h>
>> > +#include <rtems/score/memoryprotection.h>
>> > +#include <rtems/score/thread.h>
>> > +const char rtems_test_name[] = " THREAD STACK PROTECTION ";
>> > +
>> > +static void fatal_extension(
>> > + rtems_fatal_source source,
>> > + bool is_internal,
>> > + rtems_fatal_code error
>> > +)
>> > +{
>> > + if (source == RTEMS_FATAL_SOURCE_EXCEPTION)
>> > + {
>> > + printk("Internal Exception Occured \n");
>> > + }
>> > +
>> > + exit(0);
>> > +}
>> > +
>> > +void* Test_routine( void* arg )
>> > +{
>> > +
>> > +}
>> > +
>> > +void *POSIX_Init( void *argument )
>> > +{
>> > + void *stack_addr1;
>> > + void *stack_addr2;
>> > + size_t stack_size1;
>> > + size_t stack_size2;
>> > + pthread_t id1;
>> > + pthread_t id2;
>> > + pthread_attr_t attr1;
>> > + pthread_attr_t attr2;
>> > + Thread_Control Control;
>> > + TEST_BEGIN();
>> > +
>> > + /*
>> > + * We set the stack size as 8Kb.
>> > + */
>> > + stack_size1 = 8192;
>> > + stack_size2 = 8192;
>> > + printf("%d %d \n",sizeof(Control.Post_switch_actions),
>> > sizeof(Control.Start.tls_area));
>> > + /*
>> > + * We allocate page-aligned memory of the stack from the application.
>> > + */
>> > + posix_memalign(&stack_addr1, sysconf( _SC_PAGESIZE ), stack_size1 );
>> > + posix_memalign(&stack_addr2, sysconf( _SC_PAGESIZE ), stack_size2 );
>> > +
>> > + pthread_attr_init( &attr1 );
>> > + pthread_attr_init( &attr2 );
>> > +
>> > + /*
>> > + * We set the stack size and address of the thread from the application
>> > itself
>> > + */
>> > + pthread_attr_setstack( &attr1, stack_addr1, stack_size1 );
>> > + pthread_attr_setstack( &attr2, stack_addr2, stack_size2 );
>> > +
>> > + pthread_create( &id1, &attr1, Test_routine, NULL );
>> > + pthread_create( &id2, &attr2, Test_routine, NULL );
>> > + /*
>> > + * We set the memory attributes of the stack from the application.
>> > + */
>> > + _Memory_protection_Set_entries( stack_addr1, stack_size1,
>> > RTEMS_NO_ACCESS | RTEMS_MEMORY_CACHED );
>> > + _Memory_protection_Set_entries( stack_addr2, stack_size2,
>> > RTEMS_NO_ACCESS | RTEMS_MEMORY_CACHED );
>> > +
>> > + pthread_join( id1, NULL );
>> > + /*
>> > + * Write to the stack address of thread1 after it has been switched out.
>> > + */
>> > + printf("Writing to the stack of thread1 \n");
>> > + memset(stack_addr1, 0, stack_size1);
>> > +
>> > + pthread_join( id2, NULL );
>> > + /*
>> > + * Write to the stack address of thread2 after it has been switched out.
>> > + */
>> > +
>> > +
>> > +
>> > + TEST_END();
>> > + rtems_test_exit( 0 );
>> > +}
>> > +
>> > +/* configuration information */
>> > +
>> > +#define CONFIGURE_INIT
>> > +
>> > +#define CONFIGURE_APPLICATION_NEEDS_SIMPLE_CONSOLE_DRIVER
>> > +#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
>> > +
>> > +#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
>> > +
>> > +#define CONFIGURE_MAXIMUM_POSIX_THREADS 2
>> > +
>> > +#define CONFIGURE_POSIX_INIT_THREAD_TABLE
>> > +
>> > +#define CONFIGURE_INITIAL_EXTENSIONS { .fatal = fatal_extension }
>> > +
>> > +#define CONFIGURE_TASK_STACK_ALLOCATOR_INIT bsp_stack_allocate_init
>> > +#define CONFIGURE_TASK_STACK_ALLOCATOR bsp_stack_allocate
>> > +#define CONFIGURE_TASK_STACK_DEALLOCATOR bsp_stack_free
>> > +
>> > +#include <bsp/stackalloc.h>
>> > +#define CONFIGURE_INIT
>> > +#include <rtems/confdefs.h>
>> > \ No newline at end of file
>> > diff --git
>> > a/testsuites/samples/thread_stack_protection/thread_stack_protection.doc
>> > b/testsuites/samples/thread_stack_protection/thread_stack_protection.doc
>> > new file mode 100644
>> > index 0000000000..c5c9cdfa81
>> > --- /dev/null
>> > +++
>> > b/testsuites/samples/thread_stack_protection/thread_stack_protection.doc
>> > @@ -0,0 +1,2 @@
>> > +This test sample demonstrates the thread stack protection functionality.
>> > We try
>> > +to write to the stack of a switched-out thread.
>> > \ No newline at end of file
>> > diff --git
>> > a/testsuites/samples/thread_stack_protection/thread_stack_protection.scn
>> > b/testsuites/samples/thread_stack_protection/thread_stack_protection.scn
>> > new file mode 100644
>> > index 0000000000..1d36d0dfaf
>> > --- /dev/null
>> > +++
>> > b/testsuites/samples/thread_stack_protection/thread_stack_protection.scn
>> > @@ -0,0 +1,20 @@
>> > +
>> > +*** BEGIN OF TEST THREAD STACK PROTECTION ***
>> > +*** TEST VERSION: 5.0.0.a3b39c9e567f3f1ef5ab01de78b113e61b11853b-modified
>> > +*** TEST STATE: EXPECTED_PASS
>> > +*** TEST BUILD: RTEMS_NETWORKING RTEMS_POSIX_API
>> > +*** TEST TOOLS: 7.5.0 20191114 (RTEMS 5, RSB 5 (3bd11fd4898b), Newlib
>> > 7947581)
>> > +Creating thread1
>> > +Creating thread2
>> > +Joining thread1
>> > +Writing to the stack of thread1
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > +Internal Exception Occured
>> > \ No newline at end of file
>> > diff --git a/testsuites/samples/thread_stack_sharing/init.c
>> > b/testsuites/samples/thread_stack_sharing/init.c
>> > new file mode 100644
>> > index 0000000000..5bb7d01418
>> > --- /dev/null
>> > +++ b/testsuites/samples/thread_stack_sharing/init.c
>> > @@ -0,0 +1,136 @@
>> > +#ifdef HAVE_CONFIG_H
>> > +#include "config.h"
>> > +#endif
>> > +
>> > +#include <rtems.h>
>> > +#include <tmacros.h>
>> > +#include <pthread.h>
>> > +#include <sys/mman.h>
>> > +#include <sys/fcntl.h>
>> > +#include <rtems/score/memoryprotection.h>
>> > +
>> > +const char rtems_test_name[] = " THREAD STACK SHARING ";
>> > +
>> > +void* Test_routine( void* arg )
>> > +{
>> > +
>> > +}
>> > +
>> > +void *POSIX_Init( void *argument )
>> > +{
>> > + void *stack_addr1;
>> > + void *stack_addr2;
>> > + void* addr;
>> > + size_t stack_size1;
>> > + size_t stack_size2;
>> > + pthread_t id1;
>> > + pthread_t id2;
>> > + pthread_attr_t attr1;
>> > + pthread_attr_t attr2;
>> > + int fd;
>> > + char name[4] = "0x01";
>> > + char thread_name[13] = "/taskfs/0x01";
>> > +
>> > + TEST_BEGIN();
>> > +
>> > + /*
>> > + * We set the stack size as 8Kb.
>> > + */
>> > + stack_size1 = 8192;
>> > + stack_size2 = 8192;
>> > +
>> > + /*
>> > + * We allocate page-aligned memory of the stack from the application.
>> > + */
>> > + posix_memalign(&stack_addr1, sysconf( _SC_PAGESIZE ), stack_size1 );
>> > + posix_memalign(&stack_addr2, sysconf( _SC_PAGESIZE ), stack_size2 );
>> > +
>> > + pthread_attr_init( &attr1 );
>> > + pthread_attr_init( &attr2 );
>> > +
>> > + /*
>> > + * We set the stack size and address of the thread from the application
>> > itself
>> > + */
>> > + pthread_attr_setstack( &attr1, stack_addr1, stack_size1 );
>> > + pthread_attr_setstack( &attr2, stack_addr2, stack_size2 );
>> > +
>> > + pthread_create( &id1, &attr1, Test_routine, NULL );
>> > +
>> > + /*
>> > + * We set the memory attributes of the stack from the application.
>> > + */
>> > + _Memory_protection_Set_entries( stack_addr1, stack_size1,
>> > RTEMS_READ_ONLY | RTEMS_MEMORY_CACHED );
>> > +
>> > + pthread_create( &id2, &attr2, Test_routine, NULL );
>> > + _Memory_protection_Set_entries( stack_addr2, stack_size2,
>> > RTEMS_READ_ONLY | RTEMS_MEMORY_CACHED );
>> > +
>> > + /*
>> > + * Add leading "/taskfs/" to denote thread-stack name.
>> > + */
>> > + strlcat( thread_name, name, 4);
>> > +
>> > + /*
>> > + * Set the name of the thread object same as that of the shared memory
>> > object name
>> > + */
>> > + rtems_object_set_name( id1, name);
>> > +
>> > + /*
>> > + * Create a shared memory object of the stack we want to share with
>> > + * appropraite permissions. We share the stack with read and write
>> > permission
>> > + */
>> > + fd = shm_open( thread_name, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR );
>> > +
>> > + /*
>> > + * Truncate the size of the file to the size of the stack.
>> > + */
>> > + ftruncate( fd, stack_size1 );
>> > +
>> > + /*
>> > + * For sharing the stack we specify the address of the
>> > + * thread-stack we want to share with, the size of the shared stack,
>> > + * protection and access flags, file descriptor of the shared memory
>> > objcet
>> > + */
>> > + addr = mmap( stack_addr2, stack_size1, PROT_READ | PROT_WRITE, O_RDWR,
>> > fd, 0 );
>> > + rtems_test_assert( addr != NULL );
>> > +
>> > + pthread_join( id1, NULL );
>> > + /*
>> > + * Write to the stack address of thread1 after it has been switched out.
>> > + */
>> > + memset( stack_addr1, 0, stack_size1 );
>> > +
>> > + pthread_join( id2, NULL );
>> > + /*
>> > + * Write to the stack address of thread2 after it has been switched out.
>> > + */
>> > + memset( stack_addr2, 0, stack_size2 );
>> > +
>> > +
>> > + TEST_END();
>> > + rtems_test_exit( 0 );
>> > +}
>> > +
>> > +/* configuration information */
>> > +
>> > +#define CONFIGURE_INIT
>> > +
>> > +#define CONFIGURE_APPLICATION_NEEDS_SIMPLE_CONSOLE_DRIVER
>> > +#define CONFIGURE_APPLICATION_NEEDS_CLOCK_DRIVER
>> > +
>> > +#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
>> > +
>> > +#define CONFIGURE_MAXIMUM_POSIX_THREADS 4
>> > +
>> > +#define CONFIGURE_MAXIMUM_POSIX_SHMS 2
>> > +
>> > +#define CONFIGURE_LIBIO_MAXIMUM_FILE_DESCRIPTORS 10
>> > +
>> > +#define CONFIGURE_POSIX_INIT_THREAD_TABLE
>> > +
>> > +#define CONFIGURE_TASK_STACK_ALLOCATOR_INIT bsp_stack_allocate_init
>> > +#define CONFIGURE_TASK_STACK_ALLOCATOR bsp_stack_allocate
>> > +#define CONFIGURE_TASK_STACK_DEALLOCATOR bsp_stack_free
>> > +
>> > +#include <bsp/stackalloc.h>
>> > +#define CONFIGURE_INIT
>> > +#include <rtems/confdefs.h>
>> > \ No newline at end of file
>> > --
>> > 2.17.1
>> >
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
