From bca15da650a24953ab8f15dc9da21a7e2ba893c4 Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Sun, 15 Oct 2017 14:17:48 -0700 Subject: [PATCH] userspace: treat thread stacks as kernel objects We need to track permission on stack memory regions like we do with other kernel objects. We want stacks to live in a memory area that is outside the scope of memory domain permission management. We need to be able track what stacks are in use, and what stacks may be used by user threads trying to call k_thread_create(). Some special handling is needed because thread stacks appear as variously-sized arrays of struct _k_thread_stack_element which is just a char. We need the entire array to be considered an object, but also properly handle arrays of stacks. Validation of stacks also requires that the bounds of the stack are not exceeded. Various approaches were considered. Storing the size in some header region of the stack itself would not allow the stack to live in 'noinit'. Having a stack object be a data structure that points to the stack buffer would confound our current APIs for declaring stacks as arrays or struct members. In the end, the struct _k_object was extended to store this size. Signed-off-by: Andrew Boie --- include/arch/arm/arch.h | 4 +- include/arch/x86/arch.h | 4 +- include/kernel.h | 46 ++++++++------- include/linker/kobject-text.ld | 2 +- kernel/thread.c | 3 + kernel/userspace.c | 2 + scripts/gen_kobject_list.py | 103 +++++++++++++++++++++++---------- 7 files changed, 105 insertions(+), 59 deletions(-) diff --git a/include/arch/arm/arch.h b/include/arch/arm/arch.h index 374b0102d7c..86d5ea0fa17 100644 --- a/include/arch/arm/arch.h +++ b/include/arch/arm/arch.h @@ -135,7 +135,7 @@ extern "C" { * @param size Size of the stack memory region */ #define _ARCH_THREAD_STACK_DEFINE(sym, size) \ - struct _k_thread_stack_element __noinit __aligned(STACK_ALIGN) \ + struct _k_thread_stack_element __kernel_noinit __aligned(STACK_ALIGN) \ sym[size+MPU_GUARD_ALIGN_AND_SIZE] /** @@ -152,7 +152,7 @@ extern "C" { * @param size Size of the stack memory region */ #define _ARCH_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \ - struct _k_thread_stack_element __noinit __aligned(STACK_ALIGN) \ + struct _k_thread_stack_element __kernel_noinit __aligned(STACK_ALIGN) \ sym[nmemb][size+MPU_GUARD_ALIGN_AND_SIZE] /** diff --git a/include/arch/x86/arch.h b/include/arch/x86/arch.h index 7428cdf112d..e0bc6c5c7ee 100644 --- a/include/arch/x86/arch.h +++ b/include/arch/x86/arch.h @@ -718,12 +718,12 @@ static inline int _arch_is_user_context(void) #endif /* CONFIG_X86_STACK_PROTECTION */ #define _ARCH_THREAD_STACK_DEFINE(sym, size) \ - struct _k_thread_stack_element __noinit \ + struct _k_thread_stack_element __kernel_noinit \ __aligned(_STACK_BASE_ALIGN) \ sym[(size) + _STACK_GUARD_SIZE] #define _ARCH_THREAD_STACK_ARRAY_DEFINE(sym, nmemb, size) \ - struct _k_thread_stack_element __noinit \ + struct _k_thread_stack_element __kernel_noinit \ __aligned(_STACK_BASE_ALIGN) \ sym[nmemb][ROUND_UP(size, _STACK_BASE_ALIGN) + \ _STACK_GUARD_SIZE] diff --git a/include/kernel.h b/include/kernel.h index 7bc2397afef..c822e27fb9a 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -142,6 +142,7 @@ enum k_objects { K_OBJ_STACK, K_OBJ_THREAD, K_OBJ_TIMER, + K_OBJ__THREAD_STACK_ELEMENT, /* Driver subsystems */ K_OBJ_DRIVER_ADC, @@ -177,6 +178,7 @@ struct _k_object { u8_t perms[CONFIG_MAX_THREAD_BYTES]; u8_t type; u8_t flags; + u32_t data; } __packed; #define K_OBJ_FLAG_INITIALIZED BIT(0) @@ -261,6 +263,26 @@ __syscall void k_object_access_revoke(void *object, struct k_thread *thread); */ void k_object_access_all_grant(void *object); +/* Using typedef deliberately here, this is quite intended to be an opaque + * type. K_THREAD_STACK_BUFFER() should be used to access the data within. + * + * The purpose of this data type is to clearly distinguish between the + * declared symbol for a stack (of type k_thread_stack_t) and the underlying + * buffer which composes the stack data actually used by the underlying + * thread; they cannot be used interchangably as some arches precede the + * stack buffer region with guard areas that trigger a MPU or MMU fault + * if written to. + * + * APIs that want to work with the buffer inside should continue to use + * char *. + * + * Stacks should always be created with K_THREAD_STACK_DEFINE(). + */ +struct __packed _k_thread_stack_element { + char data; +}; +typedef struct _k_thread_stack_element *k_thread_stack_t; + /* timeouts */ struct _timeout; @@ -422,6 +444,8 @@ struct k_thread { #if defined(CONFIG_USERSPACE) /* memory domain info of the thread */ struct _mem_domain_info mem_domain_info; + /* Base address of thread stack */ + k_thread_stack_t stack_obj; #endif /* CONFIG_USERSPACE */ /* arch-specifics: must always be at the end */ @@ -511,28 +535,6 @@ extern void k_call_stacks_analyze(void); /* end - thread options */ #if !defined(_ASMLANGUAGE) - -/* Using typedef deliberately here, this is quite intended to be an opaque - * type. K_THREAD_STACK_BUFFER() should be used to access the data within. - * - * The purpose of this data type is to clearly distinguish between the - * declared symbol for a stack (of type k_thread_stack_t) and the underlying - * buffer which composes the stack data actually used by the underlying - * thread; they cannot be used interchangably as some arches precede the - * stack buffer region with guard areas that trigger a MPU or MMU fault - * if written to. - * - * APIs that want to work with the buffer inside should continue to use - * char *. - * - * Stacks should always be created with K_THREAD_STACK_DEFINE(). - */ -struct __packed _k_thread_stack_element { - char data; -}; -typedef struct _k_thread_stack_element *k_thread_stack_t; - - /** * @brief Create a thread. * diff --git a/include/linker/kobject-text.ld b/include/linker/kobject-text.ld index 5a46a286f26..b08e822f6f3 100644 --- a/include/linker/kobject-text.ld +++ b/include/linker/kobject-text.ld @@ -2,7 +2,7 @@ #if defined(CONFIG_DEBUG) || defined(CONFIG_STACK_CANARIES) #define KOBJECT_TEXT_AREA 256 #else -#define KOBJECT_TEXT_AREA 118 +#define KOBJECT_TEXT_AREA 128 #endif #endif diff --git a/kernel/thread.c b/kernel/thread.c index 34eeb02b570..a2436d1ee00 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -326,6 +326,8 @@ void _setup_new_thread(struct k_thread *new_thread, #ifdef CONFIG_USERSPACE new_thread->base.perm_index = get_next_thread_index(); _k_object_init(new_thread); + _k_object_init(stack); + new_thread->stack_obj = stack; /* Any given thread has access to itself */ k_object_access_grant(new_thread, new_thread); @@ -501,6 +503,7 @@ void _k_thread_single_abort(struct k_thread *thread) * and triggers errors if API calls are made on it from user threads */ _k_object_uninit(thread); + _k_object_uninit(thread->stack_obj); if (thread->base.perm_index != -1) { free_thread_index(thread->base.perm_index); diff --git a/kernel/userspace.c b/kernel/userspace.c index 67659292437..267ddb4578e 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -41,6 +41,8 @@ const char *otype_to_str(enum k_objects otype) return "k_thread"; case K_OBJ_TIMER: return "k_timer"; + case K_OBJ__THREAD_STACK_ELEMENT: + return "k_thread_stack_t"; /* Driver subsystems */ case K_OBJ_DRIVER_ADC: diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 761f0e9023c..3efb9510759 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -29,6 +29,7 @@ kobjects = [ "k_stack", "k_thread", "k_timer", + "_k_thread_stack_element", "device" ] @@ -65,6 +66,7 @@ def kobject_to_enum(ko): DW_OP_addr = 0x3 DW_OP_fbreg = 0x91 +STACK_TYPE = "_k_thread_stack_element" # Global type environment. Populated by pass 1. type_env = {} @@ -89,12 +91,38 @@ def debug_die(die, text): debug("File '%s', line %d:" % (fn, ln)) debug(" %s" % text) - # --- type classes ---- +class KobjectInstance: + def __init__(self, type_obj, addr): + self.addr = addr + self.type_obj = type_obj + + # these two are set later + self.type_name = None + self.data = 0 + + +class KobjectType: + def __init__(self, offset, name, size, api=False): + self.name = name + self.size = size + self.offset = offset + self.api = api + + def __repr__(self): + return "" % self.name + + def has_kobject(self): + return True + + def get_kobjects(self, addr): + return {addr: KobjectInstance(self, addr)} + + class ArrayType: - def __init__(self, offset, num_members, member_type): - self.num_members = num_members + def __init__(self, offset, elements, member_type): + self.elements = elements self.member_type = member_type self.offset = offset @@ -109,9 +137,35 @@ class ArrayType: def get_kobjects(self, addr): mt = type_env[self.member_type] + + # Stacks are arrays of _k_stack_element_t but we want to treat + # the whole array as one kernel object (a thread stack) + # Data value gets set to size of entire region + if isinstance(mt, KobjectType) and mt.name == STACK_TYPE: + # An array of stacks appears as a multi-dimensional array. + # The last size is the size of each stack. We need to track + # each stack within the array, not as one huge stack object. + *dimensions, stacksize = self.elements + num_members = 1 + for e in dimensions: + num_members = num_members * e + + ret = {} + for i in range(num_members): + a = addr + (i * stacksize) + o = mt.get_kobjects(a) + o[a].data = stacksize + ret.update(o) + return ret + objs = {} - for i in range(self.num_members): + # Multidimensional array flattened out + num_members = 1 + for e in self.elements: + num_members = num_members * e + + for i in range(num_members): objs.update(mt.get_kobjects(addr + (i * mt.size))) return objs @@ -191,22 +245,6 @@ class AggregateType: return objs -class KobjectType: - def __init__(self, offset, name, size, api=False): - self.name = name - self.size = size - self.offset = offset - self.api = api - - def __repr__(self): - return "" % self.name - - def has_kobject(self): - return True - - def get_kobjects(self, addr): - return {addr: self} - # --- helper functions for getting data from DIEs --- def die_get_name(die): @@ -268,8 +306,7 @@ def analyze_die_const(die): def analyze_die_array(die): type_offset = die_get_type_offset(die) - elements = 1 - size_found = False + elements = [] for child in die.iter_children(): if child.tag != "DW_TAG_subrange_type": @@ -281,10 +318,9 @@ def analyze_die_array(die): if not ub.form.startswith("DW_FORM_data"): continue - size_found = True - elements = elements * (ub.value + 1) + elements.append(ub.value + 1) - if not size_found: + if not elements: return type_env[die.offset] = ArrayType(die.offset, elements, type_offset) @@ -442,12 +478,13 @@ def find_kobjects(elf, syms): ret = {} for addr, ko in all_objs.items(): # API structs don't get into the gperf table - if ko.api: + if ko.type_obj.api: continue - if ko.name != "device": + if ko.type_obj.name != "device": # Not a device struct so we immediately know its type - ret[addr] = kobject_to_enum(ko.name) + ko.type_name = kobject_to_enum(ko.type_obj.name) + ret[addr] = ko continue # Device struct. Need to get the address of its API struct, if it has @@ -458,7 +495,8 @@ def find_kobjects(elf, syms): continue apiobj = all_objs[apiaddr] - ret[addr] = subsystem_to_enum(apiobj.name) + ko.type_name = subsystem_to_enum(apiobj.type_obj.name) + ret[addr] = ko debug("found %d kernel object instances total" % len(ret)) return ret @@ -504,7 +542,8 @@ void _k_object_wordlist_foreach(_wordlist_cb_func_t func, void *context) def write_gperf_table(fp, objs, static_begin, static_end): fp.write(header) - for obj_addr, obj_type in objs.items(): + for obj_addr, ko in objs.items(): + obj_type = ko.type_name # pre-initialized objects fall within this memory range, they are # either completely initialized at build time, or done automatically # at boot during some PRE_KERNEL_* phase @@ -516,8 +555,8 @@ def write_gperf_table(fp, objs, static_begin, static_end): val = "\\x%02x" % byte fp.write(val) - fp.write("\",{},%s,%s\n" % (obj_type, - "K_OBJ_FLAG_INITIALIZED" if initialized else "0")) + fp.write("\",{},%s,%s,%d\n" % (obj_type, + "K_OBJ_FLAG_INITIALIZED" if initialized else "0", ko.data)) fp.write(footer)