From 818a96d3afdd784a9e7a9c98df2998060999fa5c Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Fri, 3 Nov 2017 09:00:35 -0700 Subject: [PATCH] userspace: assign thread IDs at build time Kernel object metadata had an extra data field added recently to store bounds for stack objects. Use this data field to assign IDs to thread objects at build time. This has numerous advantages: * Threads can be granted permissions on kernel objects before the thread is initialized. Previously, it was necessary to call k_thread_create() with a K_FOREVER delay, assign permissions, then start the thread. Permissions are still completely cleared when a thread exits. * No need for runtime logic to manage thread IDs * Build error if CONFIG_MAX_THREAD_BYTES is set too low Signed-off-by: Andrew Boie --- include/kernel.h | 5 --- kernel/include/kernel_structs.h | 5 --- kernel/init.c | 8 ---- kernel/thread.c | 72 +-------------------------------- kernel/userspace.c | 52 ++++++++++++++++-------- kernel/userspace_handler.c | 4 +- scripts/gen_kobject_list.py | 22 +++++++++- 7 files changed, 59 insertions(+), 109 deletions(-) diff --git a/include/kernel.h b/include/kernel.h index 4df4646d3c8..0ce43c50d71 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -398,11 +398,6 @@ struct _thread_base { /* this thread's entry in a timeout queue */ struct _timeout timeout; #endif - -#ifdef CONFIG_USERSPACE - /* Bit position in kernel object permissions bitfield for this thread */ - unsigned int perm_index; -#endif }; typedef struct _thread_base _thread_base_t; diff --git a/kernel/include/kernel_structs.h b/kernel/include/kernel_structs.h index 867c6fe2f04..a135e13a2eb 100644 --- a/kernel/include/kernel_structs.h +++ b/kernel/include/kernel_structs.h @@ -118,11 +118,6 @@ struct _kernel { struct k_thread *threads; /* singly linked list of ALL threads */ #endif -#if defined(CONFIG_USERSPACE) - /* 0 bits for ids currently in use, 1 for free ids */ - u8_t free_thread_ids[CONFIG_MAX_THREAD_BYTES]; -#endif - /* arch-specific part of _kernel */ struct _kernel_arch arch; }; diff --git a/kernel/init.c b/kernel/init.c index a83a138bffa..ee2f981ff79 100644 --- a/kernel/init.c +++ b/kernel/init.c @@ -259,18 +259,10 @@ static void prepare_multithreading(struct k_thread *dummy_thread) dummy_thread->stack_info.start = 0; dummy_thread->stack_info.size = 0; #endif -#ifdef CONFIG_USERSPACE - dummy_thread->base.perm_index = 0; -#endif #endif /* _kernel.ready_q is all zeroes */ -#ifdef CONFIG_USERSPACE - /* Mark all potential IDs as available */ - memset(_kernel.free_thread_ids, 0xFF, CONFIG_MAX_THREAD_BYTES); -#endif - /* * The interrupt library needs to be initialized early since a series * of handlers are installed into the interrupt table to catch diff --git a/kernel/thread.c b/kernel/thread.c index 6b50242c3fb..2e1b457fddf 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -28,64 +28,6 @@ extern struct _static_thread_data _static_thread_data_list_start[]; extern struct _static_thread_data _static_thread_data_list_end[]; -#ifdef CONFIG_USERSPACE -static int thread_count; - -/* - * Fetch an unused thread ID. Returns -1 if all thread IDs are in use - */ -static int get_next_thread_index(void) -{ - int key, pos = -1; - - key = irq_lock(); - - if (thread_count == CONFIG_MAX_THREAD_BYTES * 8) { - /* We have run out of thread IDs! */ - goto out; - } - - /* find an unused bit in the kernel's bitfield of in-use thread IDs */ - for (int i = 0; i < CONFIG_MAX_THREAD_BYTES; i++) { - int fs; - - fs = find_lsb_set(_kernel.free_thread_ids[i]); - if (fs) { - /* find_lsb_set counts bit positions starting at 1 */ - --fs; - _kernel.free_thread_ids[i] &= ~(1 << fs); - pos = fs + (i * 8); - break; - } - } - - thread_count++; -out: - irq_unlock(key); - - return pos; -} - -static void free_thread_index(int id) -{ - int index, key; - u8_t bit; - - if (id == -1) { - return; - } - - key = irq_lock(); - - thread_count--; - index = id / 8; - bit = 1 << (id % 8); - _kernel.free_thread_ids[index] |= bit; - - irq_unlock(key); -} -#endif - #define _FOREACH_STATIC_THREAD(thread_data) \ for (struct _static_thread_data *thread_data = \ _static_thread_data_list_start; \ @@ -324,7 +266,6 @@ void _setup_new_thread(struct k_thread *new_thread, _new_thread(new_thread, stack, stack_size, entry, p1, p2, p3, prio, options); #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; @@ -417,11 +358,6 @@ _SYSCALL_HANDLER(k_thread_create, (void *)margs->arg6, (void *)margs->arg7, prio, options); - if (new_thread->base.perm_index == -1) { - k_thread_abort(new_thread); - _SYSCALL_VERIFY_MSG(0, "too many threads created"); - } - if (delay != K_FOREVER) { schedule_new_thread(new_thread, delay); } @@ -580,12 +516,8 @@ void _k_thread_single_abort(struct k_thread *thread) _k_object_uninit(thread->stack_obj); _k_object_uninit(thread); - if (thread->base.perm_index != -1) { - free_thread_index(thread->base.perm_index); - - /* Revoke permissions on thread's ID so that it may be recycled */ - _thread_perms_all_clear(thread); - } + /* Revoke permissions on thread's ID so that it may be recycled */ + _thread_perms_all_clear(thread); #endif } diff --git a/kernel/userspace.c b/kernel/userspace.c index 8f333e65c32..9c68778c3bf 100644 --- a/kernel/userspace.c +++ b/kernel/userspace.c @@ -92,6 +92,19 @@ struct perm_ctx { struct k_thread *parent; }; +static int thread_index_get(struct k_thread *t) +{ + struct _k_object *ko; + + ko = _k_object_find(t); + + if (!ko) { + return -1; + } + + return ko->data; +} + static void wordlist_cb(struct _k_object *ko, void *ctx_ptr) { struct perm_ctx *ctx = (struct perm_ctx *)ctx_ptr; @@ -105,30 +118,31 @@ static void wordlist_cb(struct _k_object *ko, void *ctx_ptr) void _thread_perms_inherit(struct k_thread *parent, struct k_thread *child) { struct perm_ctx ctx = { - parent->base.perm_index, - child->base.perm_index, + thread_index_get(parent), + thread_index_get(child), parent }; - if ((ctx.parent_id < MAX_THREAD_BITS) && - (ctx.child_id < MAX_THREAD_BITS)) { + if ((ctx.parent_id != -1) && (ctx.child_id != -1)) { _k_object_wordlist_foreach(wordlist_cb, &ctx); } } void _thread_perms_set(struct _k_object *ko, struct k_thread *thread) { - if (thread->base.perm_index < MAX_THREAD_BITS) { - sys_bitfield_set_bit((mem_addr_t)&ko->perms, - thread->base.perm_index); + int index = thread_index_get(thread); + + if (index != -1) { + sys_bitfield_set_bit((mem_addr_t)&ko->perms, index); } } void _thread_perms_clear(struct _k_object *ko, struct k_thread *thread) { - if (thread->base.perm_index < MAX_THREAD_BITS) { - sys_bitfield_clear_bit((mem_addr_t)&ko->perms, - thread->base.perm_index); + int index = thread_index_get(thread); + + if (index != -1) { + sys_bitfield_clear_bit((mem_addr_t)&ko->perms, index); } } @@ -141,29 +155,33 @@ static void clear_perms_cb(struct _k_object *ko, void *ctx_ptr) void _thread_perms_all_clear(struct k_thread *thread) { - if (thread->base.perm_index < MAX_THREAD_BITS) { - _k_object_wordlist_foreach(clear_perms_cb, - (void *)thread->base.perm_index); + int index = thread_index_get(thread); + + if (index != -1) { + _k_object_wordlist_foreach(clear_perms_cb, (void *)index); } } static int thread_perms_test(struct _k_object *ko) { + int index; + if (ko->flags & K_OBJ_FLAG_PUBLIC) { return 1; } - if (_current->base.perm_index < MAX_THREAD_BITS) { - return sys_bitfield_test_bit((mem_addr_t)&ko->perms, - _current->base.perm_index); + index = thread_index_get(_current); + if (index != -1) { + return sys_bitfield_test_bit((mem_addr_t)&ko->perms, index); } return 0; } static void dump_permission_error(struct _k_object *ko) { + int index = thread_index_get(_current); printk("thread %p (%d) does not have permission on %s %p [", - _current, _current->base.perm_index, + _current, index, otype_to_str(ko->type), ko->name); for (int i = CONFIG_MAX_THREAD_BYTES - 1; i >= 0; i--) { printk("%02x", ko->perms[i]); diff --git a/kernel/userspace_handler.c b/kernel/userspace_handler.c index 3f2d9d048de..f78e17f78ec 100644 --- a/kernel/userspace_handler.c +++ b/kernel/userspace_handler.c @@ -39,7 +39,7 @@ _SYSCALL_HANDLER(k_object_access_grant, object, thread) { struct _k_object *ko; - _SYSCALL_OBJ(thread, K_OBJ_THREAD); + _SYSCALL_OBJ_INIT(thread, K_OBJ_THREAD); ko = validate_any_object((void *)object); _SYSCALL_VERIFY_MSG(ko, "object %p access denied", (void *)object); _thread_perms_set(ko, (struct k_thread *)thread); @@ -51,7 +51,7 @@ _SYSCALL_HANDLER(k_object_access_revoke, object, thread) { struct _k_object *ko; - _SYSCALL_OBJ(thread, K_OBJ_THREAD); + _SYSCALL_OBJ_INIT(thread, K_OBJ_THREAD); ko = validate_any_object((void *)object); _SYSCALL_VERIFY_MSG(ko, "object %p access denied", (void *)object); _thread_perms_clear(ko, (struct k_thread *)thread); diff --git a/scripts/gen_kobject_list.py b/scripts/gen_kobject_list.py index 895ef25469d..40cdb1483f2 100755 --- a/scripts/gen_kobject_list.py +++ b/scripts/gen_kobject_list.py @@ -62,6 +62,7 @@ def kobject_to_enum(ko): DW_OP_addr = 0x3 DW_OP_fbreg = 0x91 STACK_TYPE = "_k_thread_stack_element" +thread_counter = 0 # Global type environment. Populated by pass 1. type_env = {} @@ -90,12 +91,22 @@ def debug_die(die, text): class KobjectInstance: def __init__(self, type_obj, addr): + global thread_counter + self.addr = addr self.type_obj = type_obj - # these two are set later + # Type name determined later since drivers needs to look at the + # API struct address self.type_name = None - self.data = 0 + + if self.type_obj.name == "k_thread": + # Assign an ID for this thread object, used to track its + # permissions to other kernel objects + self.data = thread_counter + thread_counter = thread_counter + 1 + else: + self.data = 0 class KobjectType: @@ -587,8 +598,15 @@ def main(): elf = ELFFile(fp) args.little_endian = elf.little_endian syms = get_symbols(elf) + max_threads = syms["CONFIG_MAX_THREAD_BYTES"] * 8 objs = find_kobjects(elf, syms) + if thread_counter > max_threads: + sys.stderr.write("Too many thread objects (%d)\n" % thread_counter) + sys.stderr.write("Increase CONFIG_MAX_THREAD_BYTES to %d\n", + -(-thread_counter // 8)); + sys.exit(1) + with open(args.output, "w") as fp: write_gperf_table(fp, objs, syms["_static_kernel_objects_begin"], syms["_static_kernel_objects_end"])