Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions kernel/include/sentry/managers/task.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,5 +214,18 @@ kstatus_t mgr_task_local_ipc_iterate(taskh_t owner, taskh_t *peer, uint8_t *idx)
*/
secure_bool_t mgr_task_has_respawned(taskh_t t);

/**
* @brief return the limit address of the bottom of the stack
*
* This address is useful for hardwares that support stack overflow detection,
* as it can be used as a stack guard limit registers.
*
* @param t task handle
* @param stack_limit output parameter to store the stack limit address
* @return K_STATUS_OKAY if the stack limit has been successfully retrieved,
* K_ERROR_INVPARAM if the task handle is invalid or stack limit cannot be retrieved
*/
kstatus_t mgr_task_get_stack_size(const taskh_t h, size_t* stack_limit);


#endif/*!SECURITY_MANAGER_H*/
7 changes: 7 additions & 0 deletions kernel/src/arch/asm-cortex-m/handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,13 @@ stack_frame_t *Default_SubHandler(stack_frame_t *frame)
/* clearing the sysreturn. next job is no more syscall-preempted */
mgr_task_clear_sysreturn(next);
}
#if CONFIG_ARCH_ARM_ARMV8MML

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is avaiable for armv8.1m too
To be checked for armv8m-baseline (i.e. cortex m23).

May be use a dedicated config entry (e.g. HAS_STACK_LIMIT_REG which is set to y in the corresponding arch or family config entry)

/* ARMv8-M Mainline support PSPLIM register to detect stack overflow */
size_t stack_limit = 0;
if (likely(mgr_task_get_stack_size(next, &stack_limit) == K_STATUS_OKAY)) {
asm volatile ("MSR psplim, %0" : : "r" (stack_limit) );
Comment on lines +342 to +346
}
Comment thread
PThierry marked this conversation as resolved.
#endif
return newframe;
}

Expand Down
31 changes: 31 additions & 0 deletions kernel/src/managers/task/task_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,37 @@ size_t mgr_task_get_text_region_size(const task_meta_t *meta)
/* got and data in flash are excluded (no need) */
}

/*@
assigns \nothing;

behavior ok:
assumes stack_limit != \null;
assigns *stack_limit;
behavior err:
assumes stack_limit == \null;
assigns \nothing;

complete behaviors;
disjoint behaviors;
*/
Comment thread
PThierry marked this conversation as resolved.
kstatus_t mgr_task_get_stack_size(const taskh_t h, size_t* stack_limit)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function mgr_task_get_stack_size returns stack_limit and not stack size, should be renamed

{
kstatus_t status = K_ERROR_INVPARAM;
task_t *tsk = task_get_from_handle(h);
Comment thread
PThierry marked this conversation as resolved.
if (unlikely(tsk == NULL)) {
pr_err("invalid task handle!");
goto err;
}
if (unlikely(stack_limit == NULL)) {
pr_err("invalid stack limit pointer!");
goto err;
}
*stack_limit = tsk->stack_limit;
status = K_STATUS_OKAY;
Comment on lines +108 to +119
err:
return status;
}

void task_dump_table(void)
{
#ifndef CONFIG_BUILD_TARGET_RELEASE
Expand Down
8 changes: 8 additions & 0 deletions kernel/src/managers/task/task_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,21 @@ typedef struct task {
secure_bool_t sysretassigned; /**< a syscall has assigned a sysreturn */
Status sysreturn; /**< current job syscall return */
secure_bool_t has_respawned; /**< SECURE_TRUE if the task has been respawned */
size_t stack_limit; /**< stack limit address, based on metadata info */
} task_t;


kstatus_t task_set_job_layout(task_t * const tsk);

/*@
assigns \nothing;
ensures \valid(\result);
*/
task_t *task_get_table(void);

/*@
assigns \nothing;
*/
task_t *task_get_from_handle(taskh_t h);

void task_dump_table(void);
Expand Down
6 changes: 5 additions & 1 deletion kernel/src/managers/task/task_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,11 @@ kstatus_t task_do_initiate_localinfo(task_meta_t const * const meta, task_t *tas
*/
task_ctx->sp = mgr_task_initialize_sp(0UL, stack_top, (meta->s_text + meta->entrypoint_offset), meta->s_got);
#endif

if (unlikely(stack_top < meta->stack_size)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very (very) unlikely for task, lower memory should always be reserved for kernel.
This could be enforced at build time in babrican too (at memory layout forge time and device tree sanity checks, this may require a dedicated issue, i.e. check for region overlap and out of bound)

pr_err("security invalid stack size, not mappable ");
panic(PANIC_CONFIGURATION_MISMATCH);
}
task_ctx->stack_limit = stack_top - meta->stack_size;
Comment on lines +257 to +261
task_ctx->state = JOB_STATE_READY;
task_ctx->sysretassigned = SECURE_FALSE;
task_ctx->returncode = 0UL;
Expand Down
Loading