[PATCH] Strict thread-stack isolation

Gedare Bloom gedare at rtems.org
Tue Jul 14 14:06:40 UTC 2020


I won't comment on the namespace and coding conventions, other than to
say you should focus on doing them correctly as you code, rather than
going back and fixing them later. More below.

On Mon, Jul 13, 2020 at 10:34 AM Utkarsh Rai <utkarsh.rai60 at gmail.com> wrote:
>
> - This is the complete set of changes for strict isolation of thread stacks.
> - There needs to be a confiuration operation,(#if defined(USE_THREAD_STACK_PROTECTION) for simple configuration can be used)
> - The stack attributes are allocated through malloc, this needs to be done through score unlimited objects.
> ---
>  bsps/arm/headers.am                           |   1 +
>  .../include/bsp/arm-cp15-set-ttb-entries.h    |   7 +
>  .../shared/cp15/arm-cp15-set-ttb-entries.c    |   3 +
>  bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c   |  72 +++++++++
>  bsps/shared/start/stackalloc.c                |  20 ++-
>  c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am  |   5 +-
>  cpukit/Makefile.am                            |   1 +
>  cpukit/headers.am                             |   2 +
>  cpukit/include/rtems/score/memorymanagement.h |  22 +++
>  cpukit/include/rtems/score/stackmanagement.h  |  49 ++++++
>  cpukit/score/cpu/arm/cpu.c                    |   3 +
>  cpukit/score/cpu/arm/cpu_asm.S                |  22 ++-
>  .../score/cpu/arm/include/rtems/score/cpu.h   |  20 +++
>  cpukit/score/src/stackmanagement.c            | 143 ++++++++++++++++++
>  14 files changed, 365 insertions(+), 5 deletions(-)
>  create mode 100644 bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
>  create mode 100644 bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
>  create mode 100644 cpukit/include/rtems/score/memorymanagement.h
>  create mode 100644 cpukit/include/rtems/score/stackmanagement.h
>  create mode 100644 cpukit/score/src/stackmanagement.c
>
> diff --git a/bsps/arm/headers.am b/bsps/arm/headers.am
> index 3d2b09effa..b1e86f3385 100644
> --- a/bsps/arm/headers.am
> +++ b/bsps/arm/headers.am
> @@ -15,6 +15,7 @@ include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-a9mpcore-clock.h
>  include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-a9mpcore-irq.h
>  include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-a9mpcore-regs.h
>  include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-a9mpcore-start.h
> +include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
>  include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-cp15-start.h
>  include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-errata.h
>  include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-gic-irq.h
> diff --git a/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h b/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
> new file mode 100644
> index 0000000000..39170927da
> --- /dev/null
> +++ b/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
> @@ -0,0 +1,7 @@
> +#include<bsp/arm-cp15-start.h>
> +
> +uint32_t arm_cp15_set_translation_table_entries(
> +  const void *begin,
> +  const void *end,
> +  uint32_t section_flags
> +);
> \ No newline at end of file
> diff --git a/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c b/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> index 507277dca1..f5d0494167 100644
> --- a/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> +++ b/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> @@ -14,6 +14,7 @@
>
>  #include <rtems.h>
>  #include <libcpu/arm-cp15.h>
> +#include <bsp/arm-cp15-set-ttb-entries.h>
>  #include <bspopts.h>
>
>  /*
> @@ -30,6 +31,8 @@
>   * ARM DDI 0406C.b (ID072512)
>   */
>
> +#define ARM_MMU_USE_SMALL_PAGES
> +
>  static uint32_t set_translation_table_entries(
>    const void *begin,
>    const void *end,
> diff --git a/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c b/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> new file mode 100644
> index 0000000000..978e35b86c
> --- /dev/null
> +++ b/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> @@ -0,0 +1,72 @@
> +#include <bsp/arm-cp15-start.h>
> +#include <bsp/arm-cp15-set-ttb-entries.h>
> +#include <rtems/score/memorymanagement.h>
> +#include <libcpu/arm-cp15.h>
> +#include <rtems.h>
> +
> +#ifdef USE_THREAD_STACK_PROTECTION
> +  #define ARM_MMU_USE_SMALL_PAGES
> +#endif
> +
> +void memory_entries_set(uintptr_t begin, size_t size, memory_flags flags)
> +{
> +
> +    uintptr_t end;
> +    rtems_interrupt_level irq_level;
> +    uint32_t access_flags;
> +
> +    end = begin + size;
> +    access_flags = memory_translate_flags(flags);
> +
> +    rtems_interrupt_local_disable(irq_level);

is a local isr critical section sufficient to protect changing the ttb?

> +    arm_cp15_set_translation_table_entries(begin, end, access_flags);
> +    rtems_interrupt_local_enable(irq_level);
> +}
> +
> +void memory_entries_unset(uintptr_t begin, size_t size)
> +{
> +  uint32_t access_flags;
> +  uintptr_t end;
> +  rtems_interrupt_level irq_level;
> +
> +  end = begin + size;
> +  access_flags = memory_translate_flags(NO_ACCESS);
> +
> +  rtems_interrupt_local_disable(irq_level);
> +  arm_cp15_set_translation_table_entries(begin, end, access_flags);
> +  rtems_interrupt_local_enable(irq_level);
> +}
> +
> +
> +uint32_t memory_translate_flags(memory_flags attr_flags)

This helper function could be static (and at the top of the file).

> +{
> +  uint32_t flags;
> +  switch (attr_flags)
> +  {
> +  case READ_WRITE:
> +     flags = ARMV7_MMU_READ_WRITE;
> +  break;
> +
> +  case READ_WRITE_CACHED:
> +    flags = ARMV7_MMU_DATA_READ_WRITE_CACHED;
> +  break;
> +
> +  case READ_ONLY:
> +    flags = ARMV7_MMU_READ_ONLY;
> +  break;
> +
> +  case READ_ONLY_CACHED:
> +    flags = ARMV7_MMU_READ_ONLY_CACHED;
> +  break;
> +
> +  case NO_ACCESS:
> +    flags = 0;
> +  break;
> +
> +  default:
> +     return -1;

It's not great to return a negative value in an unsigned return.
Especially considering you're not checking them.

The "safe default" to use when modifying protection state is default-deny.

> +  break;
> +  }
> +
> +  return flags;
> +}
> \ No newline at end of file
> diff --git a/bsps/shared/start/stackalloc.c b/bsps/shared/start/stackalloc.c
> index f7cf7be0f1..2ce0d6573b 100644
> --- a/bsps/shared/start/stackalloc.c
> +++ b/bsps/shared/start/stackalloc.c
> @@ -25,6 +25,7 @@
>  #include <rtems.h>
>  #include <rtems/score/heapimpl.h>
>  #include <rtems/score/wkspace.h>
> +#include <rtems/score/stackmanagement.h>
>
>  #include <bsp/linker-symbols.h>
>
> @@ -42,7 +43,8 @@ void bsp_stack_allocate_init(size_t stack_space_size)
>
>  void *bsp_stack_allocate(size_t size)
>  {
> -  void *stack = NULL;
> + void *stack = NULL;
> + uintptr_t page_table_base;
These should be two spaces indented

>
>    if (bsp_stack_heap.area_begin != 0) {
>      stack = _Heap_Allocate(&bsp_stack_heap, size);
> @@ -51,11 +53,23 @@ void *bsp_stack_allocate(size_t size)
>    if (stack == NULL) {
>      stack = _Workspace_Allocate(size);
>    }
> -
> +
Not sure why the above two lines are removing and adding seeming the
same thing. Watch out for this.

> +#ifdef USE_THREAD_STACK_PROTECTION
> +  /*
> +  This is a hard coded address, assigned just for the
> +  purpose of consistency
> +  */
Not sure what "consistency" means here. the comment is unclear.

> +  page_table_base = (uintptr_t)0x1000;
Avoid adding extra whitespaces at the end of the line

> +  /*
> +  The current way to allocate protected stack is to assign memory attributes
> +  to the allocated memory and remove memory-entry of every other stack
> +  */
> +  prot_stack_allocate( (uintptr_t)stack, size, page_table_base );
if you're not actually allocating something, you may not want to call
it "allocate" -- think about what it does when you go through your
renaming

> +#endif
>    return stack;
>  }
>
> -void bsp_stack_free(void *stack)
> +void bsp_stack_free(void *stack)
avoid (spurious) changes in existing code

>  {
>    bool ok = _Heap_Free(&bsp_stack_heap, stack);
>
> diff --git a/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am b/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am
> index cfd59475c2..490f99792e 100644
> --- a/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am
> +++ b/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am
> @@ -74,6 +74,9 @@ librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/i2c/cadence-i2c.
>  # Cache
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/shared/cache/cache-l2c-310.c
>
> +#MMU
> +librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> +
>  # Start hooks
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/start/bspstarthooks.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/start/bspstartmmu.c
> @@ -85,4 +88,4 @@ librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/xilinx-zynq/start/bspstartmm
>
>  include $(srcdir)/../../../../../../bsps/shared/irq-sources.am
>  include $(srcdir)/../../../../../../bsps/shared/shared-sources.am
> -include $(srcdir)/../../../../../../bsps/arm/xilinx-zynq/headers.am
> +include $(srcdir)/../../../../../../bsps/arm/xilinx-zynq/headers.am
> \ No newline at end of file
> diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
> index 51f38c84c7..f3b373376b 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/stackmanagement.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 fcf679f09d..9388aa0c8c 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/memorymanagement.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/stackmanagement.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/memorymanagement.h b/cpukit/include/rtems/score/memorymanagement.h
> new file mode 100644
> index 0000000000..c712e7301a
> --- /dev/null
> +++ b/cpukit/include/rtems/score/memorymanagement.h
> @@ -0,0 +1,22 @@
> +#ifndef _RTEMS_SCORE_MEMORYMANAGEMENT_H
> +#define _RTEMS_SCORE_MEMORYMANAGEMENT_H
> +
> +#include <stdlib.h>
What do you need this header for?

> +#include <stdint.h>
This should get picked up if you include basedefs like other score headers do.

> +
> +typedef enum
> +{
> +    READ_WRITE,
> +    READ_WRITE_CACHED,
> +    READ_ONLY,
> +    READ_ONLY_CACHED,
> +    NO_ACCESS
> +} memory_flags;
> +

Need documentation. Look at other score headers to guide you.

> +void memory_entries_set(uintptr_t begin_addr, size_t size, memory_flags flags);
> +
> +void memory_entries_unset(uintptr_t begin_addr, size_t size);
> +
> +uint32_t memory_translate_flags(memory_flags flags);

Does this need to be part of the public-facing interface?

> +
> +#endif
> \ No newline at end of file
> diff --git a/cpukit/include/rtems/score/stackmanagement.h b/cpukit/include/rtems/score/stackmanagement.h
> new file mode 100644
> index 0000000000..53b6a23af0
> --- /dev/null
> +++ b/cpukit/include/rtems/score/stackmanagement.h
> @@ -0,0 +1,49 @@
> +#ifndef _RTEMS_SCORE_STACKMANAGEMENT_H
> +#define _RTEMS_SCORE_STACKMANAGEMENT_H
> +
> +#if defined (ASM)
> +#include <rtems/asm.h>
> +#else
This also gets handled by basedefs.

> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <rtems/score/memorymanagement.h>
> +#include <rtems/score/chainimpl.h>
> +
> +typedef struct stack_attr
> +{
> +  Chain_Node    node;
> +  uintptr_t        stack_address;
> +  size_t        size;
> +  uint32_t      page_table_base;
> +  memory_flags  access_flags;
> +}stack_attr;
space after }

> +
> +typedef struct stack_attr_shared
> +{
> +  stack_attr    Base;
> +  Chain_Control shared_node_control;
> +}stack_attr_shared;
> +
> +typedef struct stack_attr_prot
> +{
> +  stack_attr_shared  *shared_stacks;
> +  stack_attr    Base;
Typically 'Base' should come first in the struct definition. This way,
the start of the struct is identical to its Base

> +  bool          current_stack;
> +} stack_attr_prot;
> +
> +
> +void prot_stack_allocate(uintptr_t stack_address, size_t size, uintptr_t page_table_base);
> +
> +
> +void prot_stack_share(stack_attr_shared *shared, stack_attr_prot prot_stack);
> +
> +stack_attr_prot *prot_stack_context_initialize(void);
> +
> +void prot_stack_context_switch(stack_attr_prot *stack_attr);
> +
> +void prot_stack_context_restore(stack_attr_prot *stack_attr);
> +
> +#endif
> +
> +#endif
> \ No newline at end of file
> diff --git a/cpukit/score/cpu/arm/cpu.c b/cpukit/score/cpu/arm/cpu.c
> index 07b9588afd..aa3626965a 100644
> --- a/cpukit/score/cpu/arm/cpu.c
> +++ b/cpukit/score/cpu/arm/cpu.c
> @@ -97,6 +97,9 @@ void _CPU_Context_Initialize(
>  {
>    (void) new_level;
>
> +  #if defined (USE_THREAD_STACK_PROTECTION)
> +  the_context->stack_attr = prot_stack_context_initialize();
> +  #endif
>    the_context->register_sp = (uint32_t) stack_area_begin + stack_area_size;
>    the_context->register_lr = (uint32_t) entry_point;
>    the_context->isr_dispatch_disable = 0;
> diff --git a/cpukit/score/cpu/arm/cpu_asm.S b/cpukit/score/cpu/arm/cpu_asm.S
> index 66f8ba6032..0133746b82 100644
> --- a/cpukit/score/cpu/arm/cpu_asm.S
> +++ b/cpukit/score/cpu/arm/cpu_asm.S
> @@ -36,7 +36,6 @@
>  #ifdef ARM_MULTILIB_ARCH_V4
>
>          .text
> -
spurious

>  /*
>   *  void _CPU_Context_switch( run_context, heir_context )
>   *  void _CPU_Context_restore( run_context, heir_context )
> @@ -65,6 +64,16 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)
>  #endif
>
>         str     r3, [r0, #ARM_CONTEXT_CONTROL_ISR_DISPATCH_DISABLE]
> +#ifdef USE_THREAD_STACK_PROTECTION
> +    mov r8, r0
> +       mov r9, r1
check alignment

> +       mov r10, r2
> +       ldr r0, [r0, #112]
> +       bl  prot_stack_context_switch

Is this a function call from inside the context switch code? That does
not seem like a good design.

> +       mov r0, r8
> +       mov r1, r9
> +       mov r2, r10
> +#endif
>
>  #ifdef RTEMS_SMP
>         /*
> @@ -133,6 +142,17 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)
>   */
>  DEFINE_FUNCTION_ARM(_CPU_Context_restore)
>          mov     r1, r0
> +#if defined( USE_THREAD_STACK_PROTECTION )
> +       mov r8, r0
> +       mov r9, r1
> +       mov r10, r2
> +       ldr             r0, [r0, #ARM_STACK_PROT_ATTR_OFFSET]
> +       bl      prot_stack_context_restore
ditto

> +       ldr     lr, [r2]
> +       mov r0, r8
> +       mov r1, r9
> +       mov r2, r10
> +#endif
>         GET_SELF_CPU_CONTROL    r2
>          b       .L_restore
>
> diff --git a/cpukit/score/cpu/arm/include/rtems/score/cpu.h b/cpukit/score/cpu/arm/include/rtems/score/cpu.h
> index b7b48a3ac3..fb91142f35 100644
> --- a/cpukit/score/cpu/arm/include/rtems/score/cpu.h
> +++ b/cpukit/score/cpu/arm/include/rtems/score/cpu.h
> @@ -34,6 +34,7 @@
>  #include <rtems/score/paravirt.h>
>  #endif
>  #include <rtems/score/arm.h>
> +#include <rtems/score/stackmanagement.h>
>
>  /**
>   * @addtogroup RTEMSScoreCPUARM
> @@ -157,6 +158,21 @@
>
>  #ifdef ARM_MULTILIB_HAS_THREAD_ID_REGISTER
>    #define ARM_CONTEXT_CONTROL_THREAD_ID_OFFSET 44
> +  #ifdef ARM_MULITLIB_VFP
> +    #define ARM_STACK_PROT_ATTR_OFFSET   112
> +  #else
> +    #define ARM_STACK_PROT_ATTR_OFFSET  48
> +  #endif
> +#endif
> +
> +#ifdef USE_THREAD_STACK_PROTECTION
> +  #if defined ARM_MULITLIB_VFP
> +    #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET   112
> +  #elif ARM_MULTILIB_HAS_THREAD_ID_REGISTER
> +    #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 48
> +  #else
> +    #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 44
> +  #endif
>  #endif
>
>  #ifdef ARM_MULTILIB_VFP
> @@ -185,6 +201,7 @@
>
>  #define ARM_VFP_CONTEXT_SIZE 264
>
> +
ws

>  #ifndef ASM
>
>  #ifdef __cplusplus
> @@ -235,6 +252,9 @@ typedef struct {
>  #ifdef RTEMS_SMP
>    volatile bool is_executing;
>  #endif
> +#ifdef USE_THREAD_STACK_PROTECTION
> +stack_attr_prot *stack_attr;
> +#endif
>  } Context_Control;
>
>  static inline void _ARM_Data_memory_barrier( void )
> diff --git a/cpukit/score/src/stackmanagement.c b/cpukit/score/src/stackmanagement.c
> new file mode 100644
> index 0000000000..ae66099b14
> --- /dev/null
> +++ b/cpukit/score/src/stackmanagement.c
> @@ -0,0 +1,143 @@
> +#include <rtems/score/stackmanagement.h>
> +#include <rtems/score/chainimpl.h>
> +#include <rtems/score/basedefs.h>
> +
> +Chain_Control prot_node_control = CHAIN_INITIALIZER_EMPTY(prot_node_control);
> +
> +Chain_Control *shared_node_control;
> +
> +static void prot_stack_prev_entry_remove(void)
> +{
> +
> + Chain_Node *node;
> + stack_attr_prot *stack_attr;
> +
> +  if( _Chain_Is_empty(&prot_node_control) == true ) {
> +      _Chain_Initialize_empty(&prot_node_control);
> +  }
> +     node = _Chain_First( &prot_node_control );
> +
> +     while(_Chain_Is_tail(&prot_node_control, node) == false) {
> +
> +        stack_attr = RTEMS_CONTAINER_OF(node,stack_attr_prot, Base.node);
> +
> +        if( stack_attr->current_stack == false ) {

Is there more than one current_stack?

If not, why can't you just keep track of it in a variable rather than
iterate through a list looking for it?

> +            memory_entries_unset(stack_attr->Base.stack_address, stack_attr->Base.size);
> +         //   shared_stack_entry_remove(stack_attr->shared_stacks);
> +
> +        }
> +        node =  _Chain_Immutable_next( node );
> +     }
> +
> +}
> +
> +/*
> +Iterate to the end of the chain and mark all the 'currnet' stack as false
> +Append the current stack attribute to the end of the chain
> +*/
> +static void prot_stack_chain_append (Chain_Control *control, stack_attr_prot *stack_append_attr)
> +{
> +    Chain_Node *node;
> +    stack_attr_prot *present_stacks_attr;
> +
> +    if(_Chain_Is_empty(&prot_node_control) == true ) {
> +
> +    _Chain_Initialize_one(&prot_node_control, &stack_append_attr->Base.node);
> +    } else {
> +        node = _Chain_First(&prot_node_control);
> +
> +        while(_Chain_Is_tail(&prot_node_control,node) == false) {
> +
> +            present_stacks_attr = RTEMS_CONTAINER_OF(node, stack_attr_prot, Base.node);
> +            present_stacks_attr->current_stack = false;
> +            node = _Chain_Immutable_next( node );
> +        }
> +        _Chain_Append_unprotected(&prot_node_control, &stack_append_attr->Base.node);
> +    }
> +
> +}
> +
> +void prot_stack_allocate(uintptr_t  stack_address, size_t size, uintptr_t  page_table_base)
> +{
> +    stack_attr_prot *stack_attr;
> +
> +/*This field will be refactored and score objects will be used for dynamic allocation*/
> +    stack_attr = malloc(sizeof(stack_attr_prot));
> +
> +    if(stack_attr != NULL) {
> +    stack_attr->Base.stack_address = stack_address;
> +    stack_attr->Base.size = size;
> +    stack_attr->Base.page_table_base = page_table_base;
> +    stack_attr->Base.access_flags = READ_WRITE_CACHED;
> +    stack_attr->current_stack = true;
> +    }
> +
> +    /*
> +    Add the attribute field to the end of the chain, remove the memory entries of
> +    previously allocated stack and set the memory entry of the currnet stack.
> +    */
> +    prot_stack_chain_append(&prot_node_control, stack_attr );
> +    prot_stack_prev_entry_remove();
> +    memory_entries_set(stack_address, size, READ_WRITE_CACHED);
> +
> +}
> +
> +stack_attr_prot *prot_stack_context_initialize(void)
> +{
> +    Chain_Node *node;
> +    stack_attr_prot *stack_attr;
> +
> +    if(   _Chain_Is_empty(&prot_node_control) == false ) {
> +        node = _Chain_First( &prot_node_control );
> +
> +        while( _Chain_Is_tail(&prot_node_control, node ) == false) {
> +            stack_attr = RTEMS_CONTAINER_OF( node, stack_attr_prot, Base.node);
> +
> +            if(stack_attr->current_stack == true) {
> +                return stack_attr;
> +            } else {
> +                node = _Chain_Immutable_next( node );
> +            }
> +        }
> +    }
> +
> +    return stack_attr;
> +}
> +
> +void prot_stack_context_switch(stack_attr_prot *stack_attr)
> +{
> +    void *stack_address;
> +    size_t  size;
> +    Chain_Node *node;
> +    Chain_Control *shared_node_control;
> +
> +     /*
> +      Remove the stacks shared with the current stack by iterating the chain
> +     */
> +    if( stack_attr != NULL) {
> +
> +    stack_address = stack_attr->Base.stack_address;
> +    size = stack_attr->Base.size;
> +
> +        if(stack_attr->current_stack == true) {
> +        memory_entries_unset(stack_address, size);
> +        stack_attr->current_stack = false;
> +        }
> +
> +    shared_node_control = &stack_attr->shared_stacks->shared_node_control;
> +    }
> +}
> +
> +void prot_stack_context_restore(stack_attr_prot *stack_attr)
> +{
> +    void *stack_address;
> +    size_t  size;
> +    Chain_Node *node;
> +    Chain_Control *shared_node_control;
> +    memory_flags flags;
> +
> +     if(stack_attr->current_stack == true) {
> +             memory_entries_set(stack_address, size, READ_WRITE_CACHED);

Should you be reading these flags from somewhere
(stack_attr->Base.access_flags?), instead of hard-coding them?

> +        }
> +
> +}
> --
> 2.17.1
>


More information about the devel mailing list