[PATCH] Strict thread-stack isolation and thread-stack sharing.
Utkarsh Rai
utkarsh.rai60 at gmail.com
Sat Aug 15 03:28:47 UTC 2020
On Sat, Aug 15, 2020 at 12:39 AM Gedare Bloom <gedare at rtems.org> wrote:
> On Fri, Aug 14, 2020 at 9:25 AM Utkarsh Rai <utkarsh.rai60 at 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 <gedare at rtems.org> wrote:
> >>
> >> On Thu, Aug 13, 2020 at 9:06 AM Utkarsh Rai <utkarsh.rai60 at 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.
No, actually it is just a very broad assumption that if we have n threads,
each thread can have access to remaining n-1 thread stacks at max.
> >>
> >>
> >> > +#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.
>
Yes, it is for preventing that. We also have the condition that we can only
share stacks when the sharing stack is passed as a file descriptor for a
shared memory object.
>
> >>
> >> > + 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.rai60 at 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.rai60 at 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.rai60 at 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
> >> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200815/626b04d9/attachment-0001.html>
More information about the devel
mailing list